Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incorrect foreign key references on catalog_columns table #2466

Closed
simonw opened this issue Feb 13, 2025 · 3 comments
Closed

Incorrect foreign key references on catalog_columns table #2466

simonw opened this issue Feb 13, 2025 · 3 comments

Comments

@simonw
Copy link
Owner

simonw commented Feb 13, 2025

Spotted this while playing around with Datasette Lite and ?install=datasette-visible-internal-db:

https://lite.datasette.io/?install=datasette-visible-internal-db&ref=1.0a17#/_internal/catalog_columns?_facet=type&_facet=database_name&_facet=is_pk&is_pk=4

CREATE TABLE catalog_columns (
    database_name TEXT,
    table_name TEXT,
    cid INTEGER,
    name TEXT,
    type TEXT,
    "notnull" INTEGER,
    default_value TEXT, -- renamed from dflt_value
    is_pk INTEGER, -- renamed from pk
    hidden INTEGER,
    PRIMARY KEY (database_name, table_name, name),
    FOREIGN KEY (database_name) REFERENCES databases(database_name),
    FOREIGN KEY (database_name, table_name) REFERENCES tables(database_name, table_name)
);

Those foreign key references should point to catalog_databases and catalog_tables.

@simonw
Copy link
Owner Author

simonw commented Feb 13, 2025

There are a bunch of other incorrect foreign key references in here:

FOREIGN KEY (database_name) REFERENCES databases(database_name)
);
CREATE TABLE IF NOT EXISTS catalog_columns (
database_name TEXT,
table_name TEXT,
cid INTEGER,
name TEXT,
type TEXT,
"notnull" INTEGER,
default_value TEXT, -- renamed from dflt_value
is_pk INTEGER, -- renamed from pk
hidden INTEGER,
PRIMARY KEY (database_name, table_name, name),
FOREIGN KEY (database_name) REFERENCES databases(database_name),
FOREIGN KEY (database_name, table_name) REFERENCES tables(database_name, table_name)
);
CREATE TABLE IF NOT EXISTS catalog_indexes (
database_name TEXT,
table_name TEXT,
seq INTEGER,
name TEXT,
"unique" INTEGER,
origin TEXT,
partial INTEGER,
PRIMARY KEY (database_name, table_name, name),
FOREIGN KEY (database_name) REFERENCES databases(database_name),
FOREIGN KEY (database_name, table_name) REFERENCES tables(database_name, table_name)
);
CREATE TABLE IF NOT EXISTS catalog_foreign_keys (
database_name TEXT,
table_name TEXT,
id INTEGER,
seq INTEGER,
"table" TEXT,
"from" TEXT,
"to" TEXT,
on_update TEXT,
on_delete TEXT,
match TEXT,
PRIMARY KEY (database_name, table_name, id, seq),
FOREIGN KEY (database_name) REFERENCES databases(database_name),
FOREIGN KEY (database_name, table_name) REFERENCES tables(database_name, table_name)
);

I'm also going to add a test that checks the internal DB schema has no incorrect references like that.

@simonw
Copy link
Owner Author

simonw commented Feb 13, 2025

This test currently fails:

diff --git a/tests/test_internal_db.py b/tests/test_internal_db.py
index b41cabb4..246d795e 100644
--- a/tests/test_internal_db.py
+++ b/tests/test_internal_db.py
@@ -1,4 +1,5 @@
 import pytest
+import sqlite_utils
 
 
 # ensure refresh_schemas() gets called before interacting with internal_db
@@ -59,3 +60,25 @@ async def test_internal_foreign_keys(ds_client):
         "table_name",
         "from",
     }
+
+
+@pytest.mark.asyncio
+async def test_internal_foreign_key_references(ds_client):
+    internal_db = await ensure_internal(ds_client)
+
+    def inner(conn):
+        db = sqlite_utils.Database(conn)
+        table_names = db.table_names()
+        for table in db.tables:
+            for fk in table.foreign_keys:
+                other_table = fk.other_table
+                other_column = fk.other_column
+                message = 'Column "{}.{}" references other column "{}.{}" which does not exist'.format(
+                    table.name, fk.column, other_table, other_column
+                )
+                assert other_table in table_names, message + " (bad table)"
+                assert other_column in db[other_table].columns_dict, (
+                    message + " (bad column)"
+                )
+
+    await internal_db.execute_fn(inner)

simonw added a commit that referenced this issue Feb 13, 2025
@simonw simonw closed this as completed Feb 13, 2025
@simonw
Copy link
Owner Author

simonw commented Feb 13, 2025

Wrote this bug up here: https://simonwillison.net/2025/Feb/13/url-addressable-python/

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant