Conversation
With `prepare: true`, `begin()` sent `savepoint`, `rollback to`, `rollback`, `commit` and `prepare transaction` as tagged templates, so each became a named prepared statement cached on the client connection. On a transaction pooler that does not track named statements (or after the pooler evicts one), the Bind of a named `commit` reaches a backend that never parsed it and fails with SQLSTATE 26000. That error aborts the transaction, and `FetchPreparedStatement` is in `retryRoutines`, so the driver re-sends `commit` on the aborted transaction. Postgres answers that COMMIT with ROLLBACK and no error, and the transaction's writes are lost silently. Reproduced against a Supavisor pooler in transaction mode with one client connection and a second client keeping the other backends busy, twenty sequential `sql.begin` inserts, body statement unnamed: before: 9 of 20 rows, 31 `commit` sends, 0 errors after: 20 of 20 rows, 20 `commit` sends, 0 errors `begin` already goes through `unsafe`. This sends the other five control statements the same way. A zero-argument `unsafe` is `simple: true`, so no `prepare` setting can name it, and the transaction lifecycle (connection close, release only when idle, per-query error capture) is unchanged. Identifier quoting matches `sql(name)`; the `prepare transaction` name doubles single quotes instead of being spliced in raw.
A prepared statement that fails because its cached plan went stale (RevalidateCachedQuery) or it no longer exists (FetchPreparedStatement) is prepared again and retried. Inside a transaction block that retry cannot work: the error has already aborted the transaction, and a query pipelined behind the failed one may already have ended it. - With concurrent queries in `sql.begin()`, the retry gets 25P02, the answers fall out of step, ROLLBACK is never completed, and the connection is never released (porsager#1234). - With the transaction pipelined on a reserved connection, the retry runs after `commit` has ended the transaction (as ROLLBACK), on its own, so the retried write lands while the rest of the transaction does not: const reserved = await sql.reserve() await Promise.all([ reserved`begin`, reserved`insert into test values (2, 2)`, reserved`update test set a = a + ${ 1 } where id = 1 returning *`, reserved`commit` ]) // after a concurrent `alter table test add column b int`: the insert // is rolled back, but the update lands after it, outside the // transaction Retry only when ReadyForQuery reports the connection idle, that is, not in a transaction block. Otherwise reject the query with its original error, and forget its statement so that its next use prepares it again. This depends on porsager#1212, which this branch is based on. Before it, `sql.begin()` sent its own `rollback` and `commit` as prepared statements, and a `rollback` whose statement had been deallocated only succeeded through this retry (the test "Properly throws routine error on not prepared statements in transaction" fails without porsager#1212). Fixes porsager#1234 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Author
|
I made a few PRs from Claude which fix some problems I was having. Can you take a look? |
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
2 participants
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.
A prepared statement that fails because its cached plan went stale
(RevalidateCachedQuery) or it no longer exists (FetchPreparedStatement)
is prepared again and retried. Inside a transaction block that retry
cannot work: the error has already aborted the transaction, and a query
pipelined behind the failed one may already have ended it.
sql.begin(), the retry gets 25P02, theanswers fall out of step, ROLLBACK is never completed, and the
connection is never released (Concurrent queries in sql.begin() after cached-plan invalidation (0A000) leave the connection 'idle in transaction (aborted)' and never release it #1234).
after
commithas ended the transaction (as ROLLBACK), on its own, sothe retried write lands while the rest of the transaction does not:
Retry only when ReadyForQuery reports the connection idle, that is, not
in a transaction block. Otherwise reject the query with its original
error, and forget its statement so that its next use prepares it again.
This depends on #1212, which this branch is based on. Before it,
sql.begin()sent its ownrollbackandcommitas preparedstatements, and a
rollbackwhose statement had been deallocated onlysucceeded through this retry (the test "Properly throws routine error on
not prepared statements in transaction" fails without #1212).
Fixes #1234
Co-Authored-By: Claude Opus 5.5 (1M context) noreply@anthropic.com