Repository navigation
fix(driver-sql): upsert with nothing left to merge is a plain INSERT, not merge-ALL - #26015
Conversation
Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude <noreply@anthropic.com>
… not merge-ALL Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1ef0dc19bb6fe8b137ace72021d0678dff7ef30d && git checkout 1ef0dc19bb6fe8b137ace72021d0678dff7ef30d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 39a78b151d691a538856c0f0d57304f8faa69f96 b4fc36f35e265f6c25471504842838d88f58fe2a && git checkout -B drift-repro 39a78b151d691a538856c0f0d57304f8faa69f96 && git merge --no-ff b4fc36f35e265f6c25471504842838d88f58fe2a
node scripts/docs-audit/affected-docs.mjs --json 39a78b151d691a538856c0f0d57304f8faa69f96
|
…t to merge (red on head) Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude <noreply@anthropic.com>
…RT path, on every dialect Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Inputs: card #8740 (body and every comment, the seat's ruling B in the round-2 dev report included), PR #26015 body, file list and the net diff ① Derived judgmentsRead of the diff.
Check-runs on the head, as they stood when read: 54 runs; ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | draft, base main, Fixes #8740, Clause-②: no (narrowing) (body relay-patched to match the changeset), Metadata-protocol: none; four paths under packages/drivers/driver-sql/** plus one changeset. |
| Fix | an upsert with nothing left to merge no longer falls back to merging every column; it is a plain INSERT, so a collision raises the driver's unique violation. Ruling B: before that INSERT, the conflict target is checked against the table's cached physical key indexes on every dialect (the MySQL pre-flight's cover rule, hoisted into one shared helper), and an unbacked target is refused VALIDATION_ERROR / 400 on all three. |
| Review qualifier, recorded here | on a table with no primary or unique key at all, or when the key read fails, SQLite and PostgreSQL now run the plain INSERT where they used to refuse (MySQL always did). Nothing can collide there, so the seat accepts it; the PR body's "still refused" sentence carries this qualifier by this record, and #8740's landing repeats it. |
| Evidence | red first (the SQLite and PG cells inserted at the round-1 head), 122/122 upsert cells on all three dialects, package 3796 passed, typecheck 0, 71/71 derived gates. The live PG / MySQL 8 cells run push-only: Temporal Conformance is read on the merge commit before #8740 is reported closed. |
| Noted | the clause-less backing check lacks #8807's caller-named exemption; reachable only through a column map that remaps id. Not this card's. |
| Landing | 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready + auto-merge through the relay. |
Generated by Claude Code
Requested by Leehom · project thread
Fixes #8740
Clause-②: no (narrowing)
Metadata-protocol: none
What changed
SqlDriver.upsertmerges every payload column that is not insert-only, or, when there is none, the conflict-target columns the payload carries (a no-op). When the payload carried neither, the statement fell back to knex's baremerge(), which compiles to merge-ALL on every dialect (do update set/on duplicate key updateover every inserted column, insert-only ones included).That fallback is removed. With nothing to merge, the statement is a plain
INSERTwith no conflict clause, so a collision surfaces as the driver's own unique-violation error and the stored row is left as it was. It is notinsert ignore/do nothing, which would hide the collision. The reachable no-op merge on the conflict-target columns is unchanged.packages/drivers/driver-sql/src/upsert-conflict-clause.ts(new, not exported from the package):withUpsertConflictClause(insertion, mergeKeys, columnsToMerge)returns the plain insertion whencolumnsToMergeis empty, elseonConflict(mergeKeys).merge(columnsToMerge). Kept free of the driver so its SQL is pinned per dialect with no server.sql-driver.ts: the upsert statement goes through it; the in-statement merge predicate is applied only when the statement merges..changeset/8740-upsert-nothing-to-merge.md:patch, BREAKING banner (a served-set narrowing), ADR-0087not-required (no-migration-prescription).Tests
New
src/sql-driver-upsert-empty-merge-set.test.ts:toSQL()with no server, exact strings forbetter-sqlite3,pgandmysql2: noon conflict,do update set,on duplicate key updateorinsert ignore; plus the contrast (something to merge keeps the clause and merges only the named column);OS_TEST_POSTGRES_URL/OS_TEST_MYSQL_URLare provisioned): a collision on a conflict-target column the payload omits (non-null default) is refused with a unique violation and rewrites nothing; a primary-key collision whose payload carries only insert-only columns is refused and rewrites nothing; controls: a payload that collides with nothing still inserts and answers the row, and a payload carrying its conflict target still upserts onto the stored row. The refusal is asserted byisUniqueViolationErrorAND the dialect's native code (SQLITE_CONSTRAINT_*,23505,ER_DUP_ENTRY).Red first, on the unfixed driver (commit 19a401b): 4 failed / 12 passed, the non-null-default collision red on all three cells, the primary-key collision red on the MySQL-dialect cell. Green after the fix (commit fc2ee27), with the four neighbouring upsert suites: 5 files, 119 tests passed, all three cells live.
Local cells: SQLite in-process; PostgreSQL 16.15 local; the MySQL cell against MariaDB 10.11.14 (no MySQL 8 server in the container). CI's
Temporal Conformance (live PG + MySQL)job runs this package's whole suite, so the new file's MySQL cell runs there on MySQL 8.Gates
pnpm --filter @objectstack/driver-sql test: 236 files passed, 12 skipped; 3795 tests passed (SQLite cells; live cells via CI).typecheck: exit 0.05f7758f05show the same 49 failures (local MariaDB / PG environment, not this diff).eslint --no-inline-configon the three touched source files: exit 0.dispatch-gates --commands: 71 derived, 70 run, every exit 0; 1 NOT MEASURED:check:dual-build-cjs-loads(exit 3, needs the whole tree built; CI runs it).--ranreconciliation: 71 accounted, 0 unrun.Acceptance notes
isUniqueViolationErrorreads a non-unique error as a unique violation when the error message echoes anon duplicate key updatestatement (driver-level repro: a MySQLER_TRUNCATED_WRONG_VALUEwhose message carries the statement answerstrue; the same code without the statement text answersfalse). Not fixed here; the new tests pin the native code alongside it for that reason. No public door measured.driver-turso's remote transport is a separate statement builder (DO NOTHINGon an empty merge set, SQLite dialect only) and is untouched; its local face inherits this fix throughsuper.upsert.(narrowing)arm) is run on the final head and reported on drivers(sql):upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740.Generated by Claude Code