Repository navigation
Fix query macros against CockroachDB by gating plan_cache_mode on EXPLAIN support - #4286
Merged
abonander merged 1 commit intoJul 2, 2026
Merged
Conversation
The query macros set `plan_cache_mode = 'force_generic_plan'` on their describe connection (transact-rs#3541) to keep nullability inference from being skewed by the NULL placeholder arguments. It was issued inside a `DO` block, which CockroachDB rejects at parse time -- `SET` inside a function body, SQLSTATE 0A000 -- breaking the query macros against CockroachDB since 0.9.0 (transact-rs#4274). The setting only benefits the EXPLAIN-based inference, which already runs only when `is_explain_available()` is true (false for CockroachDB, Materialize and QuestDB). Gate the setting on that same check so it is never issued on a database that cannot use it, replacing the `pg_settings` probe with a `server_version_num() >= 12` guard.
This was referenced Aug 2, 2026
TheSentinel454
added a commit
to block/buzz
that referenced
this pull request
Nov 6, 2026
## Problem sqlx 0.9.0 counts a transaction only after Postgres acknowledges its `BEGIN`. If the future is dropped while `BEGIN` is still in flight (a cancelled REQ history read, a client that disconnects, a timeout), sqlx doesn't queue a `ROLLBACK`. The pool's release ping then reads the late `BEGIN` reply, and the writer pool hands the connection to the next borrower while it is still inside that transaction. The reader pool already closes such connections in its release check (`Db::read_pool_options`). The writer pool has no release check. Load tests showed what happens next: - `there is already a transaction in progress` (25001) on the next event read's `BEGIN`. - Sometimes a burst of `current transaction is aborted` (25P02). The leftover from the event read is `READ ONLY`, so the next write fails and aborts it, and every later statement on that connection fails until the connection is closed. During a burst, ban checks failed closed with 503 and window reads returned 500. - A read-write leftover would also leave autocommit writes uncommitted, and the next read's `ROLLBACK` would discard them. ## Change - Pin every sqlx crate to transact-rs/sqlx#4394 (merged, not yet released). That commit takes the transaction count before the `BEGIN` round trip, so a cancelled begin queues its `ROLLBACK`. The crates share types, so they have to move together. The pin also picks up the other 23 commits between v0.9.0 and that fix, including the libsqlite3-sys fix `sqlx-sqlite` was already pinned for. - Add a regression test, `writer_pool_postgres_tests::begin_cancelled_in_flight_leaves_no_writer_transaction_open`. It holds `BEGIN` in flight behind a paused proxy and times it out, for both the event read's `BEGIN ... READ ONLY` and a plain `begin()`. It then asserts that the next borrower's autocommit write commits and that its backend is `idle`, not inside a transaction. Other upstream changes in the pin that reach the Postgres path: `TCP_NODELAY` on sockets (transact-rs/sqlx#4336), reuse of the TLS connector across connection attempts (transact-rs/sqlx#4242), a fix for a memory leak when the statement cache is disabled (transact-rs/sqlx#4337), a describe-time generic-plan change used only by the macros (transact-rs/sqlx#4286), and the `whoami` default-user fix (transact-rs/sqlx#4303). The rest are SQLite, MySQL, CLI, or docs changes. Remove the pin once sqlx 0.9.1 ships. ## What got simpler The lockfile loses its second `sqlx-core` copy, which the sqlx-sqlite-only pin used to pull in. No runtime code changes. A pool-level release check would have fixed the same bug, but it costs a round trip on every writer release. Comments that described the 0.9.0 behavior as current are refreshed: the reader pool's release check (`Db::read_pool_options`) and the lease heartbeat (`EntityDeletionStore::heartbeat`). The reader test `proof_timeout_mid_begin_leaves_no_reader_transaction_open` asserted that the release check closed the connection left mid-`BEGIN`. With the pin, sqlx rolls that transaction back itself, so the test now asserts the check finds nothing to close. That also catches a sqlx regression the check would otherwise hide. Identified debt (not in this PR): the release check's open-transaction arm, and the reader-session `default_transaction_isolation` pin it depends on, are now only a backstop against a sqlx regression. They could be removed in a follow-up. ## Testing - Against sqlx 0.9.0 (main's `Cargo.toml`/`Cargo.lock` at 962b3fd), the new writer test fails for each `BEGIN` form on its own. The event read fails with `cannot execute INSERT in a read-only transaction`, and after a plain `begin()` the next borrower's backend is not `idle`. With the pin it passes 5/5. - Single-machine load-test A/B against main (962b3fd), with three main runs and two pin runs. Main logged 25001 and 25P02 in every run (41/80, 10/9, 12/18); the pin logged none. Client error rate, latency, and throughput matched within run-to-run noise. - Full suite on the pin: every `just ci` recipe, the PostgreSQL lane, and `just test`. The only failure from the pin was the reader test above. Reds that don't involve sqlx: the desktop Tauri recipes (`alsa-sys` needs ALSA headers on the build host), `mobile-check`/`mobile-test` (a Flutter SDK `ColorScheme` type clash), and two `buzz-acp` `git_bootstrap` tests (no git credentials on the host). Two bridge search-timeout tests failed only under the full parallel lane; run alone they pass on both the pin and 0.9.0. - Two later commits change only tests, comments, and `Cargo.lock` (the lock now moves only the sqlx crates). At those heads: the PostgreSQL lane, the unit lane, and workspace clippy pass. Three tests failed once under a contended full lane, and each passed 3/3 alone on both the pin and 0.9.0. The two transaction tests fail on 0.9.0 and pass on the pin. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist <[email protected]> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since 0.9.0 the compile-time query macros (
query!,query_as!,query_scalar!) fail against CockroachDB:#3541 prepares the macros' describe connection by running
SET plan_cache_mode = 'force_generic_plan'inside aDO $$ … $$block.CockroachDB rejects
SETinside a function/DObody at parse time, so theblock fails before its
IF EXISTSguard runs. Closes #4274.Fix
plan_cache_mode = force_generic_planexists solely to keep EXPLAIN-basednullability inference from being skewed by the
NULLplaceholder argumentsbound during
describe. That inference already runs only whenis_explain_available()is true — false for CockroachDB, Materialize andQuestDB (detected via their
crdb_version/mz_version/questdb_versionparameter statuses).
The setting is moved out of the generic macro layer into
PgConnection,behind that same
is_explain_available()check (plus aserver_version_num() >= 12guard in place of the oldpg_settingsprobe).The macro layer now calls a per-database
DatabaseExt::prepare_describe_connectionhook (a no-op for MySQL/SQLite). The setting is therefore skipped, by
construction, on every database that cannot use it — fixing CockroachDB and
pre-empting the same breakage on Materialize/QuestDB — while behaving exactly
as before on PostgreSQL.
Behaviour on standard PostgreSQL
Unchanged: the dedicated describe connection still gets
force_generic_plan(verified by the existing
describetests).describe()itself is untouched,so no reused connection is affected.
Testing
tests/postgres/describe.rssuite passes, plus a newit_forces_generic_plan_for_describeregression test.query!macro fails with SQLSTATE 0A000; afterwards it compiles and
plan_cache_modeis left untouched. CockroachDB is not in the CI matrix, so that half cannot
be covered by an automated test; the fix is structural (gated on the
pre-existing, already-tested CockroachDB detection).
cargo clippy -- -D warningsandrustfmt --checkare clean.