postgres: record which connection counter a request bumped as one enum

[alii]

A request's dispatch bumps either nonpipelinable_requests or pipelined_requests, and finish_request has to undo the same one. That was tracked with counted plus pipelined, and finish_request picked the counter by re-reading the query's simple flag, which only works because the nonpipelinable increments happen to sit inside the simple branches. This replaces the two bools with a RequestCounter enum (None / Nonpipelinable / Pipelined) written at the five increment sites and matched in finish_request, so the decrement follows what was actually incremented and the counted-but-neither state cannot be expressed. simple stays a bool because it also selects the wire protocol.

No behavior change. Ran the mock-server postgres tests around request accounting (finish-request-underflow, split-prepare-reorder, failed-connection-resurrection, frame-boundary, multi-statement-fields, datarow-overrun) and simple-query-pipeline, prepared-pipeline-reorder and sql.test.ts against a local postgres on the debug build, plus a script mixing simple, pipelined and failing queries on one connection.

[robobun]

^{Updated 12:57 AM PT - Aug 13th, 2026}

✅ @alii, your commit 88ef80505787fb1ee04d82f207b2a6dad2aaaef7 passed in Build #93924! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38000

That installs a local version of the PR into your bun-38000 executable, so you can run:

bun-38000 --bun

[claude]

LGTM — mechanical refactor replacing two coupled bools with one three-state enum; no behavior change.

What was reviewed:

• Traced all five increment sites (2 nonpipelinable, 3 pipelined) and confirmed each records the same counter it bumps; finish_request matches exhaustively on it.
• Grepped for remaining .pipelined/.counted field reads — none; simple is correctly kept as a separate bool (still read at PostgresSQLConnection.rs:1856 for wire-protocol selection).
• Checked that the old if !counted { return } early-exit vs the new RequestCounter::None => {} fall-through are equivalent — nothing follows the match in finish_request.

Extended reasoning...

Overview

This PR replaces the counted: bool + pipelined: bool pair on PostgresSQLQuery::Flags with a single counter: RequestCounter enum (None / Nonpipelinable / Pipelined). The enum is written at each of the five sites that increment a connection request counter, and finish_request matches on it to decrement the same counter. Two files touched: PostgresSQLQuery.rs (struct/enum definition, default, two write sites in do_run) and PostgresSQLConnection.rs (finish_request rewrite, three write sites in the advance loop).

Security risks

None. Pure internal bookkeeping refactor of in-process counters; no user input parsing, no allocation changes, no FFI/GC surface touched.

Level of scrutiny

Low-to-medium. It is a type-driven refactor with a stated "no behavior change" claim, so the review focused on proving equivalence. The old finish_request chose the counter to decrement by re-reading flags.simple (falling back to flags.pipelined); I verified that at every site where the old code set counted = true, the new enum variant matches what the old branch would have picked: the two Nonpipelinable writes sit inside simple branches, and the three Pipelined writes were the exact sites that also set pipelined = true. The one structural difference — the old code returned early on !counted while the new code falls through an empty None arm — is inert because nothing follows the match in finish_request.

Other factors

Grep confirms no stragglers reading the removed pipelined/counted fields anywhere in src/sql_jsc/postgres/. simple remains a standalone bool and is still read for protocol selection (req.flags.get().simple at line 1856), so keeping it separate is correct rather than folding it into the enum. The PR description lists the request-accounting regression tests (finish-request-underflow, split-prepare-reorder, etc.) as run against the debug build. This is exactly the "make illegal states unrepresentable" cleanup the review guide encourages, small and self-contained.

[coderabbitai]

Walkthrough

The PostgreSQL driver replaces separate boolean request flags with RequestCounter. Query dispatch records request type, and connection completion clears the state while decrementing the matching non-pipelined or pipelined count.

Changes

PostgreSQL request accounting

Layer / File(s)	Summary
Counter state and query dispatch src/sql_jsc/postgres/PostgresSQLQuery.rs	Flags now stores RequestCounter. Simple and prepared query dispatch records the corresponding counter variant.
Connection request accounting src/sql_jsc/postgres/PostgresSQLConnection.rs	Simple, prepared, and unnamed prepared requests assign counter variants. finish_request clears the counter and decrements the matching connection-level count.

Suggested reviewers: jarred-sumner, robobun

Mergeability Score: 🟡 Moderate · up to 88ef8

A write-allocation failure can leave a connection with request counters and buffered protocol data out of sync, potentially causing response mismatches or connection instability. This bounded correctness risk should be fixed or explicitly accepted before merging.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: replacing separate request flags with one enum that records the connection counter used.
Description check	✅ Passed	The description explains the change, its rationale, behavior impact, and verification steps, although it does not use the template headings.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/sql_jsc/postgres/PostgresSQLQuery.rs`:
- Line 556: Update the write_item failure path in the surrounding
request-writing logic to treat allocation failure as a connection failure:
either reserve the FIFO entry before writing so buffered protocol bytes remain
matched, or abort the connection while clearing the writer and request counters,
including RequestCounter::Nonpipelinable and pipelined_requests. Preserve
query-reference cleanup and ensure no unmatched buffered request remains.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7d5574d4-19e9-4139-a055-3d5a31984188

📥 Commits

Reviewing files that changed from the base of the PR and between f1b3a92 and 88ef805.

📒 Files selected for processing (2)

• src/sql_jsc/postgres/PostgresSQLConnection.rs
• src/sql_jsc/postgres/PostgresSQLQuery.rs

[alii]

@robobun get this mergeable

[robobun]

Nothing left to change on the branch. The one open review thread (write_item OOM ordering) is pre-existing, unrelated to this diff, answered in-thread and resolved; the hardening it asked for is tracked separately. No conflicts with main (nothing under src/sql_jsc changed since the branch point), and CI on 88ef805 is green on every lane, including the full sql.test.ts (850 pass) on the x64 and x64-asan shards. I also re-ran the mock-server request accounting suites (finish-request-underflow, split-prepare-reorder, failed-connection-resurrection, frame-boundary, multi-statement-fields, datarow-overrun, simple-query-pipeline, prepared-pipeline-reorder, error-then-datarow) on a local debug build: 36 pass, 0 fail.

The only thing still blocking the merge button is an approving review (branch protection reports review required), which has to come from a maintainer other than the author.