[GrahamCampbell]

This is stacked on #1212, whose commit is the first one here, unchanged; once #1212 is merged, this is the last two commits. It contains #1239's change, so the two conflict.

A prepared statement the server answers as stale (cached plan must not change result type after a column change, prepared statement ... does not exist after DEALLOCATE ALL or behind a pooler) is sent again, but the resend does not hold its connection and is written behind whatever is already pipelined. On a busy pool the resend and another statement can then resolve with each other's rows, with no error when the rows have the same shape. Two stale statements piped together leave the second pending until the next statement's answer arrives, which it takes, while its cached statement keeps that statement's columns. A stale write can be resent inside a transaction begun behind it and rolled back with it.

This holds the connection during a resend, through a new onfull callback, and resends only when the statement is not cancelled, nothing is pipelined behind it and its ReadyForQuery says idle. Otherwise the cached statement is evicted and the statement rejects with the server's error.

That is a trade, made on purpose: a stale statement with another statement pipelined behind it now fails once with the server's error (0A000, 42804 or 26000) instead of being resent, and its next run prepares afresh. A stale statement with nothing behind it is still resent as before.

• Seven tests, each failing on Send transaction control on the simple protocol so prepare: true never names commit #1212 and passing with the change. Fix: Do not mutate parameters when serializing in Bind - fixes multiple serialization on retry #1198's three deallocate all tests now use their own client.
• The full suite passes in ESM, CJS and Deno against PostgreSQL 17.

Mechanism, the trade in full, reproduction and output

This is stacked on #1212 (transaction control on the simple protocol), whose commit is the first one here, unchanged. Like #1239, the status check below needs it: without #1212, a rollback whose prepared statement the server no longer has, after DEALLOCATE ALL or behind a pooler, fails inside the aborted transaction and is not resent, so sql.begin rejects with 26000 and leaves its connection in that transaction. This also contains #1239's change (the idle check and forget), so the two conflict; if #1239 is merged first, this reduces to the !query.cancelled && !sent.length terms, onfull and the tests. Once #1212 is merged, this is the last two commits.

A prepared statement answered with an error whose routine is in retryRoutines (FetchPreparedStatement, RevalidateCachedQuery, transformAssignedExpr) is sent again, which is how an application gets past #120's cached plan must not change result type when a column is added to a table it reads with select * or returning *. retry evicts the cached statement and calls execute again. That puts the statement at the tail of sent, and build finds it no longer prepared, so a statement with parameters becomes describe-first: only Parse/Describe/Flush is written, and its Bind/Execute/Sync follows when its ParameterDescription arrives. A statement without parameters is resent whole in one write. The resend has two faults: it does not hold its connection, and it is written behind whatever is already pipelined.

execute returns false for a describe-first statement. Everywhere else that false moves the connection to full, so the pool hands it nothing more until that statement has been answered: go, onopen, and the reserve() and begin() handlers. retry drops it, so a connection that was in busy stays there, and when the pool has no open or closed connection, handler gives the next statement to it. That statement's Bind/Execute/Sync goes out between the resend's describe and its Bind, and the server answers in that order while the resend is current, so the resend resolves with the other statement's rows decoded with its own columns, and the other statement, current next, resolves with the resend's. When the rows have the same shape, as in cases 2 and 3 below, nothing reports an error.

When the stale statement's error arrives with another statement already in sent, the resend is written behind that statement. If that statement is stale too, it fails the same way and is resent, and two describes are in flight: the second statement waits for a ParameterDescription that has already gone by, and settles only when the next statement on the connection is answered, with that statement's rows, while its cached statement keeps that statement's columns for every later run (case 4). If it is a statement with parameters running for the first time, a stale statement without parameters resent behind it is answered out of turn: the stale statement rejects with a TypeError, and the statement behind it resolves with the stale statement's rows and caches its columns, so its next run fails with ERR_OUT_OF_RANGE. If it is a write, the resent write runs after it: on a reserve() connection, an insert resent behind the update that followed it leaves the row as the insert wrote it, and both resolve. And the resend is decided by the status byte of the stale statement's own ReadyForQuery, but runs after everything in sent. sql.begin sends its BEGIN like any statement, so on a busy connection it can be pipelined behind a stale autocommit write, whose ReadyForQuery still says idle. The resend then goes out after the BEGIN and runs inside the transaction, the write resolves with its row, and the transaction's rollback erases it (case 5). #1239 (for #1234) reads the same byte and resends here too.

This change holds the connection, and resends only where the resend runs as the original statement would have. retry reports a false return from execute through a new onfull callback, beside onopen, onend and onclose, and the pool answers it with move(c, full), the hold it gives any describe-first statement. A reserve() connection is held the same way: its handler queues while the connection is in full, and connection.reserved() hands it back at the resend's ReadyForQuery. The statement is resent only when it is not cancelled, nothing is pipelined behind it, and its ReadyForQuery says idle; otherwise #1239's forget evicts the cached statement and rejects the statement with the server's error. With nothing behind it, the status the statement was answered with is the status its resend runs in, and no other describe can be in flight ahead of the resend. Inside a transaction the status is E after the error, so nothing is resent there, as with #1239. The !sent.length term is also what makes the hold safe: with onfull and without it, case 4's second statement keeps its connection in full behind a describe that never completes, and that connection and every statement given to it stay pending for good.

The !query.cancelled term is for #1252's cancelled statements. A statement cancelled while it waits pipelined, that stays current because it was last in sent and is answered with one of these errors, goes to retry, whose execute returns at once for a cancelled statement, so it never settles. With this change it rejects with its own error.

This is a trade, made on purpose. A stale statement that has another statement pipelined behind it on its connection when its error arrives now fails once with the server's error instead of being resent. That is 0A000 (cached plan must not change result type) after a column change, 42804 (from transformAssignedExpr) after a column's type changes under a cached insert or update, or 26000 (prepared statement ... does not exist) after DEALLOCATE ALL, after DISCARD ALL or behind a pooler that loses named statements, with or without parameters, and on a reserve() connection as on the pool. Its cached statement is evicted, so its next run prepares afresh, and a stale statement with nothing behind it is still resent as before. Where the statements behind a stale statement without parameters are fully written, open no transaction and do not depend on it having run first, master's resend was correct, so there the change costs one failure and gains nothing. For a statement with parameters, master's resend was right only if nothing reached the connection during its describe. The connection keeps no state that would tell those followers apart from a describe in flight, a BEGIN or a dependent write, so the stale statement fails once behind any of them. In return, the hold fixes case 2 without the permanent hang it would otherwise cause in case 4, and the term ends case 3's crossed answers (there the stream is already pipelined behind the stale statement, so read(1) now fails once instead), the second statement of case 4 taking another statement's answer, the write resent into a transaction begun after it, the stale statement answered with a first-run statement's rows while that statement caches the wrong columns, and the writes on a reservation run out of order.

#1198's three deallocate all tests now use their own client. On the suite's shared max: 1 connection they left every statement cached before them stale, and "reserve connection" then pipelined a stale select 1 as x ahead of select 3 as x on a reservation, which this change fails once with 26000. The new tests are "Fails a stale prepared statement once when a query is piped behind it" (the trade, and the next run preparing afresh), "Holds the connection while retrying a prepared statement" (case 2), "Answers two stale prepared statements piped together" (case 4), "Does not retry a stale prepared statement inside a transaction begun behind it" (case 5), "Does not give a stale prepared statement the answer of a statement piped behind it", "Does not reorder writes on a reserved connection when retrying a stale prepared statement" and "Does not retry a cancelled stale prepared statement". Each fails on #1212 and passes with this change. The last commit is the output of npm run build. Since master's generated copies have not been rebuilt since #1198, it carries #1198's line and tests into them as well, and it can be dropped if you would rather build them yourself.

Only postgres, against a throwaway server started from the stock image:

docker run --rm -d --name pg-resend-repro -p 127.0.0.1:55432:5432 -e POSTGRES_PASSWORD=repro postgres:17
PGHOST=127.0.0.1 PGPORT=55432 PGUSER=postgres PGPASSWORD=repro PGDATABASE=postgres node repro.mjs

import postgres from 'postgres'

// Every pool takes its server and credentials from PGHOST, PGPORT, PGUSER, PGPASSWORD and PGDATABASE.
const pool = options => postgres({ max: 1, onnotice() {}, ...options })
const other = pool()
const settle = (p, ms = 3000) => Promise.race([
  p.then(x => 'resolved ' + JSON.stringify(x), e => 'rejected ' + (e.code || e.message)),
  new Promise(r => setTimeout(r, ms, `STILL PENDING after ${ ms } ms`))
])

async function table() {
  await other`drop table if exists t`
  await other`create table t (id int primary key, v text)`
  await other`insert into t values (1, 'a'), (2, 'b')`
}

// A column added by another session makes a warm `select * from t` stale: its next run fails with
// 0A000 "cached plan must not change result type" (RevalidateCachedQuery), and the driver resends it.
const stale = () => other`alter table t add column extra text`

console.log('1. A stale statement resent with nothing else on its connection')
{
  await table()
  const sql = pool()
  const read = id => sql`select * from t where id = ${ id }`
  await read(1)
  await stale()
  console.log('  read(1):', await settle(read(1)))
  await sql.end({ timeout: 0 })
}

console.log('2. A statement handed to the pool while the stale statement is resent')
{
  await table()
  let builds = 0
    , handed
  const sql = pool({
    // The third build of the read is its resend (warm-up, stale run, resend).
    debug: (_, string) => string === 'select * from t where id = $1' && ++builds === 3 && (handed = settle(lookup(2)))
  })
  const read = id => sql`select * from t where id = ${ id }`
  const lookup = id => sql`select id, v, null::text as extra from t where id = ${ id }`
  await read(1)
  await lookup(2)
  await stale()
  console.log('  read(1)  :', await settle(read(1)))
  console.log('  lookup(2):', await handed)
  await sql.end({ timeout: 0 })
}

console.log('3. No hook: a stream of statements, one per turn of the event loop, while the stale statement is resent')
{
  await table()
  const sql = pool()
  const read = id => sql`select * from t where id = ${ id }`
  const echo = n => sql`select ${ n }::int as id, 'stream' as v, null::text as extra`
  await read(1)
  await echo(0)
  await stale()
  const stream = []
  let done = false
  const own = settle(read(1))
  own.then(() => done = true)
  next()
  console.log('  read(1):', await own)
  const answers = await Promise.all(stream)
  console.log('  stream :', answers.filter(x => !x).length, 'of', answers.length, 'answered with another statement\'s row')
  await sql.end({ timeout: 0 })

  function next() {
    if (done)
      return
    const n = stream.length + 1
    stream.push(echo(n).then(([x]) => x.id === n && x.v === 'stream', () => false))
    setImmediate(next)
  }
}

console.log('4. Two stale statements pipelined on one connection, then the next statement')
{
  await table()
  const sql = pool()
  const read = id => sql`select * from t where id = ${ id }`
  await read(1)
  await stale()
  const first = settle(read(1))
      , second = read(2).execute()
  console.log('  read(1)            :', await first)
  console.log('  read(2)            :', await settle(second))
  console.log('  next               :', await settle(sql`select 1 as x`))
  console.log('  read(2), after next:', await settle(second))
  console.log('  read(1), run again :', await settle(read(1)))
  await sql.end({ timeout: 0 })
}

console.log('5. A stale autocommit insert with a transaction begun right behind it, whose callback throws')
{
  await other`drop table if exists w`
  await other`create table w (id serial primary key, note text)`
  const sql = pool()
  const add = note => sql`insert into w (note) values (${ note }) returning *`
  await add('warm')
  await other`alter table w add column extra text`
  const insert = settle(add('stale'))
  const transaction = settle(sql.begin(async sql => {
    await sql`select 1`
    throw new Error('rolled back')
  }))
  console.log('  insert     :', await insert)
  console.log('  transaction:', await transaction)
  console.log('  w holds    :', JSON.stringify((await other`select note from w order by id`).map(x => x.note)))
  await sql.end({ timeout: 0 })
}

await other`drop table t, w`
await other.end()
process.exit(0)

Each case uses a pool of its own with max: 1, so its statements share one connection, as on a larger pool whose connections are all busy, and other is a second client that creates the tables and adds the column that makes the warm statements stale. Case 1 is the control: a stale statement alone on its connection is resent and gets its own row. In case 2 the debug option, which build calls, hands a statement to the pool while the resend is built; it reaches the pool's handler a microtask later, after retry has returned. Case 3 has no hook, only a stream of statements of the same row shape, one per turn of the event loop, from the stale statement's first run until it settles. In case 4 the two stale reads are issued together, and next follows once the second has been pending for 3 s. In case 5 the insert and the sql.begin are issued together, so the BEGIN is pipelined behind the insert.

On master (411429e), and the same on #1212:

1. A stale statement resent with nothing else on its connection
  read(1): resolved [{"id":1,"v":"a","extra":null}]
2. A statement handed to the pool while the stale statement is resent
  read(1)  : resolved [{"id":2,"v":"b","extra":null}]
  lookup(2): resolved [{"id":1,"v":"a","extra":null}]
3. No hook: a stream of statements, one per turn of the event loop, while the stale statement is resent
  read(1): resolved [{"id":6,"v":"stream","extra":null}]
  stream : 4 of 9 answered with another statement's row
4. Two stale statements pipelined on one connection, then the next statement
  read(1)            : resolved [{"id":1,"v":"a","extra":null}]
  read(2)            : STILL PENDING after 3000 ms
  next               : rejected Cannot read properties of undefined (reading 'length')
  read(2), after next: resolved [{"x":1}]
  read(1), run again : resolved [{"x":1}]
5. A stale autocommit insert with a transaction begun right behind it, whose callback throws
  insert     : resolved [{"id":2,"note":"stale","extra":null}]
  transaction: rejected rolled back
  w holds    : ["warm"]

With this change:

1. A stale statement resent with nothing else on its connection
  read(1): resolved [{"id":1,"v":"a","extra":null}]
2. A statement handed to the pool while the stale statement is resent
  read(1)  : resolved [{"id":1,"v":"a","extra":null}]
  lookup(2): resolved [{"id":2,"v":"b","extra":null}]
3. No hook: a stream of statements, one per turn of the event loop, while the stale statement is resent
  read(1): rejected 0A000
  stream : 0 of 7 answered with another statement's row
4. Two stale statements pipelined on one connection, then the next statement
  read(1)            : rejected 0A000
  read(2)            : resolved [{"id":2,"v":"b","extra":null}]
  next               : resolved [{"x":1}]
  read(2), after next: resolved [{"id":2,"v":"b","extra":null}]
  read(1), run again : resolved [{"id":1,"v":"a","extra":null}]
5. A stale autocommit insert with a transaction begun right behind it, whose callback throws
  insert     : rejected 0A000
  transaction: rejected rolled back
  w holds    : ["warm"]

Each output was the same on a second run apart from case 3's row and counts, which follow timing. With the change, read(1) in cases 3 and 4 and the insert in case 5 fail once with 0A000, because other statements were pipelined behind each when its error arrived, and the eviction makes the next run prepare afresh (case 4's last line). Each part is needed: without onfull, case 2 prints as on master; without !sent.length, case 3's read(1) resolves with its own row, but in case 4 read(2) and every statement after it stay pending, and case 5 prints as on master; with #1239's change alone, cases 2 to 5 print as on master.

Not covered here: sql.release() called while a describe-first statement is still in flight on a reserved connection returns the connection to the pool, which can then hand it a statement before that statement's Bind is written, and any statement already queued on the reservation behind it is never sent, as on master for any describe-first statement. Since the resend now holds a reserved connection the same way, a statement sent on a reservation during a resend and followed by release() before the resend is answered now stays pending, where on master the resend took that statement's answer instead; the change leaves release() itself alone. A statement cancelled while it is running is not marked cancelled, so if one of these errors answers it before the CancelRequest arrives, it is still resent, as on master.