ReviewOS

also looking at this

stacks/bun-query-builder

fix: a multi-row insert names every column any row carries (#1189)

#1190
Merged glennmichael123 wants to merge fix/multi-row-insert-column-union into main
3 files +384 -11
8 days ago

Fixes #1189.

The bug

The column list came from Object.keys(rows[0]), and each row was then read through it as row[keys[c]]. A key appearing only in a later row had nowhere to go, so its value was dropped from the statement — from every row that carried it — with no error and the full row count reported.

Four builders had it independently, each hand-rolled from the same first row. I reproduced all four with the reporter's own rows before touching anything:

entry pointbeforeafter
insertInto().values([...])previous_text: null'SHOULD PERSIST'
createMany()previous_text: null'SHOULD PERSIST'
insertMany()previous_text: null'SHOULD PERSIST'
buildInsertClause() (upsert / insertOrIgnore / insertGetId / updateOrInsert)droppedkept

The single-row control stored it correctly throughout, which is what made the issue's "split it into two inserts and it works" observation the right diagnosis.

The fix

insertColumns() returns the union of every row's keys in first-appearance order, plus whether the batch is uniform.

Uniform is the common case and its fast paths are unchanged: no per-cell presence check, placeholders stay dense, the single-row shape cache is untouched. Only a heterogeneous batch takes the slower per-cell branch.

The gap is filled with DEFAULT rather than a bound NULL, so the column's own default still applies — NULL would override a non-null default, and a NOT NULL column with a default would reject the row outright. DEFAULT consumes no placeholder, which is why the heterogeneous branches advance their parameter index only on bound cells and trim the pre-allocated tail.

One dialect caveat, documented

SQLite has no DEFAULT inside a VALUES list — I checked against bun:sqlite, which answers near "DEFAULT": syntax error — so there it binds NULL and the column default is lost:

dialectgapeffect
Postgres, MySQLDEFAULTcolumn default applies
SQLiteNULLcolumn default bypassed

That's a real difference rather than a detail, so it's written up in docs/advanced/dialects.md with the two explicit ways out. It's still the caller's value reaching the right column, where before it reached nothing at all.

Verification

Proved on a live Postgres that the gap really takes the column default: a NOT NULL column with a default, set on the second row only, stores the default for the first row. A bound NULL would have failed the constraint, so that case distinguishes the two — it isn't inferable from the SQL text.

Mutation-checked. With the union disabled, 6 of the 8 new cases fail. The 2 that still pass are the deliberate controls: a uniform batch emitting byte-identical SQL, and an empty batch still emitting no INSERT (#1097).

Canonical bun run test: 2522 pass, 0 fail, 219 files. tsc clean. Lint 0 errors — the +4 warnings versus the previous run are duplicate headings in CHANGELOG.md from the v0.3.18 release, not from this change.

🤖 Generated with Claude Code

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.