Skip to content

fix(sqlite): correctly infer nullability for INTEGER PRIMARY KEY in RETURNING#4323

Open
at264939-ctrl wants to merge 16 commits into
transact-rs:mainfrom
at264939-ctrl:fix/sqlite-returning-rowid-nullability
Open

fix(sqlite): correctly infer nullability for INTEGER PRIMARY KEY in RETURNING#4323
at264939-ctrl wants to merge 16 commits into
transact-rs:mainfrom
at264939-ctrl:fix/sqlite-returning-rowid-nullability

Conversation

@at264939-ctrl

Copy link
Copy Markdown

fixes #4322. This is a fix for a regression introduced in 0.9.0.

After upgrading to sqlx 0.9.0, many users noticed that INSERT ... RETURNING queries on INTEGER PRIMARY KEY columns started returning Option instead of T, which broke a lot of code that expects a simple i64. I traced this down to commit 69ee0df from PR #4088.

The issue is that in sqlx-sqlite/src/statement/handle.rs, column_nullable was changed to return None for rowid aliases. This forces the logic to fall back to the bytecode simulation in explain.rs, which relies on PRAGMA table_info. In SQLite, this pragma returns notnull=0 for INTEGER PRIMARY KEY columns because they can be passed as NULL during an insert. However, in any actual result set, these columns are never NULL.

To fix this, I updated root_block_columns in explain.rs to include the pk column from table_info so the VM simulation can properly identify these rowid aliases and mark them as non-nullable. I also updated handle.rs to return Some(false) instead of None for these cases to provide a more accurate initial inference. Finally, I adjusted a few existing tests in explain.rs that were previously asserting these columns as nullable. I've verified the fix with integration tests and it correctly restores the expected i64 behavior

@at264939-ctrl

Copy link
Copy Markdown
Author

Is there no one here for review?

@uckelman uckelman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that this PR fixes the problem I reported in #4322.

I would very much like to see a test added involving INSERT ... RETURNING which triggers the problem, pre-patch, to prevent it from recurring.

Comment thread sqlx-sqlite/src/connection/explain.rs
Comment thread sqlx-sqlite/src/statement/handle.rs
Comment thread tests/sqlite/macros.rs
Comment thread sqlx-sqlite/src/types/time.rs
@uckelman

Copy link
Copy Markdown

The added test passes on its own, so does not show that the patch fixes the problem: See PR #4324.

I'm trying to find something which fails before this PR...

@at264939-ctrl

at264939-ctrl commented Jun 27, 2026

Copy link
Copy Markdown
Author

The added test passes on its own, so does not show that the patch fixes the problem: See PR #4324.

I'm trying to find something which fails before this PR...

You're right that the test may not
be sufficient to demonstrate the regression.
I will take look at PR #4324 to understand the failing scenario better.
In the meantime, can you share what you're finding fails before this PR?
That would help me write a proper regression test that actually fails
on the old code and passes with this patch

@uckelman

uckelman commented Jun 27, 2026

Copy link
Copy Markdown

I'm trying to pare down one of my cases which fails.

With this table:

CREATE TABLE IF NOT EXISTS foo (
  package_id INTEGER PRIMARY KEY NOT NULL,
  project_id INTEGER NOT NULL,
  FOREIGN KEY(project_id) REFERENCES projects(project_id)
);

this query fails to compile on main:

        let id = sqlx::query_scalar!(
            "
INSERT INTO foo (
    project_id
)
VALUES (1)
RETURNING package_id
            "
        )
        .fetch_one(&pool)
        .await
        .unwrap();

If I remove the FOREIGN KEY clause, it does compile on main.

My RETURNING queries which continued to work with 0.9.0 are exactly the ones on tables that have no foreign keys.

…TURNING clauses

- Fix INTEGER PRIMARY KEY type inference to be non-nullable (T instead of Option<T>)
- Update explain simulation to track primary key status from PRAGMA table_info
- Add handling for OP_COLUMN p2=-1 (rowid alias access) in VM simulation
- Modernize time crate format descriptions to resolve all CI deprecation warnings
- Add robust regression tests covering FOREIGN KEY and implicit rowid scenarios
Comment thread tests/sqlite/setup.sql Outdated
Comment thread tests/sqlite/setup.sql Outdated
Comment thread tests/sqlite/macros.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite INSERT ... RETURNING queries incorrectly return nullable

2 participants