Answer a query that fails to build in its place in the pipeline

[chrbala]

When a query throws while it is built (an undefined value, more than 65534 parameters, a serializer that throws), execute has already queued it to await an answer, but nothing of it was written. The catch then rejected the connection's current query with the new query's error and left the failed query waiting for an answer that never comes, so every later answer on the connection went to the wrong query:

await sql`select 1`
const a = sql`select pg_sleep(0.1), 1 as x`
    , b = sql`select ${ undefined }`
    , c = sql`select 3 as x`
// a rejects with UNDEFINED_VALUE, b with "Cannot set properties of
// null (setting 'columns')", c never settles, nor does any later query

This is also why sql.begin(sql => [sqlselect 1, sqlselect ${ undefined }]) never settles (#1082). On a reserved connection with the transaction pipelined (begin, insert, failing query, insert, commit), both inserts committed.

Send a query the server is sure to refuse in the failed query's place. The failed query then gets an answer of its own, every later answer stays with its query, and a transaction it was part of is aborted rather than committed without it. The failed query is rejected with its own error, as a retried query already is. It is no longer marked to be described first or as a cursor, since either would make the server's error send a Sync of its own and shift the next answer.

Fixes #1082

[chrbala]

I made a few PRs from Claude which fix some problems I was having. Can you take a look?

[garrytan]

An independent production report for this one, from garrytan/gbrain on postgres 3.4.9 under Bun 1.4, against Postgres 16 both directly and through PgBouncer in transaction mode.

How it looked in production: with prepare: false and shared_types: true, the follower behind the unbuildable statement is describe-first. It never gets its ParameterDescription, so its Bind/Execute/Sync is never written, and the backend sits in active / ClientRead until the process exits. With max: 1 the whole pool wedges. With a larger pool each occurrence permanently loses one connection, and while that happens a health check that only borrows idle connections keeps reporting OK.

const sql = postgres(url, { max: 1, prepare: false, shared_types: true })
await sql`select 1`
const head    = sql`select pg_sleep(0.3), 'head' as who`
const culprit = sql`select ${ undefined }::text as who`
const next    = sql`select 'next' as who, ${ 1 }::int as n`
// 3.4.9: head rejects UNDEFINED_VALUE, culprit gets a TypeError on query.statement, next never settles

We vendored this PR's approach: on a build failure, send a statement the server is certain to refuse in the culprit's place. It fixes all of our cases: the pipelined culprit above; a reserved, pipelined begin, insert, <unbuildable>, insert, commit, which now yields BEGIN, INSERT, UNDEFINED_VALUE, 25P02, ROLLBACK with nothing committed; the same inside sql.begin; and MAX_PARAMETERS_EXCEEDED. In every case the backend goes back to idle. An earlier attempt of ours only rejected the culprit and let the pipeline continue, and it committed both inserts. So the placeholder approach here is the right call, not just a cosmetic one. +1 for merging.

[GrahamCampbell]

We fixed the same fault another way, in execute's catch: a query that fails to build while it is the current query is rejected after a Sync, as before, and one that fails while it waits behind others is taken back off sent and rejected alone, so nothing of it reaches the server and every other answer stays with its query. Against this PR's tests that passes the pipelined, cursor, too-many-parameters and sql.begin (#1082) cases, the last because the scope rolls back on the rejection; it differs only for a transaction pipelined by hand on a reserved connection, where the rest of the pipeline runs and commits without the failed query (BEGIN,INSERT,UNDEFINED_VALUE,INSERT,COMMIT,2), which your placeholder aborts instead, the safer result for that case; we only note the difference.

    } catch (error) {
      query === q
        ? (write(Sync), errored(error))
        : (sent.remove(q), queryError(q, error))
      return true
    }