diff --git a/.changeset/mysql-upsert-ambiguous-conflict-target.md b/.changeset/mysql-upsert-ambiguous-conflict-target.md new file mode 100644 index 0000000000..16149d5518 --- /dev/null +++ b/.changeset/mysql-upsert-ambiguous-conflict-target.md @@ -0,0 +1,29 @@ +--- +"@objectstack/driver-sql": minor +--- + +fix(driver-sql): refuse a MySQL upsert whose named conflict target another UNIQUE key can absorb (#8755) + +`ON DUPLICATE KEY UPDATE` — the only merge statement MySQL compiles — carries no +conflict target, so the merge lands on whichever UNIQUE key the row collides with +first. `#8621` closed the half where nothing backed the named target; this closes +the half where the target IS backed and a *second* UNIQUE key absorbs the +conflict instead. + +Measured on live MySQL 8.0.46, `email` and `tax_id` both `unique: true`, the +caller naming `email`: the second upsert merged on `tax_id`, across two different +values of the named key, leaving one row and no error. The identical call on +SQLite and PostgreSQL raises `UNIQUE constraint failed: …tax_id` and leaves the +seeded row untouched. + +**Accept-set change, MySQL only.** An `upsert(object, data, conflictKeys)` naming +a non-primary target on a table that carries any other UNIQUE key is now refused +before the statement is compiled — `code: 'VALIDATION_ERROR'`, `status: 400`, +nothing written and no auto-number reserved. The message names the colliding +index and both workarounds: drop or rename the extra UNIQUE key, or run the +object on a dialect that honours the target. + +Deliberately unchanged: a table whose only UNIQUE key IS the conflict target (the +common shape) merges exactly as before, as do the `conflictKeys`-less default and +an explicitly named primary key. The MySQL dialect limit and that residue are +documented under *Database Drivers → MySQL*. diff --git a/content/docs/data-modeling/drivers.mdx b/content/docs/data-modeling/drivers.mdx index d11aaf0520..996e72c66c 100644 --- a/content/docs/data-modeling/drivers.mdx +++ b/content/docs/data-modeling/drivers.mdx @@ -282,6 +282,86 @@ is moved into the client's URL slot (`connectionString` for pg, `uri` for mysql2) before reaching Knex. This affects only what Knex receives — the config you passed is preserved as-is on the driver. +## MySQL (via `@objectstack/driver-sql`) + +```bash +pnpm add @objectstack/driver-sql mysql2 +``` + +```typescript +import { SqlDriver } from '@objectstack/driver-sql'; + +new SqlDriver({ + client: 'mysql2', + connection: 'mysql://admin:secret@db.example.com:3306/myapp', +}); +``` + +Everything on this page's PostgreSQL section applies unchanged — the same +`SqlDriver`, the same connect-timeout defaults, the same tenant scoping. One +behaviour genuinely differs, and it is a limit of the dialect rather than of this +driver. + +### `upsert` conflict targets: the one dialect limit + + +On MySQL, `upsert(object, data, conflictKeys)` cannot promise that the merge +happens on `conflictKeys`. Where the table carries a UNIQUE key *outside* the +named target, the driver **refuses the call** rather than let it merge into a row +the caller never targeted +([#8755](https://github.com/objectstack-ai/objectstack/issues/8755)). + + +The same `upsert` call compiles differently per dialect, and only two of the +three can carry a conflict target at all: + +| Dialect | Compiles to | Honours the named target? | +| :--- | :--- | :--- | +| SQLite / PostgreSQL | `INSERT … ON CONFLICT (email) DO UPDATE …` | **Yes.** The named index is the arbiter. A collision on any *other* unique key raises a unique violation — a legible error. | +| MySQL | `INSERT … ON DUPLICATE KEY UPDATE …` | **No.** The statement carries no target at all, so the merge lands on whichever UNIQUE key the row collides with first. | + +Measured on MySQL 8.0.46, a table with `email` and `tax_id` both declared +`unique: true`, the caller naming `email`: + +```text +upsert({ email: 'a@b.com', tax_id: 'T-1', title: 'first' }, ['email']) -> seeded +upsert({ email: 'other@b.com', tax_id: 'T-1', title: 'second' }, ['email']) + -> ONE row. `email` did not collide; `tax_id` did, and MySQL merged on it — + rewriting a row whose `email` the caller never asked to touch. +``` + +The identical second call on SQLite and PostgreSQL fails with +`UNIQUE constraint failed: …tax_id` and leaves the seeded row untouched. + +So on MySQL the driver checks the table's physical keys *before* compiling, and +refuses what it cannot honour — with `code: 'VALIDATION_ERROR'` and `status: 400`, +before any row is written and before any auto-number is reserved: + +| Call, on MySQL | Result | +| :--- | :--- | +| `upsert(o, row)` — no `conflictKeys` | Merges. Not pre-flighted (see the residue below). | +| `upsert(o, row, ['id'])` — the primary key | Merges. Compiles identically to the line above. | +| `upsert(o, row, ['email'])`, the table's only UNIQUE key being on `email` | Merges on `email`. **The common shape is unaffected.** | +| `upsert(o, row, ['email'])`, the table also carrying `UNIQUE(tax_id)` | **Refused.** The message names `tax_id`'s index and the workarounds. | +| `upsert(o, row, ['email'])`, no unique index on `email` at all | **Refused** on every dialect ([#8621](https://github.com/objectstack-ai/objectstack/issues/8621)). | + +Two ways out, both stated in the error message: + +1. **Drop or rename the extra UNIQUE key** so the conflict target is the only one + on the table — appropriate when the second key was incidental. +2. **Run the object on a dialect that honours the target** (SQLite, PostgreSQL) — + appropriate when both keys are genuine business constraints, since one of them + must otherwise be given up. + + +**The residue, stated rather than hidden.** The refusal covers a *caller-named* +non-primary target. It does not cover the `conflictKeys`-less default or an +explicitly named primary key: those two compile to the same statement, and on a +table with several UNIQUE keys that statement can still merge on one you did not +name. If you need the target honoured exactly, name it — and on MySQL, keep one +UNIQUE key per table. + + ## MongoDB Configuration properties for the MongoDB driver. diff --git a/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts b/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts index 4a568935ba..f4311b3eb0 100644 --- a/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts @@ -58,8 +58,16 @@ * UNIQUE indexes before compiling, and answers the same sentence this sweep * asserts on the other two dialects. The last section of this file is that * refusal's pins — rewritten from #8592's characterization, as that card - * instructed, not relaxed. What remains un-refused there is the narrower #8755: - * `ON DUPLICATE KEY UPDATE` has no target even when the named one IS backed. + * instructed, not relaxed. + * + * ✅ **[#8755] has since closed the second half**, on the same introspection: + * `ON DUPLICATE KEY UPDATE` has no target even when the named one IS backed, so + * a second UNIQUE key on the table can absorb the conflict. That call is now + * refused too, with its OWN sentence (#5240 — one condition, one wording; this + * is a different condition from "no index backs your target" and every remedy it + * names is different). What is deliberately left merging, and documented rather + * than silent, is the PRIMARY-KEY-targeted call and the `conflictKeys`-less + * default — see {@link WRONG_KEY} and the residue pins below. * * ✅ **[#8622] has since repaired the primary-key half**, and only that half. * `id` is now insert-only on the merge path for every dialect, so the pin that @@ -553,6 +561,16 @@ describe('[#8567] MySQL: `onConflict().merge()` compiles the conflict target awa * backed and the pre-flight passes it; see #8755). Same claim, same strength, * same failure message; a fixture that can still exhibit the behaviour. * + * ⚠️ **[#8755] moved that same pin a SECOND time, one step further, and for the + * same reason.** #8755 refuses the two-unique-key call as well, so naming + * `email` on {@link WRONG_KEY} no longer merges either. The pin now runs on the + * `conflictKeys`-LESS default call against that table — the shape no pre-flight + * has ever probed, where MySQL still merges on whichever UNIQUE key collides. + * Measured on live MySQL 8.0.46 while implementing #8755, and it is the same + * phenomenon: one row, merged on `tax_id`, the stored `id` surviving. If a later + * card removes THAT wrong-key merge too, this pin moves again rather than being + * deleted — it is #8622's only MySQL-cell coverage of a landed fix. + * * # How this was measured * * #8567 left the MySQL half as an inference from compiled SQL — "merges on @@ -640,12 +658,19 @@ const MISMATCHED = { } as any; /** - * [#8621] Both business columns unique — so the named target `email` IS backed, - * the pre-flight passes the call, and MySQL merges it on whichever unique index - * the row actually collides with. This is the table where a wrong-key merge - * still happens after this card (#8755), which is why #8622's identity pin now - * runs here: that pin measures identity ACROSS a wrong-key merge, and needs a - * fixture that can still produce one. + * [#8621 → #8755] Both business columns unique. The named target `email` IS + * backed, so #8621's pre-flight passes the call — and MySQL then merges it on + * whichever unique index the row actually collides with, which is the whole of + * #8755. + * + * **This is now the REFUSAL fixture for #8755**: naming a non-primary target on + * this table is refused before compiling, because `uniq_os8621_wrong_key_tax_id` + * can absorb the conflict instead of the named `uniq_os8621_wrong_key_email`. + * + * It remains the wrong-key-MERGE fixture too, on the two shapes #8755 + * deliberately does not refuse and documents as the dialect's residue: the + * `conflictKeys`-less default, and an explicitly named PRIMARY KEY. That is + * where #8622's identity pin lives now. */ const WRONG_KEY = { name: 'os8621_wrong_key', @@ -656,6 +681,25 @@ const WRONG_KEY = { }, } as any; +/** + * [#8755] The DISCRIMINATING CONTROL the ruling names first: one unique key + * besides the primary, and the caller names exactly it. + * + * This is the common shape — a business object with one natural key — and it + * must keep merging, or the refusal is a blanket ban on `conflictKeys` upserts + * over MySQL rather than the narrow rule that was ruled. It is a table of its + * own rather than a reuse of {@link MISMATCHED} with `['tax_id']` (which has the + * same physical shape today) precisely so the control cannot be weakened by a + * later edit to a fixture that exists to be mismatched. + */ +const SINGLE_KEY = { + name: 'os8755_single_key', + fields: { + email: { type: 'string', unique: true }, + title: { type: 'string' }, + }, +} as any; + declareDialectCell( MYSQL_CELL, 'unbacked conflict-target refusal (pre-flight, MySQL)', @@ -677,12 +721,14 @@ function declareMysqlPreflightRefusal(cell: DialectCell): void { knexInstance = (driver as any).knex; await knexInstance.schema.dropTableIfExists(MISMATCHED.name); await knexInstance.schema.dropTableIfExists(WRONG_KEY.name); - await driver.initObjects([MISMATCHED, WRONG_KEY]); + await knexInstance.schema.dropTableIfExists(SINGLE_KEY.name); + await driver.initObjects([MISMATCHED, WRONG_KEY, SINGLE_KEY]); }); afterAll(async () => { await knexInstance?.schema.dropTableIfExists(MISMATCHED.name).catch(() => {}); await knexInstance?.schema.dropTableIfExists(WRONG_KEY.name).catch(() => {}); + await knexInstance?.schema.dropTableIfExists(SINGLE_KEY.name).catch(() => {}); await driver?.disconnect?.(); }); @@ -691,6 +737,7 @@ function declareMysqlPreflightRefusal(cell: DialectCell): void { beforeEach(async () => { await knexInstance(MISMATCHED.name).delete(); await knexInstance(WRONG_KEY.name).delete(); + await knexInstance(SINGLE_KEY.name).delete(); }); /** @@ -863,18 +910,23 @@ function declareMysqlPreflightRefusal(cell: DialectCell): void { * ⚠️ **[#8621] moved this pin's FIXTURE, and nothing else.** It measures * identity preservation ACROSS a merge on a key the caller never named, and * on {@link MISMATCHED} that call is now refused before it runs — the - * phenomenon is gone from that table, so the pin cannot live there. It runs - * on {@link WRONG_KEY} instead, where MySQL still merges on the wrong key - * (both columns unique, so the named target passes the pre-flight — #8755). - * The assertion, its strength and its failure message are untouched: - * deleting it, or weakening it to fit the refusal, would have dropped - * #8622's only MySQL-cell coverage of a landed fix. + * phenomenon is gone from that table, so the pin cannot live there. + * + * ⚠️ **[#8755] moved the fixture again, for the same reason and no other.** + * Naming `email` on {@link WRONG_KEY} is refused now too, so the call that + * used to exhibit the wrong-key merge here is gone as well. The surviving + * shape is the `conflictKeys`-LESS default: no pre-flight has ever probed it + * (the driver's own `['id']`), the minted id cannot collide, and MySQL + * merges on whichever UNIQUE key does — measured on live MySQL 8.0.46 while + * implementing #8755. The assertion, its strength and its failure message + * are untouched: deleting it, or weakening it to fit the refusal, would have + * dropped #8622's only MySQL-cell coverage of a landed fix. */ it('KEEPS the surviving row’s primary key, even merging on that wrong key', async () => { - await driver.upsert(WRONG_KEY.name, { email: 'a@b.com', tax_id: 'T-1', title: 'first' }, ['email']); + await driver.upsert(WRONG_KEY.name, { email: 'a@b.com', tax_id: 'T-1', title: 'first' }); const seededId = (await rows(WRONG_KEY.name))[0].id; - await driver.upsert(WRONG_KEY.name, { email: 'other@b.com', tax_id: 'T-1', title: 'second' }, ['email']); + await driver.upsert(WRONG_KEY.name, { email: 'other@b.com', tax_id: 'T-1', title: 'second' }); const merged = (await rows(WRONG_KEY.name))[0]; expect( @@ -892,35 +944,146 @@ function declareMysqlPreflightRefusal(cell: DialectCell): void { }); /** - * [#8755] The scope boundary, pinned so nobody reads #8621 as more than it - * is. `ON DUPLICATE KEY UPDATE` carries no conflict target even when the - * named one IS backed, so a second unique index can still absorb the - * conflict. The pre-flight does not refuse this — the target it was given is - * genuinely backed — and refusing it would mean refusing every - * `conflictKeys` upsert on any MySQL table with more than one unique index, - * an accept-set change far past this card's ruling. + * ✅ **[#8755] The condition #8621 left standing, now refused.** This was a + * characterization pin ("still merges on another unique key when the NAMED + * target is backed") whose failure message said that if it ever went red the + * pin — not the behaviour — was the thing to rewrite. That is this rewrite. + * + * `email` IS backed here, so #8621's arm passes the call. What refuses it is + * the second arm: `uniq_os8621_wrong_key_tax_id` is a UNIQUE key outside the + * named target, `ON DUPLICATE KEY UPDATE` carries no target, and MySQL would + * therefore merge on whichever of the two collided first — measured on live + * MySQL 8.0.46 before the fix as ONE row, merged on `tax_id`, across two + * different values of the key the caller named. * - * Pinned rather than left implicit because the alternative is a reader - * concluding from the pins above that MySQL now honours `conflictKeys` as a - * target. It does not, and this is where that stops being true. + * `code` AND `status`, never a bare `rejects.toThrow()`: an unrelated failure + * (a dead connection, an unknown column) would satisfy a bare throw while the + * accept set had not moved at all. */ - it('[#8755] still merges on another unique key when the NAMED target is backed', async () => { - await driver.upsert(WRONG_KEY.name, { email: 'a@b.com', tax_id: 'T-1', title: 'first' }, ['email']); - // `email` is unique here, so the pre-flight passes the call. The row that - // follows collides on `tax_id` — a different unique index — and MySQL - // merges on it, across two different values of the key the caller named. + it('[#8755] REFUSES the upsert when a second UNIQUE key can absorb the conflict', async () => { const err = await captureError(() => - driver.upsert(WRONG_KEY.name, { email: 'other@b.com', tax_id: 'T-1', title: 'second' }, ['email']), + driver.upsert(WRONG_KEY.name, { email: 'a@b.com', tax_id: 'T-1', title: 'first' }, ['email']), + ); + + expect( + err, + 'MySQL accepted a conflict target another UNIQUE key can absorb — the second arm of the ' + + 'pre-flight did not run (#8755)', + ).not.toBeNull(); + expect(err!.code).toBe(StandardErrorCode.enum.VALIDATION_ERROR); + expect(err!.status).toBe(400); + expect( + await rows(WRONG_KEY.name), + 'the refused upsert still wrote — the second arm is running after the statement, not before it', + ).toHaveLength(0); + }); + + /** + * [#8755] The ruling requires the message to NAME the colliding key and to + * state the way out. Asserted as text because an untested message drifts + * into uselessness — and because the whole reason A was ruled over B is that + * a refusal an author can read beats a merge they cannot see. + */ + it('[#8755] names the second UNIQUE key and both workarounds in the message', async () => { + const err = await captureError(() => + driver.upsert(WRONG_KEY.name, { email: 'a@b.com', tax_id: 'T-1', title: 'first' }, ['email']), + ); + + // The colliding key, by the name an operator will find in SHOW INDEXES — + // and its column, since the name alone is not actionable on a table whose + // indexes were created by hand. + expect(err!.message).toContain('uniq_os8621_wrong_key_tax_id'); + expect(err!.message).toContain('tax_id'); + // The named target, so the sentence says which call is being refused. + expect(err!.message).toContain('"email"'); + expect(err!.message).toContain(WRONG_KEY.name); + // Workaround ①: drop or rename the extra key. Workaround ②: a dialect + // without the limitation, named rather than alluded to. + expect(err!.message).toMatch(/drop(ping)? or renam/i); + expect(err!.message).toMatch(/SQLite and PostgreSQL/); + expect(err!.message).toMatch(/ON DUPLICATE KEY UPDATE/); + // And the primary-key path, which this refusal deliberately leaves open. + expect(err!.message).toMatch(/primary key is unaffected/i); + }); + + /** + * [#8755] The payload contract, on the new arm: schema identifiers are the + * ground truth an operator acts on, row values are not. Same claim #8621's + * `cause` pin makes for the unbacked arm, asserted separately because this + * arm builds a different `cause`. + */ + it('[#8755] keeps row values out of the refusal, and puts the rival keys on `cause`', async () => { + const err = await captureError(() => + driver.upsert( + WRONG_KEY.name, + { email: 'leaked@example.com', tax_id: 'T-9', title: 'secret-title' }, + ['email'], + ), + ); + + expect(err!.message).not.toContain('leaked@example.com'); + expect(err!.message).not.toContain('secret-title'); + expect(err!.message).not.toMatch(/insert into/i); + + const causeText = String((err!.cause as Error | undefined)?.message); + expect(causeText).toContain('uniq_os8621_wrong_key_tax_id'); + expect(causeText).not.toContain('leaked@example.com'); + expect(causeText).not.toContain('secret-title'); + }); + + /** + * ✅ **[#8755] THE discriminating control.** One unique key besides the + * primary, named by the caller: the common shape, and it must still merge. + * + * Without this case every pin above is satisfied by a pre-flight that + * refuses every `conflictKeys` upsert on MySQL — which is option C + * un-narrowed, the accept-set change the ruling explicitly did not make + * ("the single-key fast path stays untouched"). Narrowness is the entire + * reason A was ruled over C, so it is pinned rather than argued. + */ + it('[#8755] single-unique-key upsert still MERGES — the fast path is untouched', async () => { + await driver.upsert(SINGLE_KEY.name, { email: 'one@b.com', title: 'first' }, ['email']); + const err = await captureError(() => + driver.upsert(SINGLE_KEY.name, { email: 'one@b.com', title: 'second' }, ['email']), ); - expect(err, 'a backed conflict target must not be refused').toBeNull(); - const after = await rows(WRONG_KEY.name); expect( - after, - 'two rows would mean MySQL had started honouring the named target — good news, but it ' + - 'would mean #8755 was fixed and this pin is the one to rewrite', - ).toHaveLength(1); - expect(after[0].email).toBe('other@b.com'); + err, + 'a table whose only UNIQUE key IS the conflict target must never be refused — this is the ' + + 'shape the ruling protects, and refusing it turns A into a blanket ban', + ).toBeNull(); + + const after = await rows(SINGLE_KEY.name); + expect(after).toHaveLength(1); + expect(after[0].title).toBe('second'); + expect(after[0].email).toBe('one@b.com'); + }); + + /** + * [#8755] The residue this card deliberately does NOT refuse, pinned so it is + * documented behaviour rather than an accident nobody measured: an + * explicitly named PRIMARY KEY on a table that also carries UNIQUE keys. + * + * The reasoning is on `refuseAmbiguousConflictTarget` in `sql-driver.ts`. In + * one line: this call 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 — and the only + * `conflictKeys` the platform itself issues is exactly this one (the + * lifecycle archiver's hot→cold copy). + */ + it('[#8755] leaves an explicitly named PRIMARY KEY merging, UNIQUE keys or not', async () => { + const err = await captureError(() => + driver.upsert(WRONG_KEY.name, { id: 'os8755_pk', email: 'pk@b.com', tax_id: 'T-4', title: 'first' }, ['id']), + ); + expect(err, 'the primary-key fast path must not be refused').toBeNull(); + + await driver.upsert(WRONG_KEY.name, { id: 'os8755_pk', email: 'pk@b.com', tax_id: 'T-4', title: 'second' }, ['id']); + + const after = await rows(WRONG_KEY.name); + expect(after).toHaveLength(1); + expect(after[0].id).toBe('os8755_pk'); + expect(after[0].title).toBe('second'); }); /** diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index fcd0dde74c..6d9b681744 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -1068,6 +1068,104 @@ function refuseUnbackedConflictTarget(object: string, mergeKeys: string[], cause return err; } +/** + * [#8755] A `conflictKeys` upsert whose target IS backed, on a MySQL table that + * carries another UNIQUE key — the one that key can absorb the conflict instead. + * + * # A different condition from the one above, so a different sentence + * + * #5240 is "one condition, one wording", not "one function, one wording". + * {@link refuseUnbackedConflictTarget} answers *no index backs your target*, and + * every remedy it names is about creating that index. Here the index exists and + * the target is perfectly well formed; what cannot be honoured is the TARGETING, + * because `ON DUPLICATE KEY UPDATE` carries no target and MySQL merges on + * whichever unique key the row collides with first. Reusing the unbacked wording + * would tell an author to declare a `unique: true` they already declared, and + * send them to re-run a schema sync that would change nothing. + * + * Measured on live MySQL 8.0.46, `email` and `tax_id` both `unique: true`, the + * caller naming `email` — reproduced for this card through the same knex + + * `mysql2` path `upsert` takes: + * + * ``` + * upsert({email:'a@b.com', tax_id:'T-1', title:'first'}, ['email']) -> seeded + * upsert({email:'other@b.com', tax_id:'T-1', title:'second'}, ['email']) + * -> RESOLVED, ONE row: merged on `tax_id`, across two different `email` values. + * ``` + * + * # Why `VALIDATION_ERROR` / 400 and not `NOT_IMPLEMENTED` / 501 + * + * The #5907 classifier this file already applies twice + * ({@link uncompilableAggregateFunctionError}, {@link refuseDateBucketedGroupBy}) + * sorts a refusal by *what the caller would have to change*: a CAPABILITY gap in + * the backend — the request is spelled correctly and no schema anywhere makes it + * work — is 501, and a request that does not validate against the target it was + * given is 400. This one is the second: it is conditional on the TABLE, not on + * the dialect. The identical statement against the identical MySQL server is + * honoured the moment the table carries a single unique key, which is why the + * remedy in the message is a schema change and not "wait for the backend to + * implement it". 400 also keeps it off the retry-inviting 5xx band (nothing here + * is transient — the next attempt fails identically) and keeps the sentence + * itself on the wire: `@objectstack/rest` withholds the message body of any 5xx, + * and this message is the deliverable — the ruling requires it to name the + * colliding key and the way out. + * + * # What is NOT refused, and why the PRIMARY KEY never counts as a rival + * + * The primary key is excluded from the keys that trigger this refusal, in both + * directions, and neither is an oversight: + * + * - **As a rival** — every table this driver creates carries an `id` PRIMARY + * KEY, so counting it would refuse *every* `conflictKeys` upsert on MySQL. + * The ruling's "the single-key fast path stays untouched" would then describe + * nothing, and the remedy this message states ("drop or rename the extra + * key") is not available for a primary key — a refusal whose only stated way + * out is impossible is worse than no refusal. + * - **As the named target** — `upsert(…, ['id'])` is the driver's own identity + * path, byte-identical in compilation to the default `conflictKeys`-less + * call, which this pre-flight deliberately never probes. 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 (the lifecycle archiver's + * hot→cold copy), whose remedy would read "drop the unique constraint you + * declared on your own business column". + * + * Both residues are real and are documented as the dialect limit in + * `content/docs/data-modeling/drivers.mdx` rather than left for a reader to + * discover: on MySQL a merge can still land on an unnamed unique key whenever no + * target was named at all, or when the named target is the primary key. + */ +function refuseAmbiguousConflictTarget( + object: string, + mergeKeys: string[], + tableName: string, + rivals: PhysicalIndex[], +): Error { + const keys = mergeKeys.map((k) => `"${k}"`).join(', '); + const named = rivals.map((i) => `${i.name}(${i.columns.join(', ')})`).join(', '); + const err = new Error( + `Cannot upsert into "${object}" on conflict keys (${keys}): a UNIQUE key other than the ` + + `conflict target exists on "${tableName}" — ${named} — 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 ${named} 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.`, + ) as Error & { code?: string; status?: number; cause?: unknown }; + err.code = StandardErrorCode.enum.VALIDATION_ERROR; + err.status = 400; + // The introspected keys, exactly as the unbacked refusal attaches them: schema + // identifiers are the ground truth an operator acts on, and row values are not. + err.cause = new Error( + `conflict target (${mergeKeys.join(', ')}) on "${tableName}" is backed, but these UNIQUE keys ` + + `can absorb the conflict instead: ${named}`, + ); + return err; +} + /** * [#5158] A `FilterArray` reached the driver unlowered. * @@ -3375,7 +3473,7 @@ export class SqlDriver implements IDataDriver { /** * [#8621] PHYSICAL key indexes per table (tableName → the PRIMARY KEY and - * UNIQUE indexes that actually exist), for {@link assertConflictTargetBacked}. + * UNIQUE indexes that actually exist), for {@link assertConflictTargetHonoured}. * * Deliberately not `managedObjectIndexes`, which records what metadata * DECLARES. The pre-flight answers "can this conflict target resolve to a key @@ -5060,9 +5158,14 @@ export class SqlDriver implements IDataDriver { } /** - * [#8621] Refuse an upsert whose `conflictKeys` no PRIMARY KEY or UNIQUE index - * backs — BEFORE the statement is compiled, on the dialect where the server - * will never say so itself. + * Refuse an upsert whose named `conflictKeys` this dialect will not honour as + * the merge target — BEFORE the statement is compiled, on the dialect where + * the server will never say so itself. Two conditions, one introspection: + * + * - **[#8621] unbacked** — no PRIMARY KEY or UNIQUE index covers the named + * target, so there is no merge target at all; + * - **[#8755] ambiguous** — the target IS covered, but another UNIQUE key on + * the table can absorb the conflict instead of it. * * # Why a pre-flight exists at all, when a refusal already did * @@ -5108,13 +5211,16 @@ export class SqlDriver implements IDataDriver { * "the compiler drops the conflict target, so the server can never be asked", * and MySQL is the dialect that meets it. * - * # Why it refuses only what it can PROVE is unbacked + * # Why it refuses only what it can PROVE * * A false refusal breaks a working merge, which is the expensive direction — * the same asymmetry `isUnbackedConflictTargetError` records. So every * uncertain answer proceeds: a failed introspection, a table with no keys at * all (see {@link introspectKeyIndexes}), and a possibly stale cache, which is - * re-read from the database before any refusal is thrown. + * re-read from the database before any refusal is thrown. The stale-cache leg + * covers BOTH verdicts and in opposite directions — an index created since the + * read would make a backed target read as unbacked, and one DROPPED since the + * read would make a rival key exist that no longer does. * * The comparison is against the conflict target as EMITTED. `onConflict()` * receives `mergeKeys` verbatim — the write column map is applied to the row, @@ -5124,11 +5230,12 @@ export class SqlDriver implements IDataDriver { * dialects infer an arbiter index, and the only reading under which a * composite key means anything). * - * ⚠️ **This closes the unbacked-target hole, and not the whole MySQL gap.** + * # [#8755] The SECOND condition this pre-flight answers, added later + * * `ON DUPLICATE KEY UPDATE` carries no target even when the target IS backed, * so a table with a second unique index can still merge on a key the caller - * never named. Measured here on live MySQL 8.0.46, both columns unique, - * caller naming `email`: + * never named. Measured on live MySQL 8.0.46, both columns unique, caller + * naming `email` — re-measured for #8755 before it was fixed: * * ``` * upsert({email:'a@b.com', tax_id:'T-1', title:'first'}, ['email']) -> seeded @@ -5136,29 +5243,71 @@ export class SqlDriver implements IDataDriver { * -> ONE row, merged on `tax_id`. The named target was backed the whole time. * ``` * - * That is a different condition with a different fix and is filed separately - * (#8755); it is deliberately NOT smuggled in here, because refusing it would - * mean refusing every `conflictKeys` upsert on any MySQL table carrying more - * than one unique index — an accept-set change far past what this card rules. - */ - protected async assertConflictTargetBacked(object: string, mergeKeys: string[]): Promise { + * #8621 deliberately left that standing — it is a different condition, and + * refusing it narrows the accept set past what that card ruled. #8755's + * maintainer ruling then took it, choosing refusal (its option A) over + * emulating a target-honouring statement on MySQL (its option B, rejected: + * a SELECT-then-branch with a race window and an execution path unlike every + * other dialect — *"dressing 'the dialect can't' up as 'it did'"*). + * + * The two conditions share this method because they share the QUESTION — + * "what keys does this table physically carry?" — asked once, cached once + * ({@link physicalKeyIndexes}), invalidated in one place. They do NOT share an + * answer: see {@link refuseAmbiguousConflictTarget} for why the second gets + * its own sentence, and for why the PRIMARY KEY is never a rival key. + * + * ⚠️ **Renamed from `assertConflictTargetBacked` when the second condition + * landed.** "Backed" was the whole question while #8621 was the whole method; + * it is now one of two, and a name that describes half of what a guard refuses + * is how the other half gets deleted by someone tidying up. + */ + protected async assertConflictTargetHonoured(object: string, mergeKeys: string[]): Promise { // The table the statement will actually hit — same resolution `getBuilder` // performs for the insert, rotation shard included. const target = this.rotationWriteTarget(object) ?? object; const tableName = this.physicalTableByObject[target] ?? target; const wanted = new Set(mergeKeys); - const backs = (keys: PhysicalIndex[]): boolean => - keys.some((i) => i.columns.length === wanted.size && i.columns.every((c) => wanted.has(c))); + const covers = (i: PhysicalIndex): boolean => + i.columns.length === wanted.size && i.columns.every((c) => wanted.has(c)); + + type Verdict = + | { kind: 'honoured' } + | { kind: 'unbacked' } + | { kind: 'ambiguous'; rivals: PhysicalIndex[] }; + + const judge = (keys: PhysicalIndex[]): Verdict => { + if (!keys.some(covers)) return { kind: 'unbacked' }; + // [#8755] The named target is the PRIMARY KEY: the driver's own identity + // path, and the one shape whose refusal would differ from the + // byte-identical default call this pre-flight never probes. Left to merge + // — the residue is documented, not silent (see the refusal's docblock). + if (keys.some((i) => i.primary === true && covers(i))) return { kind: 'honoured' }; + // A UNIQUE key that is neither the named target nor the primary key can + // absorb the conflict instead of it. An index over the SAME columns as the + // target is not a rival: colliding on it is colliding on the target. + const rivals = keys.filter((i) => i.primary !== true && !covers(i)); + return rivals.length > 0 ? { kind: 'ambiguous', rivals } : { kind: 'honoured' }; + }; let keys = await this.introspectKeyIndexes(tableName); - if (keys !== null && keys.length > 0 && !backs(keys)) { + if (keys !== null && keys.length > 0 && judge(keys).kind !== 'honoured') { // A cache filled before the index was created is the only way a real key - // can be missing here, and it is cheaper to re-read once on the refusing - // path than to risk refusing a call the database would have merged. + // can be missing here, and a cache filled before one was DROPPED is the + // only way a rival key can be reported that no longer exists. Both are + // cheaper to re-read once on the refusing path than to risk refusing a + // call the database would have merged — the same asymmetry, now covering + // both verdicts. keys = await this.introspectKeyIndexes(tableName, { fresh: true }); } - if (keys === null || keys.length === 0 || backs(keys)) return; + if (keys === null || keys.length === 0) return; + + const verdict = judge(keys); + if (verdict.kind === 'honoured') return; + + if (verdict.kind === 'ambiguous') { + throw refuseAmbiguousConflictTarget(object, mergeKeys, tableName, verdict.rivals); + } // The introspected keys stand where the server's sentence stands on the // other two dialects: the caller-visible message is the shared wording, and @@ -5191,8 +5340,8 @@ export class SqlDriver implements IDataDriver { const mergeKeys = conflictKeys && conflictKeys.length > 0 ? conflictKeys : ['id']; - // [#8621] Pre-flight the conflict target — see - // {@link assertConflictTargetBacked} for the mechanism and why it is + // [#8621, #8755] Pre-flight the conflict target — see + // {@link assertConflictTargetHonoured} for the mechanism and why it is // MySQL-only. Two placement facts, both load-bearing: // // - It runs only when the CALLER named a target. The default `['id']` is @@ -5206,7 +5355,7 @@ export class SqlDriver implements IDataDriver { // reservation for a statement that never executes — a visible gap in an // externally meaningful sequence, handed out for a rejected call. if (conflictKeys && conflictKeys.length > 0 && this.isMysql) { - await this.assertConflictTargetBacked(object, mergeKeys); + await this.assertConflictTargetHonoured(object, mergeKeys); } // #6943. Measured: `upsert` does NOT share `bulkCreate`'s shape. It is