sql(postgres): route the six Bind encoder call sites through PostgresSQLConnection::encode_request; pin Bind wire bytes

Merged
RO
1mo ago
+497-115

Behaviour change: none

Part 1 of 3 of the follow-up to #34732 (plan). Rebased on main after #43898 merged.

Problem

  • Six call sites encode a request's Bind parameters: four in PostgresSQLConnection::advance and two in PostgresSQLQuery::do_run. Each calls one of three batch writers in PostgresRequest.rs directly, and advance carries seven copies of the same error arm.
  • The next two PRs change what happens around that encode. With six entry points each change needs six edits, and a seventh call site can bypass them.

Fix

  • PostgresSQLConnection::encode_request(global, EncodeRequest) is now the only route to bind_and_execute, prepare_and_query_with_signature and parse_and_bind_and_execute. They and write_bind are private to PostgresRequest.rs.
  • advance rejects a request whose write failed through one function, reject_failed_write, at all seven arms. The one difference between the old arms (a statement's first write marks the statement failed) is its new_statement argument.
  • postgres-bind-wire.test.ts pins the exact frontend bytes for 13 kinds of parameter, a binary result column, the sql() helper, no parameters, and prepare: false. It passes on main without this PR. wire-frames.ts gets frontend builders with a layout self-test.
  • Verified: existing coverage is sql.test.ts, sql-prepare-false.test.ts, postgres-prepared-pipeline-reorder.test.ts, postgres-split-prepare-reorder.test.ts, postgres-simple-query-pipeline.test.ts, postgres-bytea-bind.test.ts, wire-frames.test.ts. All 255 tests in test/js/sql/postgres-* pass on the debug build.

Background

  • A Bind carries the parameter values of one execution. Encoding them calls into JS (toString, toJSON, getters).
  • The corpus leaves out the types whose announced format and encoding disagree today (float4, numeric, time, int4[]). #41976 fixes those.

Downsides

  • Per batch: one call and one match on a 3-variant enum of 48 bytes, if encode_request is not inlined. No allocation, no syscall.

[human-review] gate passed · iteration 3 · 6 files touched

fails on main (without fix) 
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/sql/postgres-bind-wire.test.ts test/js/sql/wire-frames.test.ts
ninja: Entering directory `/workspace/bun/build/debug'
[0/2] cargo plan → /workspace/bun/build/debug/rust-target/plan.json
FAILED: [code=1] rust-target/plan.json /workspace/bun/build/debug/rust-target/plan.json 
/workspace/bun/build/release/bun /workspace/bun/scripts/build/stream.ts cargo --console /workspace/bun/build/release/bun /workspace/bun/scripts/build/rust/plan.ts /workspace/bun/build/debug/rust-target/plan.input.json /workspace/bun/build/debug/rust-target/plan.json
error: cannot update the lock file /workspace/bun/Cargo.lock because --locked was passed to prevent this
help: to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.
error: /root/.cargo/bin/cargo build -p bun_runtime --lib … exited with 101
ninja: error: rebuilding 'build.ninja': subcommand failed
error: script "bd" exited with code 1
__F:-1:S:0

release without fix: all passed
bun test v1.4.3-canary.1 (41e52b124)

test/js/sql/wire-frames.test.ts:
(pass) mysqlLenencInt encodes per page_protocol_basic_dt_integers.html [0.16ms]
(pass) pgErrorResponse encodes per §55.7 [0.17ms]
(pass) frontend message builders encode per §55.7 [0.55ms]
(pass) postgres: pgAuthenticationOk + pgReadyForQuery are accepted by Bun's parser [6.68ms]
(pass) postgres: COPY OUT response frames are consumed and the following result set decodes [3.62ms]
(pass) postgres: pgMinimalReadyServer satisfies connect() [0.88ms]
(pass) mysql: mysqlHandshakeV10 + mysqlOkPacket are accepted by Bun's parser [1.71ms]

test/js/sql/postgres-bind-wire.test.ts:
(pass) named statements > null [1.80ms]
(pass) named statements > boolean as bool [0.57ms]
(pass) named statements > integer as int4 [0.46ms]
(pass) named statements > BigInt as int8 [0.50ms]
(pass) named statements > double as float8 [0.42ms]
(pass) named statements > Date as timestamptz [0.50ms]
(pass) named statements > ASCII string as text [0.36ms]
(pass) named statements > non-ASCII string as text [0.50ms]
(pass) named statements > object as jsonb [0.62ms]
(pass) named statements > array as json [0.51ms]
(pass) named stateme
... (truncated)
passes on PR (with fix) 
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/sql/postgres-bind-wire.test.ts test/js/sql/wire-frames.test.ts
bun test v1.4.3 (367d939d9)

test/js/sql/wire-frames.test.ts:
(pass) mysqlLenencInt encodes per page_protocol_basic_dt_integers.html [12.75ms]
(pass) pgErrorResponse encodes per §55.7 [8.47ms]
(pass) frontend message builders encode per §55.7 [36.72ms]
(pass) postgres: pgAuthenticationOk + pgReadyForQuery are accepted by Bun's parser [380.29ms]
(pass) postgres: COPY OUT response frames are consumed and the following result set decodes [236.22ms]
(pass) postgres: pgMinimalReadyServer satisfies connect() [46.69ms]
(pass) mysql: mysqlHandshakeV10 + mysqlOkPacket are accepted by Bun's parser [69.90ms]

test/js/sql/postgres-bind-wire.test.ts:
(pass) named statements > null [79.26ms]
(pass) named statements > boolean as bool [23.18ms]
(pass) named statements > integer as int4 [20.74ms]
(pass) named statements > BigInt as int8 [24.29ms]
(pass) named statements > double as float8 [20.51ms]
(pass) named statements > Date as timestamptz [23.33ms]
(pass) named statements > AS
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     ec7e250da5
  features     lto, baseline

23 deps, 136 codegen, 1176 objects in 1214ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] fetch lolhtml
[lolhtml] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
244 units: 172 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[1/1493] install /workspace/bun
bun install v1.4.3-canary.1 (41e52b124)

Checked 26 installs across 65 packages (no changes) [42.00ms]
[2/1493] rustc unicode_ident 
[3/1493] rustc heck 
[4/1493] rustc build_script_build 
[5/1493] install /workspace/bun/src/node-fallbacks
bun install v1.4.3-canary.1 (41e52b124)

Checked 111 installs across 104 packages (no changes) [18.00ms]
[6/1493] build.rs build_script_build
[7/1493] rustc build_script_build 
[8/1493] gen generated_host_exports.rs
generated_host_exports.rs: 121 exports (host=5, lazy=10, generic=106, r
... (truncated)
diff hotspot 
src/sql_jsc/postgres/PostgresRequest.rs       |  70 ++++++-
 src/sql_jsc/postgres/PostgresSQLConnection.rs | 162 +++++++----------
 src/sql_jsc/postgres/PostgresSQLQuery.rs      |  23 ++-
 test/js/sql/postgres-bind-wire.test.ts        | 253 ++++++++++++++++++++++++++
 test/js/sql/wire-frames.test.ts               |  36 ++++
 test/js/sql/wire-frames.ts                    |  68 +++++++
 6 files changed, 497 insertions(+), 115 deletions(-)

gate history · 1 passed · 0 rejected · iteration 3

evidence per changed file 
file                                           reads  edits  tests
src/sql_jsc/postgres/PostgresRequest.rs            5      4     32
src/sql_jsc/postgres/PostgresSQLConnection.rs     10      1     30
src/sql_jsc/postgres/PostgresSQLQuery.rs           8      3     31
test/js/sql/postgres-bind-wire.test.ts             0      1     16
test/js/sql/wire-frames.test.ts                    0      0     21
test/js/sql/wire-frames.ts                         3      1     35