Skip to content

Commit c8806ae

Browse files
hotlongclaude
andauthored
fix(driver-sql): refuse an unbacked upsert conflict target on MySQL before compiling (#8761)
* fix(driver-sql): refuse an unbacked upsert conflict target on MySQL before compiling (#8621) SQLite and Postgres already refuse a `conflictKeys` upsert whose target no PRIMARY KEY or UNIQUE index backs: the server cannot infer an arbiter index and raises, and `isUnbackedConflictTargetError` turns that into a VALIDATION_ERROR / 400. On MySQL the same call resolved. knex compiles `onConflict(...).merge(...)` to `ON DUPLICATE KEY UPDATE`, which takes no conflict target at all, so the named keys are dropped before the statement leaves the process and the server is never asked to find an index for them. The reactive catch has nothing to catch. Measured on live MySQL 8.0.46 (`email` unnamed by any index, `tax_id` carrying the only unique key): the call merged on `tax_id`, across two different `email` values, and separately left two rows sharing the `email` the caller had asked to merge on. A wrong write, with no error. `upsert` now consults the table's physical PRIMARY KEY and UNIQUE indexes before compiling on MySQL and refuses with the wording, `code` and `status` the other two dialects already answer (#5240 -- one condition, one wording). It runs on MySQL alone by design: SQLite and Postgres refuse this from the server, with the server's own sentence attached as `cause`, and no pre-flight can reconstruct that. Only what can be PROVEN unbacked is refused -- a failed introspection, a table reporting no keys at all, and a possibly stale cache all proceed, the last after a fresh re-read. Not closed by this change, and filed as #8755: `ON DUPLICATE KEY UPDATE` carries no target even when the named one IS backed, so a second unique index can still absorb the conflict on MySQL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 * docs(changeset): driver-sql minor — MySQL upsert accept-set narrowing (#8621) 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 e783e16 commit c8806ae

3 files changed

Lines changed: 538 additions & 81 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
---
4+
5+
fix(driver-sql): MySQL refuses an upsert whose `conflictKeys` no PRIMARY KEY or UNIQUE index backs — calls that previously "resolved" now throw (#8621)
6+
7+
**This narrows MySQL's accept set.** A `SqlDriver.upsert(object, data, conflictKeys)`
8+
call on MySQL whose conflict target is backed by no PRIMARY KEY and no UNIQUE
9+
index used to resolve; it now throws `VALIDATION_ERROR` / 400. That is why this
10+
is a `minor` and not a patch: code that ran without error against MySQL will
11+
start failing, deliberately, and the rows it was writing were not the rows the
12+
caller asked for.
13+
14+
SQLite and Postgres have refused this exact call since #8445 / #8567, with this
15+
exact sentence. MySQL did not, and could not: knex compiles
16+
`onConflict([...]).merge(...)` on `mysql2` to `ON DUPLICATE KEY UPDATE`, which
17+
takes **no conflict target at all**, so the named keys are dropped before the
18+
statement leaves the process and the server is never asked to find an index for
19+
them. The existing refusal classifies an error the server raised, so on MySQL it
20+
had nothing to classify.
21+
22+
Measured on live MySQL 8.0.46 — `email` is the column the caller names, `tax_id`
23+
carries the only unique index:
24+
25+
```
26+
seed upsert({email:'a@b.com', tax_id:'T-1', title:'first'}, ['email']) -> resolved
27+
B upsert({email:'other@b.com', tax_id:'T-1', title:'second'}, ['email']) -> resolved
28+
ONE row: merged on `tax_id`, which the caller never named, across two
29+
different `email` values.
30+
D seed, then upsert({email:'a@b.com', tax_id:'T-2'}, ['email']) -> resolved
31+
TWO rows, both `email='a@b.com'`: the merge that WAS asked for did not
32+
happen either.
33+
```
34+
35+
So the failure being replaced is not an illegible error — it is a silent wrong
36+
write. `upsert` now consults the table's physical keys before compiling on MySQL
37+
and answers the wording, `code` and `status` the other two dialects already
38+
answer (#5240 — one condition, one wording).
39+
40+
**What this means for an existing MySQL deployment.** The calls that change are
41+
exactly those naming a conflict target no key covers — the same calls that have
42+
always been errors on SQLite and Postgres. The most likely one to surface is a
43+
tenant-scoped `unique: true` field: its index materializes as the composite
44+
`(COALESCE(organization_id, '__global__'), field)` (ADR-0120 D3), so
45+
`conflictKeys: ['field']` alone is not backed by it. The remedy is the one the
46+
refusal already prints: declare the column(s) `unique: true` and re-run schema
47+
sync, name the full composite, or upsert on the primary key.
48+
49+
Deliberately unchanged:
50+
51+
- **SQLite and Postgres.** They already refuse this from the server, and they
52+
attach the server's own sentence as `cause` — ground truth a pre-flight cannot
53+
reconstruct. Running the pre-flight there would replace a planner verdict with
54+
an introspection verdict for no gain.
55+
- **The default `['id']` path.** The pre-flight runs only when the caller names
56+
a target; the default is this driver's own primary key on every table it
57+
creates, so probing it would add a round trip to every ordinary upsert to
58+
answer a question with only one possible answer.
59+
- **Anything the pre-flight cannot prove.** A failed introspection, a table
60+
reporting no keys at all (indistinguishable from a table that does not exist),
61+
and a possibly stale cache all proceed rather than refuse — the cache is
62+
re-read from the database before any refusal is thrown.
63+
64+
**Not fixed here, and filed as #8755:** `ON DUPLICATE KEY UPDATE` carries no
65+
conflict target even when the named one IS backed, so on MySQL a second unique
66+
index can still absorb the conflict and merge on a key the caller never named.
67+
This change closes the unbacked-target hole; it does not make MySQL honour
68+
`conflictKeys` as a target.

0 commit comments

Comments
 (0)