Skip to content

Commit d00d2f6

Browse files
hotlongclaude
andauthored
fix(driver-sql): refuse and roll back a MySQL upsert that merges onto a row the caller never identified (#8807) (#8940)
* wip(driver-sql): post-hoc identity check for the unnamed-key merge (#8807) * test(driver-sql): pin both branches per dialect for #8807 * docs+ledger(driver-sql): archiver measurement, MySQL dialect docs, ADR-0087 entry, changeset (#8807) * docs(driver-sql): retire the stale 'residue is only documented' claims (#8807) * fix(driver-sql): bind and tenant-scope the identity read; pin the scoping decision (#8807) * chore(spec): regenerate the migration registry after the origin/main merge (#8807) Both entries stack: #8936's engine-dotted-filter-refused and this card's driver-sql-upsert-cross-row-identity-merge-refused. 97 semantic entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ac6bc1d commit d00d2f6

7 files changed

Lines changed: 1063 additions & 68 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
fix(driver-sql): refuse — and roll back — a MySQL upsert that merges onto a row the caller never identified (#8807)
7+
8+
`ON DUPLICATE KEY UPDATE` carries no conflict target, so on MySQL a merge lands on
9+
whichever UNIQUE key the row collides with first. `#8621` closed the half where
10+
nothing backed a caller-named target; `#8755` closed the half where a rival key
11+
could absorb a caller-named one. This closes the residue those two left by
12+
construction: the `conflictKeys`-less call and the `['id']` call, which compile
13+
byte-identically and which no pre-flight can judge, because neither names anything.
14+
15+
Measured on live MySQL 8.0.46, `email` and `tax_id` both `unique: true`, **no**
16+
`conflictKeys`: seeding `{email:'d@b.com', tax_id:'T-9'}` inserted one row, and
17+
`{email:'e@b.com', tax_id:'T-9'}` then resolved with no error — one row, the
18+
*seeded* one, its `email` rewritten `d@b.com` to `e@b.com`, and the id the caller
19+
was handed back present in no row at all. The identical pair on SQLite raises
20+
`UNIQUE constraint failed: …tax_id` and leaves the seeded row untouched.
21+
22+
Per the maintainer ruling on #8807 this enforces a contract principle, not a MySQL
23+
detail: *an `upsert` must never modify a row whose identity the caller did not
24+
supply and whose conflict key it did not name.*
25+
26+
**Accept-set change, MySQL only.** After the statement and inside the same
27+
transaction, the driver checks whether the row it landed on is the one the call
28+
supplied. If it is not, the write is **rolled back** and the call refuses with
29+
`code: 'VALIDATION_ERROR'`, `status: 400`, naming the UNIQUE key that absorbed the
30+
merge and stating that nothing was changed.
31+
32+
The check is exact rather than heuristic — `id` is insert-only on the merge path
33+
(#8622), so a row merged on the primary key always still carries the supplied id
34+
and a row merged on any other key never does — which is why it has no false
35+
refusals.
36+
37+
Deliberately unchanged: tables whose only key is the primary key are not verified
38+
and open no transaction, so the ordinary upsert keeps its single round trip; every
39+
insert and every re-upsert of the same row still merges; the caller-named
40+
single-unique-key fast path is untouched; and SQLite and PostgreSQL are unaffected,
41+
because `ON CONFLICT (...)` already honours the named arbiter. The lifecycle
42+
archiver's hot→cold copy passes by construction — it supplies each row's own id —
43+
and of the two objects declaring `lifecycle.archive`, neither carries a
44+
non-primary unique field. The dialect limit is documented under
45+
*Database Drivers → MySQL*.

content/docs/data-modeling/drivers.mdx

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,8 @@ before any row is written and before any auto-number is reserved:
339339

340340
| Call, on MySQL | Result |
341341
| :--- | :--- |
342-
| `upsert(o, row)`no `conflictKeys` | Merges. Not pre-flighted (see the residue below). |
343-
| `upsert(o, row, ['id'])`the primary key | Merges. Compiles identically to the line above. |
342+
| `upsert(o, row)`no `conflictKeys` | Merges on the primary key. **Refused** if the merge lands on a different row (see below). |
343+
| `upsert(o, row, ['id'])`the primary key | Identical to the line abovesame statement, same answer. |
344344
| `upsert(o, row, ['email'])`, the table's only UNIQUE key being on `email` | Merges on `email`. **The common shape is unaffected.** |
345345
| `upsert(o, row, ['email'])`, the table also carrying `UNIQUE(tax_id)` | **Refused.** The message names `tax_id`'s index and the workarounds. |
346346
| `upsert(o, row, ['email'])`, no unique index on `email` at all | **Refused** on every dialect ([#8621](https://github.com/objectstack-ai/objectstack/issues/8621)). |
@@ -353,13 +353,43 @@ Two ways out, both stated in the error message:
353353
appropriate when both keys are genuine business constraints, since one of them
354354
must otherwise be given up.
355355

356+
### The merge that lands on a row you never identified
357+
358+
The pre-flight above covers a *caller-named* non-primary target. It cannot cover
359+
the `conflictKeys`-less default or an explicitly named primary key, because
360+
neither call names anything a pre-flight could checkand those two compile to
361+
the same statement. On a table with several UNIQUE keys that statement can still
362+
collide on a key you did not name, and MySQL will merge there:
363+
364+
```text
365+
seed upsert({ email: 'd@b.com', tax_id: 'T-9', title: 'first' }) -- no conflictKeys
366+
-> inserted, id = iVvD35rMk4BIayYc
367+
B upsert({ email: 'e@b.com', tax_id: 'T-9', title: 'second' }) -- no conflictKeys
368+
-> the fresh id did not collide; `tax_id` did, so MySQL merged onto the
369+
SEEDED row — rewriting an `email` this call never asked to touch.
370+
```
371+
372+
So the driver checks, after the statement and inside the same transaction,
373+
whether the row it landed on is the one the call supplied. If it is not, the
374+
write is **rolled back** and the call is refused with `code: 'VALIDATION_ERROR'`
375+
and `status: 400`. Nothing is left changed.
376+
377+
| Call, on MySQL, table carrying a rival UNIQUE key | Result |
378+
| :--- | :--- |
379+
| The row is new, or matches an existing row's `id` | **Merges**, exactly as before. |
380+
| The row collides on a UNIQUE key you did not name | **Refused**, and the write is rolled back. |
381+
356382
<Callout type="info">
357-
**The residue, stated rather than hidden.** The refusal covers a *caller-named*
358-
non-primary target. It does not cover the `conflictKeys`-less default or an
359-
explicitly named primary key: those two compile to the same statement, and on a
360-
table with several UNIQUE keys that statement can still merge on one you did not
361-
name. If you need the target honoured exactly, name itand on MySQL, keep one
362-
UNIQUE key per table.
383+
**This check is selective, and only MySQL pays for it.** A table whose only key
384+
is its primary key can never exhibit the condition, so nothing is verified and
385+
no transaction is opened there. SQLite and PostgreSQL compile
386+
`ON CONFLICT (id)`, which honours the arbiter and raises a unique violation on
387+
any other keythey already behave this way and are unchanged.
388+
389+
If you meant to merge on a business key, **name it** (`conflictKeys`), which
390+
makes the intent checkable. On MySQL, a table with more than one UNIQUE key
391+
cannot have every merge honouredkeep one UNIQUE key per table, or run the
392+
object on SQLite/PostgreSQL.
363393
</Callout>
364394

365395
## MongoDB

0 commit comments

Comments
 (0)