Skip to content

postgres: decode binary int4[]/float4[] with NULLs and multiple dimensions - #33577

Closed
robobun wants to merge 2 commits into
mainfrom
farm/3a701987/pg-binary-array-null-multidim
Closed

robobun wants to merge 2 commits into
mainfrom
farm/3a701987/pg-binary-array-null-multidim

Conversation

@robobun

@robobun robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

What

A Bun.SQL (postgres) query over the extended protocol fails outright when an int4[]/float4[] column value contains a NULL or has more than one dimension:

array[1,NULL,3]::int4[]      extended = ERROR ERR_POSTGRES_NULLS_IN_ARRAY_NOT_SUPPORTED_YET
array[[1,2],[3,4]]::int4[]   extended = ERROR ERR_POSTGRES_MULTIDIMENSIONAL_ARRAY_NOT_SUPPORTED_YET
array[1,2,3]::int4[]         extended = Int32Array [1,2,3]

The simple/text protocol returns the same data fine ([1, null, 3], [[1,2],[3,4]]). Users don't choose the protocol: any parameterized query (tagged templates, sql.unsafe(q, params)) uses the extended protocol, so select ids from t where owner = ${x} works until one row's array picks up a NULL, then the whole query throws.

Cause

Bun requests the binary result format for int4[]/float4[] (Tag::is_binary_format_supported drives the Bind result-format codes), then the binary decoder from_bytes_typed_array in src/sql_jsc/postgres/DataCell.rs early-returned an error on contains_nulls != 0 and dimensions > 1. So Bun rejected a reply its own Bind request produced. postgres.js and node-postgres return [1, null, 3] on both paths.

Fix

A TypedArray can hold neither null holes nor nested arrays, so those two cases now decode into a plain JS array instead of throwing:

  • NULL elements (wire length prefix -1) become null.
  • Extra dimensions become nested arrays.

Plain 1-D, NULL-free arrays keep the fast Int32Array/Float32Array path, so the common case is unchanged. The new binary decoder validates the server-controlled dimension count and per-element lengths against the buffer before reading, matching the bounds checks already in the typed-array path.

Verification

New hermetic tests in test/js/sql/postgres-binary-array-bounds.test.ts drive a mock wire server that sends binary-format array columns:

  • int4[] / float4[] with a NULL decode to [1, null, 3] / [1.5, null, 3.5]
  • 2-D int4[] decodes to [[1,2],[3,4]]
  • 2-D int4[] with a NULL decodes to [[1,null],[3,4]]
  • existing well-formed int4[] still returns Int32Array([1,2,3])

The four new tests fail on the released binary with the two ERR_POSTGRES_* errors and pass with this change; the out-of-bounds rejection tests still pass.

…sions

The binary array decoder for int4[]/float4[] rejected any column whose
value contained a NULL (ERR_POSTGRES_NULLS_IN_ARRAY_NOT_SUPPORTED_YET) or
had more than one dimension (ERR_POSTGRES_MULTIDIMENSIONAL_ARRAY_NOT_SUPPORTED_YET).
Bun requests the binary result format for these types, so any
parameterized query (the extended protocol) hit this path and the whole
query failed when a row's array happened to contain a NULL, while the
simple/text protocol returned the data correctly.

A TypedArray cannot hold null holes or nested arrays, so those cases now
decode into a plain JS array: NULL elements (wire length prefix -1)
become null, and extra dimensions become nested arrays. Plain 1-D,
NULL-free arrays keep the fast Int32Array/Float32Array path.
@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1d89f4a7-b285-435f-8983-a5386f7cfb89

📥 Commits

Reviewing files that changed from the base of the PR and between a12475e and e870ea1.

📒 Files selected for processing (2)
  • src/sql_jsc/postgres/DataCell.rs
  • test/js/sql/postgres-binary-array-bounds.test.ts

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

@github-actions github-actions Bot added the claude label Jul 7, 2026
@robobun

robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:05 AM PT - Jul 7th, 2026

❌ @robobun, your commit e870ea1 has some failures in Build #69551 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 33577

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

bun-33577 --bun

@robobun

robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

The diff is green. Both CI runs so far have failed only on the :darwin: 26 aarch64 - test-bun lane with

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.

which is Buildkite artifact-download infra on that agent, not a test failure (the job never ran any tests). 257 jobs passed on build 69551, including every lane that ran test/js/sql/postgres-binary-array-bounds.test.ts. The one annotated flaky retry was test/bake/dev-and-prod.test.ts on Windows, unrelated to this change.

Ready for review; the darwin-26-aarch64 jobs can be retried individually from the Buildkite UI if needed.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-07, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant