Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Closed
simonw opened this issue Aug 19, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@simonw
Copy link
Owner

simonw commented Aug 19, 2023

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 simonw added the bug Something isn't working label Aug 19, 2023
@simonw
Copy link
Owner Author

simonw commented Aug 19, 2023

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 simonw changed the title New .add_foreign_key() can break if PRAGMA legacy_alter_table=ON New .add_foreign_key() can break if PRAGMA legacy_alter_table=ON and there's an invalid foreign key reference Aug 19, 2023
@simonw
Copy link
Owner Author

simonw commented Aug 19, 2023

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
Copy link
Owner Author

simonw commented Aug 19, 2023

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.

@simonw simonw closed this as completed Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant