Skip to content

fix(sqlite): stop multi-statement queries copying their tail on every prepare - #4419

Open
andeki92 wants to merge 2 commits into
transact-rs:mainfrom
andeki92:sqlite-nul-terminated-tail
Open

andeki92 wants to merge 2 commits into
transact-rs:mainfrom
andeki92:sqlite-nul-terminated-tail

Conversation

@andeki92

Copy link
Copy Markdown

Does your PR solve an issue?

fixes #4415

The query buffer now ends in a NUL terminator that every tail keeps, so sqlite3_prepare_v3 parses in place instead of copying the rest of the query for each statement. Executing N statements goes from quadratic to linear: 80,000 statements in one raw_sql call take 201 ms instead of 4.78 s.

  • prepare() now makes one sqlite3_prepare_v3 call per invocation. prepare_next already calls it again when no statement comes back, and the one-call shape lets the regression test see every tail handed to SQLite.
  • A query containing a NUL byte used to hang the worker thread. VirtualStatement::new now rejects it with Error::InvalidArgument, and prepare() returns an error instead of looping when SQLite consumes nothing.

Tests:

  • every_tail_handed_to_sqlite_ends_in_nul (unit): the regression test. It fails before the fix.
  • prepare_errors_when_sqlite_consumes_nothing (unit) and it_rejects_a_query_containing_a_nul_byte: both hang before the fix.
  • it_executes_statements_followed_by_whitespace_or_a_comment and it_executes_a_query_that_is_only_a_comment: they pass before and after, and cover the new loop-exit check.

Run locally on the pinned 1.94.1 toolchain: cargo fmt --check, ./x.py --clippy (20/20, no warnings), and cargo test -p sqlx-sqlite --all-features. The full ./x.py matrix passes except for 29 MariaDB targets that fail the same way on main: the client_ssl URL in x.py is missing the leading . in ssl-key=%2Ftests…, and the 10.6 and 10.11 images don't enable TLS while x.py asks for ssl-mode=required. I ran the Postgres/MySQL/MariaDB targets in a Linux container, because macOS native-tls can't do a TLS handshake with the Ed25519 test certificates.

Is this a breaking change?

No. Any query that returned before returns the same results now. The only behaviour change is for queries containing a NUL byte, which used to hang forever and now return an error.


Worked on with the help of Claude Opus 5.5.

…r copies the rest

prepare() now makes one sqlite3_prepare_v3 call per invocation: prepare_next already calls it again when no statement came back, and the one-call shape lets the regression test see every tail handed to SQLite.

Refs transact-rs#4415
@andeki92

Copy link
Copy Markdown
Author

The MySQL 8 failures look unrelated to this change: every connection in --test any gets EOF before the server greeting, in the step that drops ssl-mode=disabled. sqlx-sqlite isn't compiled in these jobs, and the same job failed identically on #4413 (run 35537003373), which changes nothing the MySQL job builds. Could a maintainer re-run the failed jobs?

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: executing a multi-statement string is quadratic in its length, because every sqlite3_prepare_v3 call copies the whole remaining tail

1 participant