Skip to content

Fix query macros against CockroachDB by gating plan_cache_mode on EXPLAIN support - #4286

Merged
abonander merged 1 commit into
transact-rs:mainfrom
BastienClement:fix-crdb-plan-cache-mode
Jul 2, 2026
Merged

abonander merged 1 commit into
transact-rs:mainfrom
BastienClement:fix-crdb-plan-cache-mode

Conversation

@BastienClement

Copy link
Copy Markdown
Contributor

Problem

Since 0.9.0 the compile-time query macros (query!, query_as!,
query_scalar!) fail against CockroachDB:

error: error returned from database: unimplemented: SET usage inside a
function definition (SQLSTATE 0A000)

#3541 prepares the macros' describe connection by running
SET plan_cache_mode = 'force_generic_plan' inside a DO $$ … $$ block.
CockroachDB rejects SET inside a function/DO body at parse time, so the
block fails before its IF EXISTS guard runs. Closes #4274.

Fix

plan_cache_mode = force_generic_plan exists solely to keep EXPLAIN-based
nullability inference from being skewed by the NULL placeholder arguments
bound during describe. That inference already runs only when
is_explain_available() is true — false for CockroachDB, Materialize and
QuestDB (detected via their crdb_version / mz_version / questdb_version
parameter statuses).

The setting is moved out of the generic macro layer into PgConnection,
behind that same is_explain_available() check (plus a
server_version_num() >= 12 guard in place of the old pg_settings probe).
The macro layer now calls a per-database DatabaseExt::prepare_describe_connection
hook (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 describe tests). describe() itself is untouched,
so no reused connection is affected.

Testing

  • Existing tests/postgres/describe.rs suite passes, plus a new
    it_forces_generic_plan_for_describe regression test.
  • Verified manually against CockroachDB v26.2.1: before this change a query!
    macro fails with SQLSTATE 0A000; afterwards it compiles and plan_cache_mode
    is 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 warnings and rustfmt --check are clean.

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.
@abonander
abonander merged commit 1d674f5 into transact-rs:main Jul 2, 2026
147 checks passed
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]>
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.

query! macros fail against CockroachDB in 0.9.0: SET inside DO block not supported

2 participants