Skip to content

Count DO as a schema change, and do not retry a stale statement inside a transaction - #11

Merged
necessarylion merged 1 commit into
mainfrom
schema-ddl-fixes
Nov 6, 2026
Merged

necessarylion merged 1 commit into
mainfrom
schema-ddl-fixes

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Stale prepared statements after a schema change (follow-up to #10).

  • Done: DDL inside DO. A DO block did not count as a schema change. So a table rebuilt in a DO block kept the old param types ('007' stored as '7'). Now the DO CommandComplete tag counts too. DO is mostly used for migrations, so this does not touch the hot path. New test (fails on main).
  • Already there: changed result type from another process or a function. The driver already catches "cached plan must not change result type" (0A000), drops the statement, prepares it again and sends the query once more. A code comment now says why this retry is safe: the error comes from the plan check at Bind, before the query runs. Any other error ends the query's own implicit transaction (own Sync), so the server rolls back what it did. New test with two pools (fails on main inside a transaction, see next line).
  • Fix: no retry inside a transaction. The error has already aborted the transaction, so the retry could only fail with "current transaction is aborted" (25P02). That hid the real error. Worse, a retry sent after a pipelined COMMIT would run outside the transaction. Now a query with a transaction id is not retried, and Dart gets the real 0A000 error.
  • Limit (documented): param types. DDL from another process or inside a function is still not seen for the param types. Postgres gives no error for that case, so there is nothing to catch. The limit is in a code comment and the CHANGELOG.

No native ABI change. No version bump. CHANGELOG under Unreleased.

Checks: cargo fmt / clippy / test (12 passed), dart format / analyze, Dart tests on Postgres 16 (43 passed).

Measure at release: the query path (/db, /queries). is_ddl has one more compare per CommandComplete. Expected: no change.

@mydoxie

mydoxie commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

Blocking: none.

Non-blocking:

  • native/src/conn.rs:1151: "has a transaction id" misses a transaction that the user opens by hand with query('BEGIN') on the pool. That query has tx == 0, so the retry still runs and fails with 25P02, as it did before this PR. It is safe, and nothing runs twice. A stronger test, at no cost: the status byte of ReadyForQuery. Retry only when it is I, because it is E inside a failed transaction. This would also cover the hand-made case.
  • The comment on GONE says that only effects which a rollback does not undo can happen twice. Besides nextval, session advisory locks (pg_advisory_lock) and dblink are such effects. This is only a run-time 0A000 from a cached statement in autocommit, which is very rare. The rule existed before this PR. One more example in the comment would be enough.

Checked:

  • The retry cannot run a statement twice with a visible effect. A plan-check 0A000 comes at Bind, before the statement runs. Every query has its own Sync, so in autocommit a late error rolls back that query's own implicit transaction. Inside a dox transaction (tx != 0), there is now no retry. A retry request has req: None, so it never retries again. Pipelined queries keep their own Sync each. Savepoints and BEGIN/COMMIT go through in_tx, so their queries carry the tx id. A tx == 0 query only goes to a connection with no transaction (link.rs:487).
  • DO\0 matches the CommandComplete tag. The test for a table rebuilt inside DO covers it.
  • The new Dart test proves that a query in a transaction gets the real 0A000, not 25P02.
  • Hot path: one more compare on the CommandComplete tag. No allocation, no extra round trip.
  • No ABI change. No version bump. The CHANGELOG is under ## Unreleased.
  • Local: cargo fmt --check, cargo clippy --release --all-targets -D warnings, cargo test (12 passed), dart format, dart analyze --fatal-infos: all clean. Dart tests pass on Postgres 16 (43 passed). On GitHub, test and 4 of 5 build jobs are green, and Windows is still running.

@necessarylion
necessarylion merged commit 999e8b1 into main Nov 6, 2026
8 checks passed
@necessarylion
necessarylion deleted the schema-ddl-fixes branch November 6, 2026 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants