SIGN IN SIGN UP
oven-sh / bun UNCLAIMED

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one

0 0 150 Rust

sql: keep all-digit column names with '_' separators as named keys (#31572)

## What does this PR do?

Fixes two related issues in the SQL object-building path, both surfacing
as a debug abort on `ASSERT(cell.index < count)` at
`src/jsc/bindings/SQLClient.cpp:353`.

**1. Regression: digit-with-underscore column names misclassified as
indices.** A result column whose name is all digits with an interior
underscore (e.g. `2024_01`) was treated as a positional array index
instead of a named key.

Reproduced against a real MySQL/MariaDB:

| Binary | `SELECT product, \`2024_01\`, \`2024_02\`` → row |
| --- | --- |
| **1.3.14** (release, Zig) | `{ product: "widget", "2024_01": 10,
"2024_02": 20 }` ✅ |
| **canary** (Rust) | `{ "202401": 10, "202402": 20, product: "widget"
}` ❌ |

So `2024_01`/`2024_02` collapse to `202401`/`202402`, and the named keys
disappear. This is a regression: at `bun-v1.3.14` the code was Zig and
classified `2024_01` as a name; the `.rs` port landed after 1.3.14 (in
the Rust rewrite) and changed the behavior.

**2. Latent bug: the assertion itself is wrong.** `cell.index` for an
indexed column holds the column name parsed as an integer, not a
positional ordinal, so it can legitimately be `>= count`. The
indexed-only fast path 16 lines above documents exactly this (`//
cell.index can be > count`, e.g. `select 1 as "8"`), but the mixed
named+indexed slow path asserted `cell.index < count` and aborted in
debug builds — reachable with **correctly**-classified input, e.g.
`SELECT product, 42 AS \`8\`` (`Name` + `Index(8)`, count=2 → `8 < 2`).

### Cause

**(1)** `ColumnIdentifier::init` decided Name-vs-Index by parsing the
name with `bun_core::parse_unsigned`, which ports `std.fmt.parseInt` and
**skips embedded `_` separators** (`parse_with_sign` in
`src/bun_core/fmt.rs` does `if c == b'_' { continue; }`). So `"2024_01"`
parsed to `202401`. The original Zig `ColumnIdentifier.init` never used
`std.fmt.parseInt` — it hand-parsed a byte loop accepting only
`'0'..'9'` and broke to the "name" branch on any other byte. The Rust
port lost that.

**(2)** Pre-existing since #16512 (Jan 2025), untouched by the rewrite.

### Fix

**(1)** Replace the `parse_unsigned` call with the exact Zig hand-loop:
accept only `'0'..'9'`, treat any other byte as a name. `parse_unsigned`
itself is untouched (its `_` handling is correct for other callers —
semver, content-length). The `< u32::MAX` JSC bound is preserved.
`2024_01` stays a name (matches 1.3.14); pure-digit names like `5`/`123`
still index.

**(2)** Remove the `ASSERT(cell.index < count)` from the indexed branch
of the slow path so it matches the fast path's documented
`putDirectIndex` sparse-index handling. The named-branch assertion at
:363 is left intact (named cells set `index` to the positional ordinal,
always `< count`).

The classifier is shared by the Postgres and MySQL decoders, so (1)
covers both.

## How did you verify your code works?

`test/js/sql/sql-mysql-column-name-digits.test.ts` runs against a
**real** MySQL/MariaDB (not a mock): the docker-compose `mysql_plain`
service in CI (`describeWithContainer`), and the native server
otherwise. The fixture does a real `CREATE TEMPORARY TABLE` with
backtick-quoted `` `2024_01` ``/`` `2024_02` `` (digits+underscore) and
`` `8` `` (pure digit, value > column count) columns, inserts a row,
selects it back, and asserts the decoded row keeps the named keys and
round-trips everything.

- **Canary vs this patch**, same test file, same real MariaDB, only the
binary differing:
- published canary → **fails** (`2024_01`/`2024_02` decode as
`202401`/`202402`).
  - this patch → **passes** (`{ product, "2024_01", "2024_02", "8" }`).
- **ASAN (debug) fail/pass:** without the `src/` changes the fixture
aborts on `ASSERT(cell.index < count)`; with them it passes.
- 1.3.14-vs-canary behavior confirmed against the official release
binaries (table above). Leading-zero names (`"01"` → index `1`) are
invariant across 1.3.14 / pre-PR / this branch, so they're intentionally
left as-is to preserve parity (tracked separately).

Supersedes #31516.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
R
robobun committed
cb4a5c69db1c6794634306daeffd336cb533167c
Parent: 051f782
Committed by GitHub <noreply@github.com> on 5/29/2026, 5:49:08 PM