-
Notifications
You must be signed in to change notification settings - Fork 1.5k
(Fix) Handle nullability of SQLite rowid alias columns #4088
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It might be worth an additional assertion for selecting rowid directly from each of the tables, since rowid behavior is unchanged between the 2 tables.
tyrelr
commented
Nov 15, 2025
The explain.rs changes look reasonable to me. But I haven't touched sqlx in a long time, so I won't try to guess whether returning None from handle.rs column_nullable could lead to surprises.
I initially assumed that the handle.rs is not needed for the macro magic to work. But I vaguely remember some portion of the query type describe mechanism executed select queries to sniff the results... so maybe that is what forced the handle.rs changes?
Lege19
commented
Nov 15, 2025
column_nullable is only called from sqlx-sqlite/src/connection/describe.rs in function describe.
In that function the explain.rs nullability is used as a fallback.
I couldn't think of a reasonably efficient way to determine if a column was a rowid alias in column_nullable. However in explain.rs, obviously SQLite knows which columns are rowid aliases, and it uses special opcodes in those cases which are easy to recognise and handle.
The requested extra test makes sense. Will add.
Tests the nullability of explicitly referring to the rowid column (not through an alias).
Uh oh!
There was an error while loading. Please reload this page.
Does your PR solve an issue?
fixes #3967
Is this a breaking change?
This is a breaking change.
Previously an
INTEGER PRIMARY KEYcolumn was wrongly inferred to be nullable, so code usingquery!will break, as the result will now be ani64instead of anOption<i64>.query_as!should be fine though becauseOption<i64>implementsFrom<i64>This will apply anywhere someone has used an
INTEGER PRIMARY KEYand not overridden the nullability of that column in their queries.