Skip to content

fix(driver-sql): refuse a MySQL upsert whose named conflict target another unique key can absorb (#8755) - #8806

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-8755-mysql-upsert-second-unique-key
Aug 15, 2026
Merged

fix(driver-sql): refuse a MySQL upsert whose named conflict target another unique key can absorb (#8755)#8806
hotlong merged 2 commits into
mainfrom
claude/issue-8755-mysql-upsert-second-unique-key

Conversation

@hotlong

@hotlonghotlong commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Fixes#8755

Implements the maintainer ruling in comment 5299846509: option A (pre-flight refusal when a second unique key exists on the target table) plus option C's documentation half. Option B — probe-then-write / SELECT ... FOR UPDATE emulation — is not built and the comparison is not re-opened.

The condition, reproduced before it was fixed

Live MySQL 8.0.46 in this container, driven through the same knex + mysql2 path SqlDriver.upsert takes, both business columns unique: true so the named target email is backed and #8621's pre-flight passes it:

CREATE TABLE `probe_two_unique` (
`id` varchar(255) NOT NULL, `email` varchar(255) DEFAULT NULL,
`tax_id` varchar(255) DEFAULT NULL, `title` varchar(255) DEFAULT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `uniq_probe_two_unique_email` (`email`),
UNIQUE KEY `uniq_probe_two_unique_tax_id` (`tax_id`)
)
seed upsert({email:'a@b.com', tax_id:'T-1', title:'first'}, ['email']) -> RESOLVED
B upsert({email:'other@b.com', tax_id:'T-1', title:'second'}, ['email']) -> RESOLVED
rows = 1 [{email:'other@b.com', tax_id:'T-1', title:'second'}]
`email` did not collide; `tax_id` did, and MySQL merged on it.

One correction to the card's body, measured. It says the identical call on SQLite and Postgres "inserts a second row" — its own next clause says the opposite, and the measurement agrees with the next clause. Measured on SQLite through the same driver:

B upsert({email:'other@b.com', tax_id:'T-1', ...}, ['email'])
-> UNIQUE constraint failed: probe_two_unique.tax_id
rows = 1, still the seeded row, untouched.

The divergence is real and is exactly what the card is about — a silent wrong write on MySQL versus a legible error elsewhere — but the docs in this PR state the measured shape, not the "second row" one.

What changed

assertConflictTargetBacked is extended (renamed assertConflictTargetHonoured) rather than joined by a second introspection: same physicalKeyIndexes read, same cache, same invalidation, same stale-cache re-read before any refusal — now answering two questions off one index list instead of one. The call site, its MySQL-only gate and its position beforefillAutoNumberFields are unchanged, so a refusal still never burns an autonumber.

The refusal fires when, and only when: the caller named conflictKeys, the dialect is MySQL, the named target is backed, the named target is not the primary key, and a UNIQUE key exists whose columns differ from the target's.

The wording is its own, not a reuse of the unbacked refusal — #5240 is one condition, one wording, and this is a different condition; the unbacked sentence would tell an author to declare a unique: true they already declared. VALIDATION_ERROR / 400 rather than NOT_IMPLEMENTED / 501, by the #5907 classifier this file already applies twice: the refusal is conditional on the table (the same server merges the same statement fine once the table carries one unique key), not a capability gap in the backend. 400 also keeps the sentence on the wire, and the sentence is the deliverable the ruling asks for.

Cannot upsert into "os8621_wrong_key" on conflict keys ("email"): a UNIQUE key other than the
conflict target exists on "os8621_wrong_key" — uniq_os8621_wrong_key_tax_id(tax_id) — and this
backend is MySQL, whose only merge statement is ON DUPLICATE KEY UPDATE. That statement carries no
conflict target, so the merge lands on whichever UNIQUE key the row collides with FIRST: the
conflict target is backed, but a collision on uniq_os8621_wrong_key_tax_id(tax_id) would silently
merge a row the caller never targeted, across two different values of the named key. Fix by
dropping or renaming the extra UNIQUE key(s) so the conflict target is the only one on the table,
or by running this object on a dialect that honours the target — SQLite and PostgreSQL compile
ON CONFLICT (...), which merges on the named key alone. Upserting on the primary key is
unaffected: supply "id" and omit conflictKeys.

What is deliberately NOT refused

The PRIMARY KEY is never counted as a rival key, and a named primary key is never refused. Both are load-bearing, not oversights:

  • Every table this driver creates carries an id PRIMARY KEY, so counting it would refuse everyconflictKeys upsert on MySQL — the ruling's "the single-key fast path stays untouched" would describe nothing — and the remedy the message states ("drop or rename the extra key") is not available for a primary key.
  • upsert(o, row, ['id']) compiles byte-identically to the conflictKeys-less default that no pre-flight has ever probed, so refusing the explicit spelling while merging the implicit one would make the accept set a property of how the caller typed the same statement. It is also the only conflictKeys shape the platform itself issues (lifecycle-service.ts, the archiver's hot-to-cold copy), whose refusal would read "drop the unique constraint you declared on your own business column".

That residue is documented rather than silent — in the new docs section, in refuseAmbiguousConflictTarget's docblock, and as a pin. It is also filed as #8807, with its own measurement and options, so it is not lost.

Verification

Live MySQL 8.0.46 raised in the container (default_time_zone='+08:00', process TZ=America/New_York, so the D-B2 zone-skew guard is satisfied), plus the SQLite cell.

Both branches pinned as the ruling requires, on the live MySQL cell:

PinAssertion
two unique keys, named emailrefused, VALIDATION_ERROR / 400, nothing written
the messagecontains uniq_os8621_wrong_key_tax_id, tax_id, "email", the drop/rename workaround, the named alternative dialects, primary key is unaffected
payload contractno row values in message or cause; rival keys on cause
single unique key, named emailstill merges — the discriminating control
explicit ['id'], two unique keysstill merges (the documented residue)
#8622's identity pinmoved fixture to the default-path merge, assertion untouched

Reverse verification, direction predicted before running, both matched:

  1. Ablation — delete the ambiguity arm: predicted exactly the three new [#8755] refusal pins red and every control green. Measured: 3 failed | 24 passed, the three being precisely those pins.
  2. Ablation — count the PRIMARY KEY as a rival: predicted the refusal pins stay green while the merge pins go red. Measured: 4 failed | 23 passedMERGES when a declared unique index does back the conflict target, accepts the primary key as an explicit conflict target, the single-unique-key control, and the PK fast path. The asymmetry between the two ablations is the evidence that the narrowing is narrow rather than blanket.

Both ablations were run against a committed fix and restored with git checkout of the branch's own copy of the file; git status clean afterwards.

Verified at 967916b05 (the tree of the final commit — both the test run and the gate union below were taken at this sha):

pnpm --filter @objectstack/driver-sql typecheck -> clean
pnpm --filter @objectstack/driver-sql test -> 100 passed | 1 skipped (101 files)
1895 passed | 23 skipped [live MySQL cells ON]

Gate union, derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs and re-run at 967916b05 — all green: check:changeset-gate-self-tests, check:docs-audit-scope, check:objectui-changeset, check:role-word, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:nul-bytes, check:type-check-coverage, check:type-check-debt (re-measured on a built closure: 33 ledger entries, none above ceiling), check-adr-0087-registration, check-changeset-no-major, check-empty-changeset.

Scope

#8740 is not addressed here — the empty-merge-set fallback in the same function stays on hold, and this change does not make it easier or harder (the fallback is reached only when the merge set is empty, which is orthogonal to how many unique keys the table carries).


Generated by Claude Code

…other unique key can absorb (#8755)
`ON DUPLICATE KEY UPDATE` carries no conflict target, so on MySQL the merge lands
on whichever UNIQUE key the row collides with first -- including one the caller
never named, and including when the named target IS backed. #8621 closed the
unbacked half; this closes the rest.
Measured on live MySQL 8.0.46 through the same knex + mysql2 path `upsert` takes,
both business columns `unique: true`, caller naming `email`:
seed upsert({email:'a@b.com', tax_id:'T-1'}, ['email']) -> RESOLVED
B upsert({email:'other@b.com', tax_id:'T-1'}, ['email']) -> RESOLVED, ONE row
merged on `tax_id`, across two different values of the named key.
The identical call on SQLite raises `UNIQUE constraint failed: ….tax_id` and
leaves the seeded row untouched -- a legible error rather than a silent wrong
write. (Measured; the card's body says "inserts a second row", which its own next
clause contradicts and this measurement disproves.)
Per the maintainer ruling on #8755 this takes option A -- preflight refusal --
and rejects option B's probe-then-write emulation. The pre-flight extends #8621's
introspection rather than adding a second one: the same `physicalKeyIndexes`
read, the same cache and invalidation, the same stale-cache re-read before any
refusal, now answering two questions instead of one. `assertConflictTargetBacked`
is renamed `assertConflictTargetHonoured` because "backed" now describes half of
what it refuses.
The refusal gets its own wording, `code` and `status` are VALIDATION_ERROR / 400:
#5240 is one condition one wording, and this is a different condition from "no
index backs your target" -- reusing that sentence would tell an author to declare
a `unique: true` they already declared. Not 501: the #5907 classifier this file
applies twice sorts by what the caller must change, and this is conditional on
the TABLE (a single-unique-key table merges fine on the same server), not a
capability gap in the backend. 400 also keeps the message on the wire, and the
message is the deliverable -- the ruling requires it to name the colliding key
and the way out.
Two shapes are deliberately left merging and documented as the dialect's residue
rather than silently narrowed: the `conflictKeys`-less default (never probed by
any pre-flight), and an explicitly named PRIMARY KEY -- which compiles
byte-identically to that default, and is the only `conflictKeys` shape the
platform itself issues (the lifecycle archiver's hot->cold copy). Counting the
primary key as a rival would refuse every `conflictKeys` upsert on MySQL, since
every table this driver creates carries an `id` PRIMARY KEY, and its stated
remedy ("drop or rename the extra key") is not available for one.
Both branches pinned on the live MySQL cell, as the ruling requires: the
single-unique-key upsert still merges, and the two-unique-key upsert refuses with
the second key's name and both workarounds asserted as text. #8622's identity pin
moves fixture once more, to the default-path merge that still exhibits the
wrong-key merge it measures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
…iver differences live (#8755)
The other half of #8755's ruling, which folded in option C's documentation: the
dialect limit is now stated on the Database Drivers page beside the PostgreSQL,
MongoDB and SQLite sections, which had no MySQL section at all.
States what was measured rather than what was assumed: SQLite and PostgreSQL
compile `ON CONFLICT (email)`, so a collision on another unique key raises
`UNIQUE constraint failed: ...tax_id` and leaves the seeded row untouched; MySQL
compiles `ON DUPLICATE KEY UPDATE`, which carries no target at all. (The card's
body says the identical call "inserts a second row" on those two dialects -- its
own next clause says otherwise, and the measurement agrees with the next clause.)
Carries the accept-set table per call shape, both workarounds, and -- explicitly
rather than by omission -- the residue this card does not refuse: the
`conflictKeys`-less default and an explicitly named primary key still merge on
whichever unique key collides.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
@vercel

vercelBot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 15, 2026 3:36am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via @objectstack/driver-sql)
  • content/docs/getting-started/glossary.mdx(via @objectstack/driver-sql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/driver-sql)
  • content/docs/plugins/anatomy.mdx(via @objectstack/driver-sql)
  • content/docs/plugins/packages.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/driver-sql)
  • content/docs/protocol/objectql/query-syntax.mdx(via @objectstack/driver-sql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/driver-sql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

drivers(sql): on MySQL an upsert still merges on a unique key the caller never named — even when the named conflict target IS backed

2 participants

@hotlong@claude