porsager / porsager/postgres

sql.begin() always throws UNSAFE_TRANSACTION when max_pipeline is 0

Open
#1,210 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
8.7k
Forks
374
Avg merge
11d 16h
Merged PRs (30d)
1

Description

Version: 3.4.9 (also current master, 411429e)
Node: v26.8.1, plain PostgreSQL 16 (no pooler involved in the repro)

With the connection option max_pipeline: 0, every sql.begin() rejects with UNSAFE_TRANSACTION: Only use sql.begin, sql.reserved or max: 1, even though the transaction is being run through sql.begin.

Reproduction
import postgres from 'postgres'
const url = process.env.DATABASE_URL
for (const [label, opts] of [['default', {}], ['max_pipeline: 1', { max_pipeline: 1 }], ['max_pipeline: 0', { max_pipeline: 0 }]]) {
  const sql = postgres(url, { max: 4, ...opts })
  try { const r = await sql.begin(tx => tx`select 1 as ok`); console.log(label, 'OK', r) }
  catch (e) { console.log(label, 'FAIL', e.code, e.message) }
  finally { await sql.end({ timeout: 2 }) }
}

Output on 3.4.9 / master:

default OK Result(1) [ { ok: 1 } ]
max_pipeline: 1 OK Result(1) [ { ok: 1 } ]
max_pipeline: 0 FAIL UNSAFE_TRANSACTION UNSAFE_TRANSACTION: Only use sql.begin, sql.reserved or max: 1
Cause

execute(q) in src/connection.js returns a single && chain:

      build(q)
      return write(toBuffer(q))
        && !q.describeFirst
        && !q.cursorFn
        && sent.length < max_pipeline
        && (!q.options.onexecute || q.options.onexecute(connection))

begin() in src/index.js sends BEGIN via sql.unsafe('begin ...', [], { onexecute }) and relies on that onexecute hook to capture the connection and move(c, reserved) / set c.reserved. Because the hook sits after sent.length < max_pipeline in the same chain, max_pipeline: 0 (0 < 0 is false) short-circuits before it, so the hook never runs. The connection is never reserved, and when the BEGIN's CommandComplete arrives the guard

    if (result.command === 'BEGIN' && max !== 1 && !connection.reserved)
      return errored(Errors.generic('UNSAFE_TRANSACTION', 'Only use sql.begin, sql.reserved or max: 1'))

fires. One expression is doing two unrelated jobs: "may the pool pipeline another query onto this connection" and "may the reservation hook run".

Why max_pipeline: 0

Behind a transaction-mode connection pooler (e.g. Supavisor/PgBouncer) a second query pipelined onto the socket before the first is answered is not handled correctly, so disabling pipelining entirely is the natural setting there. max_pipeline: 1 still lets one extra query be queued on the socket.

Expected / actual
  • Expected: sql.begin() works with max_pipeline: 0 exactly as with any other value; the option should only affect how many queries are queued on a connection.
  • Actual: every sql.begin() rejects with UNSAFE_TRANSACTION.

A PR with a fix and tests follows.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/connection.js and src/index.js, then run the supplied reproduction with max_pipeline set to 0, 1, and the default. Trace the execute(q) chain and the sql.begin reservation hook. Done means sql.begin() succeeds with max_pipeline: 0 and regression tests cover the option without breaking pipeline limits.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, postgresql
Domain
backend, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.