simonw / sqlite-utils

Python CLI utility and library for manipulating SQLite databases
https://sqlite-utils.datasette.io
Apache License 2.0
1.58k stars 106 forks source link

New .add_foreign_key() can break if PRAGMA legacy_alter_table=ON and there's an invalid foreign key reference #587

Closed simonw closed 11 months ago

simonw commented 11 months ago

Extremely detailed story of how I got to this point:

Steps to reproduce (only if that pragma is on though):

python -c '
import sqlite_utils
db = sqlite_utils.Database(memory=True)
db.execute("""
CREATE TABLE "logs" (
   [id] INTEGER PRIMARY KEY,
   [model] TEXT,
   [prompt] TEXT,
   [system] TEXT,
   [prompt_json] TEXT,
   [options_json] TEXT,
   [response] TEXT,
   [response_json] TEXT,
   [reply_to_id] INTEGER,
   [chat_id] INTEGER REFERENCES [log]([id]),
   [duration_ms] INTEGER,
   [datetime_utc] TEXT
);
""")
db["logs"].add_foreign_key("reply_to_id", "logs", "id")
'

This succeeds in some environments, fails in others.

simonw commented 11 months ago

Simplest possible recreation of the bug:

python -c '
import sqlite_utils
db = sqlite_utils.Database(memory=True)
db.execute("""
CREATE TABLE "logs" (
   [id] INTEGER PRIMARY KEY,
   [chat_id] INTEGER REFERENCES [log]([id]),
   [reply_to_id] INTEGER
);
""")
db["logs"].add_foreign_key("reply_to_id", "logs", "id")
'

That chat_id line is the line that causes the problem - because it is defining a reference to a table that no longer exists!

simonw commented 11 months ago

Although this is revealing a problem in the underlying code (that schema is invalid), it also represents a regression: sqlite-utils 3.34 ran this just fine, but it fails on sqlite-utils 3.35 due to the change made in:

simonw commented 11 months ago

I'm inclined to say this isn't a bug in sqlite-utils though - it's a bug in the code that calls it. So I'm not going to fix it here.