From 19a401bd3aaa90fc96440e5bd6f15bedcb1d60bf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 08:27:26 +0000 Subject: [PATCH 1/5] test(driver-sql): pin upsert with nothing left to merge (red on main) Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude --- .../sql-driver-upsert-empty-merge-set.test.ts | 231 ++++++++++++++++++ .../driver-sql/src/upsert-conflict-clause.ts | 31 +++ 2 files changed, 262 insertions(+) create mode 100644 packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts create mode 100644 packages/drivers/driver-sql/src/upsert-conflict-clause.ts diff --git a/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts b/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts new file mode 100644 index 00000000000..db6717761ea --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts @@ -0,0 +1,231 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * An upsert with nothing left to merge is a plain INSERT. + * + * `upsert`'s merge leg writes every payload column that is not insert-only + * (`insertOnlyUpsertColumns`), or, when that set is empty, the conflict-target + * columns the payload carries (a no-op: a conflict means they matched). When + * both are empty the statement used to fall back to knex's bare `merge()`, + * which compiles to merge-ALL on every dialect: `do update set` / + * `on duplicate key update` over EVERY inserted column, insert-only ones + * included. That fallback is gone. With nothing to merge the statement carries + * no conflict clause, so a collision surfaces as the driver's own + * unique-violation error and the stored row is left as it was. + * + * Pinned here: + * - the compiled SQL per dialect, with no server (`toSQL()`); + * - on every provisioned cell: a collision on a conflict-target column that + * the payload omits (the column has a non-null default) is refused and + * rewrites nothing; a collision on the primary key with only insert-only + * columns in the payload is refused and rewrites nothing; + * - controls: a payload that collides with nothing still inserts and answers + * the row, and a payload that carries its conflict target still upserts + * onto the stored row. + * + * The tables are created by hand and registered DDL-free, so the driver has not + * observed an `updated_at` column on them and the payloads below reach the + * empty merge set. Live cells run where their URL is provisioned; elsewhere they + * are declared un-run, never silently passed. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest'; +import knex from 'knex'; +import { isUniqueViolationError } from '@objectstack/types'; +import { SqlDriver } from '../src/index.js'; +import { withUpsertConflictClause } from './upsert-conflict-clause.js'; +import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js'; + +// ───────────────────────────────────────────────────────────────────────── +// The compiled statement, per dialect — no server +// ───────────────────────────────────────────────────────────────────────── + +describe('SqlDriver.upsert — the statement compiled when nothing is left to merge', () => { + const compile = (client: string, columnsToMerge: string[]): string => { + const k = knex({ client, connection: {} } as any); + try { + return withUpsertConflictClause(k('os8740_t').insert({ id: '1', created_at: 'x' }), ['email'], columnsToMerge) + .statement.toSQL().sql; + } finally { + void k.destroy(); + } + }; + + const EXPECTED_PLAIN: Record = { + 'better-sqlite3': 'insert into `os8740_t` (`created_at`, `id`) values (?, ?)', + pg: 'insert into "os8740_t" ("created_at", "id") values (?, ?)', + mysql2: 'insert into `os8740_t` (`created_at`, `id`) values (?, ?)', + }; + + for (const [client, expected] of Object.entries(EXPECTED_PLAIN)) { + it(`compiles a plain INSERT with no conflict clause (${client})`, () => { + const sql = compile(client, []); + expect(sql).not.toMatch(/do update set/i); + expect(sql).not.toMatch(/on duplicate key update/i); + expect(sql).not.toMatch(/on conflict/i); + expect(sql).not.toMatch(/insert ignore/i); + expect(sql).toBe(expected); + }); + } + + // The contrast: with a column to merge, the same helper still emits the + // conflict clause and merges exactly that column, never every column. + it('keeps the conflict clause, merging only the named columns, when there is something to merge', () => { + expect(compile('better-sqlite3', ['title'])).toBe( + 'insert into `os8740_t` (`created_at`, `id`) values (?, ?) on conflict (`email`) do update set `title` = excluded.`title`', + ); + expect(compile('pg', ['title'])).toBe( + 'insert into "os8740_t" ("created_at", "id") values (?, ?) on conflict ("email") do update set "title" = excluded."title"', + ); + expect(compile('mysql2', ['title'])).toBe( + 'insert into `os8740_t` (`created_at`, `id`) values (?, ?) on duplicate key update `title` = values(`title`)', + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +// Behaviour, per provisioned dialect +// ───────────────────────────────────────────────────────────────────────── + +/** A conflict-target column with a non-null default, so an omitted value still collides. */ +const DEFAULTED = { + name: 'os8740_defaulted', + fields: { email: { type: 'string' }, title: { type: 'string' } }, +} as any; + +/** A business key the payloads below omit, beside insert-only `created_at`. */ +const STAMPED = { + name: 'os8740_stamped', + fields: { email: { type: 'string' }, title: { type: 'string' } }, +} as any; + +const KEPT_ID = 'os8740_kept_row'; +const KEPT_CREATED = '2020-01-01 00:00:00'; + +/** Each dialect's native unique-violation code, as its client library reports it. */ +const UNIQUE_VIOLATION_CODES: Record = { + sqlite: ['SQLITE_CONSTRAINT_PRIMARYKEY', 'SQLITE_CONSTRAINT_UNIQUE'], + pg: ['23505'], + mysql: ['ER_DUP_ENTRY'], +}; + +for (const cell of DIALECT_CELLS) { + declareDialectCell(cell, 'upsert with nothing left to merge', declareEmptyMergeSetPins); +} + +function declareEmptyMergeSetPins(cell: DialectCell): void { + describe(`SqlDriver.upsert — nothing left to merge (${cell.label})`, () => { + let driver: SqlDriver; + let knexInstance: any; + + const rows = async (table: string): Promise>> => { + const found = await knexInstance(table).select('id', 'email', 'title').orderBy('id'); + return found.map((r: Record) => ({ ...r })); + }; + + const createdAtOf = async (id: string): Promise => { + const row = await knexInstance(STAMPED.name).where('id', id).first(); + const value = row?.created_at; + return value instanceof Date ? value.toISOString() : String(value); + }; + + /** + * The dialect's own unique-violation code, beside the shared predicate: the + * predicate matches its vocabulary anywhere in the message, and a statement + * echoed into an unrelated error's message can carry that vocabulary. + */ + const expectUniqueViolation = (err: unknown): void => { + expect(err, 'the collision was merged instead of refused').not.toBeNull(); + const { code, message } = err as { code?: string; message?: string }; + expect(isUniqueViolationError(err), `not a unique violation: ${String(message)}`).toBe(true); + expect(UNIQUE_VIOLATION_CODES[cell.id], `not a unique violation: ${String(message)}`).toContain(code); + }; + + const captureError = async (run: () => Promise): Promise => { + try { + await run(); + return null; + } catch (e) { + return e; + } + }; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + knexInstance = (driver as any).knex; + await knexInstance.schema.dropTableIfExists(DEFAULTED.name); + await knexInstance.schema.dropTableIfExists(STAMPED.name); + await knexInstance.schema.createTable(DEFAULTED.name, (t: any) => { + t.string('id', 64).primary(); + t.string('email', 191).notNullable().defaultTo('unset').unique(); + t.string('title', 191).nullable(); + }); + await knexInstance.schema.createTable(STAMPED.name, (t: any) => { + t.string('id', 64).primary(); + t.timestamp('created_at').nullable(); + t.string('email', 191).nullable().unique(); + t.string('title', 191).nullable(); + }); + driver.registerExternalObject({ ...DEFAULTED, external: { remoteName: DEFAULTED.name } }); + driver.registerExternalObject({ ...STAMPED, external: { remoteName: STAMPED.name } }); + }); + + afterAll(async () => { + await knexInstance?.schema.dropTableIfExists(DEFAULTED.name).catch(() => {}); + await knexInstance?.schema.dropTableIfExists(STAMPED.name).catch(() => {}); + await driver?.disconnect?.(); + }); + + beforeEach(async () => { + await knexInstance(DEFAULTED.name).delete(); + await knexInstance(STAMPED.name).delete(); + await knexInstance(DEFAULTED.name).insert({ id: KEPT_ID, email: 'unset', title: 'kept' }); + await knexInstance(STAMPED.name).insert({ + id: KEPT_ID, + created_at: new Date(`${KEPT_CREATED.replace(' ', 'T')}Z`), + email: 'kept@os8740.test', + title: 'kept', + }); + }); + + it('refuses a collision on an omitted conflict-target column with a unique violation, and rewrites no column', async () => { + const before = await rows(DEFAULTED.name); + + const err = await captureError(() => driver.upsert(DEFAULTED.name, { id: 'os8740_second_row' }, ['email'])); + + expectUniqueViolation(err); + expect(await rows(DEFAULTED.name), 'the stored row was rewritten, or a row was added').toEqual(before); + }); + + it('refuses a primary-key collision whose payload carries only insert-only columns, and rewrites no column', async () => { + const before = await rows(STAMPED.name); + const createdBefore = await createdAtOf(KEPT_ID); + + const err = await captureError(() => + driver.upsert(STAMPED.name, { id: KEPT_ID, created_at: '2030-06-01 00:00:00' }, ['email']), + ); + + expectUniqueViolation(err); + expect(await rows(STAMPED.name)).toEqual(before); + expect(await createdAtOf(KEPT_ID), 'an insert-only column was rewritten').toBe(createdBefore); + }); + + it('still inserts, and answers the row, when the payload collides with nothing', async () => { + const written = await driver.upsert(STAMPED.name, { id: 'os8740_fresh_row' }, ['email']); + + expect(written.id).toBe('os8740_fresh_row'); + expect((await rows(STAMPED.name)).map((r) => r.id).sort()).toEqual(['os8740_fresh_row', KEPT_ID].sort()); + }); + + it('still upserts onto the stored row when the payload carries its conflict target', async () => { + const before = await rows(STAMPED.name); + const createdBefore = await createdAtOf(KEPT_ID); + + const written = await driver.upsert(STAMPED.name, { id: KEPT_ID }, ['id']); + + expect(written.id).toBe(KEPT_ID); + expect(await rows(STAMPED.name)).toEqual(before); + expect(await createdAtOf(KEPT_ID)).toBe(createdBefore); + }); + }); +} diff --git a/packages/drivers/driver-sql/src/upsert-conflict-clause.ts b/packages/drivers/driver-sql/src/upsert-conflict-clause.ts new file mode 100644 index 00000000000..7f104971ea0 --- /dev/null +++ b/packages/drivers/driver-sql/src/upsert-conflict-clause.ts @@ -0,0 +1,31 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { Knex } from 'knex'; + +/** + * The conflict clause of `SqlDriver.upsert`'s statement, or none. + * + * `columnsToMerge` is what the merge leg may write: every payload column that + * is not insert-only, or — when that is empty — the conflict-target columns the + * payload carries, a provable no-op (a conflict means those columns matched). + * When BOTH are empty there is nothing the merge leg may write, so the + * statement is a plain INSERT with no conflict clause, and a collision surfaces + * as the driver's own unique-violation error. + * + * ⛔ Never a bare `merge()` here: knex compiles it to merge-ALL on every dialect + * (`do update set` / `on duplicate key update` over EVERY inserted column), the + * inverse of the insert-only set the merge leg exists to respect. ⛔ Nor + * `ignore()`: knex lowers it on MySQL to `insert ignore`, which swallows + * unrelated errors and drops the conflict target. + * + * Kept free of the driver so its compiled SQL is checkable per dialect with no + * server at all (`toSQL()`). + */ +export function withUpsertConflictClause( + insertion: Knex.QueryBuilder, + mergeKeys: readonly string[], + columnsToMerge: readonly string[], +): { statement: Knex.QueryBuilder; merges: boolean } { + if (columnsToMerge.length === 0) return { statement: insertion, merges: false }; + return { statement: insertion.onConflict([...mergeKeys]).merge([...columnsToMerge]), merges: true }; +} From fc2ee2770b19ef20ec1ffaf33c5cab98bcf9ced5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 08:28:30 +0000 Subject: [PATCH 2/5] fix(driver-sql): upsert with nothing left to merge is a plain INSERT, not merge-ALL Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude --- packages/drivers/driver-sql/src/sql-driver.ts | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 6789b4f9d46..ab93e7e1a7f 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -157,6 +157,7 @@ import { type UndeliveredAttribute, } from './builtin-column-collision.js'; import { recoverUnencodedJsonText } from './unencoded-json-text.js'; +import { withUpsertConflictClause } from './upsert-conflict-clause.js'; // [#25521 cut 1] The catalog-introspection helpers (`introspectColumnOrder`, // `introspectColumns` and the foreign-key, primary-key and unique-constraint reads), // [cut 4] `introspectSchema`, the whole-schema read built on them, and [cut 5] @@ -9974,8 +9975,8 @@ export class SqlDriver implements IDataDriver { // never reached it: `stampInsertTimestamps` puts a mergeable `updated_at` // in the payload there. [#11176] `stampUpsertUpdatedAt` now does the same // on Postgres and MySQL, so a TIMESTAMPED object no longer reaches this - // fallback on any dialect either — it is still reachable, and still #8740's - // to decide, for an object with no observed `updated_at` column.) + // fallback on any dialect either — it is still reachable for an object + // with no observed `updated_at` column.) // // The remedy keeps the emitted SQL byte-identical to what `main` emits // for this shape. Falling back to the CONFLICT-TARGET columns present in @@ -9985,14 +9986,15 @@ export class SqlDriver implements IDataDriver { // mergeable column in this case. So this PR's net behavioural delta on the // degenerate path is nil rather than a #7011 regression. // - // The last-ditch `merge()` is left as-is: it needs the conflict target to - // be absent from the payload, which means the INSERT never supplies it, so - // on every dialect here the target is NULL and cannot collide — the DO - // UPDATE clause is unreachable rather than merely unused. Filed separately - // rather than widened into: merge-ALL is the wrong shape for "nothing to - // merge" on any dialect, and the alternatives are not accept-set-neutral - // (knex compiles `.ignore()` to MySQL's `insert ignore`, which swallows - // unrelated errors too — measured). + // [#8740] When the payload carries no conflict-target column either, there + // is nothing the merge leg may write, and the statement is a plain INSERT + // with no conflict clause: a collision surfaces as the driver's own + // unique-violation error and the stored row is left as it was. This used + // to fall back to knex's bare `merge()`, which is merge-ALL on every + // dialect: it re-admitted every insert-only column the lines above + // exclude. ⛔ Not `ignore()` either: knex lowers it on MySQL to + // `insert ignore`, which swallows unrelated errors and drops the conflict + // target. See {@link withUpsertConflictClause}. const noopMergeColumns = Object.keys(formatted).filter((c) => mergeKeys.includes(c)); const columnsToMerge = mergeColumns.length > 0 ? mergeColumns : noopMergeColumns; @@ -10017,8 +10019,8 @@ export class SqlDriver implements IDataDriver { // is `injectTenantOnInsert`'s job (called above), which is exactly why // this one is deliberately not scoped. const builder = this.getBuilder(writeTable, writeOptions); - const insertion = builder.insert(formatted).onConflict(mergeKeys); - const merging = columnsToMerge.length > 0 ? insertion.merge(columnsToMerge) : insertion.merge(); + const insertion = builder.insert(formatted); + const { statement, merges } = withUpsertConflictClause(insertion, mergeKeys, columnsToMerge); // [#21185] SQLite and PostgreSQL: the fence is a predicate INSIDE the // merge statement — `… DO UPDATE SET … WHERE` the stored tenant column // equals the written one — so a conflict on a row of another @@ -10030,8 +10032,11 @@ export class SqlDriver implements IDataDriver { // a platform row with no organization is fenced off too. The target // table is named by its bare physical name, which both dialects accept // for the INSERT target in this clause, schema-qualified or not. - // MySQL takes no `WHERE` here; its fence runs after the statement. - await (fence && !this.isMysql ? merging.where(this.mergeLandsInWrittenOrganization(writeTable, fence.column)) : merging); + // MySQL takes no `WHERE` here; its fence runs after the statement. A + // plain INSERT ([#8740]) has no merge leg to fence. + await (merges && fence && !this.isMysql + ? statement.where(this.mergeLandsInWrittenOrganization(writeTable, fence.column)) + : statement); if (mysqlFence) { // [#21185] One read, under the written tenant exactly; #8807's // identity verdict rides on it when a rival key made one owed. From d0ba27bfb79cb28141bb6d32b940d923d13d016a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 08:30:43 +0000 Subject: [PATCH 3/5] chore(changeset): driver-sql upsert with nothing left to merge Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude --- .changeset/8740-upsert-nothing-to-merge.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .changeset/8740-upsert-nothing-to-merge.md diff --git a/.changeset/8740-upsert-nothing-to-merge.md b/.changeset/8740-upsert-nothing-to-merge.md new file mode 100644 index 00000000000..e5e930bb302 --- /dev/null +++ b/.changeset/8740-upsert-nothing-to-merge.md @@ -0,0 +1,20 @@ +--- +'@objectstack/driver-sql': patch +--- + +fix(driver-sql): an `upsert` with nothing left to merge is a plain INSERT, so a collision surfaces as the driver's unique violation instead of merging every column + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING** (a served-set narrowing) for an `upsert` whose payload carries only insert-only columns (`id`, `created_at`, `auto_number` columns) and none of its conflict-target columns, when that row collides with a stored row. Such a call used to merge EVERY payload column into the stored row, insert-only columns included. It now raises the driver's unique-violation error and leaves the stored row as it was. + +`SqlDriver.upsert` merges 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 carries neither, the statement used to fall back to knex's bare `merge()`, which compiles to merge-ALL on every dialect (`do update set` / `on duplicate key update` over every inserted column). That fallback is removed: with nothing to merge, the statement is a plain `INSERT` with no conflict clause. It is not `insert ignore` / `do nothing` either, which would hide a collision. + +Unchanged: a payload with a column to merge, or one that carries its conflict target, upserts exactly as before, and a payload that collides with nothing is inserted and answered as before. + +Also changed, on SQLite and PostgreSQL only: a conflict target that no unique index backs is no longer refused when the payload carries nothing to merge and none of the target's columns. There is no conflict clause left for the database to judge, so the row is inserted. MySQL still refuses it before the statement, and every call with something to merge is still refused on all three dialects. + +**What changes for you.** If you upsert a payload made only of insert-only columns under a business conflict key the payload omits, catch the unique-violation error on a collision, or use `update()` to change a stored row. From 8f210ba45ddf3feaa218c646958d4df37b30668e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 09:44:21 +0000 Subject: [PATCH 4/5] test(driver-sql): pin the unbacked-target refusal when nothing is left to merge (red on head) Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude --- .../sql-driver-upsert-empty-merge-set.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts b/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts index db6717761ea..e351f9324e4 100644 --- a/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-upsert-empty-merge-set.test.ts @@ -19,6 +19,9 @@ * the payload omits (the column has a non-null default) is refused and * rewrites nothing; a collision on the primary key with only insert-only * columns in the payload is refused and rewrites nothing; + * - a conflict target that no PRIMARY KEY or UNIQUE index backs is still + * refused (`VALIDATION_ERROR` / 400) on that path, and nothing is written — + * the same answer every dialect gives when there IS something to merge; * - controls: a payload that collides with nothing still inserts and answers * the row, and a payload that carries its conflict target still upserts * onto the stored row. @@ -99,6 +102,12 @@ const STAMPED = { fields: { email: { type: 'string' }, title: { type: 'string' } }, } as any; +/** No index on `email` at all: a conflict target nothing backs. */ +const UNBACKED = { + name: 'os8740_unbacked', + fields: { email: { type: 'string' }, title: { type: 'string' } }, +} as any; + const KEPT_ID = 'os8740_kept_row'; const KEPT_CREATED = '2020-01-01 00:00:00'; @@ -155,6 +164,7 @@ function declareEmptyMergeSetPins(cell: DialectCell): void { knexInstance = (driver as any).knex; await knexInstance.schema.dropTableIfExists(DEFAULTED.name); await knexInstance.schema.dropTableIfExists(STAMPED.name); + await knexInstance.schema.dropTableIfExists(UNBACKED.name); await knexInstance.schema.createTable(DEFAULTED.name, (t: any) => { t.string('id', 64).primary(); t.string('email', 191).notNullable().defaultTo('unset').unique(); @@ -167,18 +177,26 @@ function declareEmptyMergeSetPins(cell: DialectCell): void { t.string('title', 191).nullable(); }); driver.registerExternalObject({ ...DEFAULTED, external: { remoteName: DEFAULTED.name } }); + await knexInstance.schema.createTable(UNBACKED.name, (t: any) => { + t.string('id', 64).primary(); + t.string('email', 191).nullable(); + t.string('title', 191).nullable(); + }); driver.registerExternalObject({ ...STAMPED, external: { remoteName: STAMPED.name } }); + driver.registerExternalObject({ ...UNBACKED, external: { remoteName: UNBACKED.name } }); }); afterAll(async () => { await knexInstance?.schema.dropTableIfExists(DEFAULTED.name).catch(() => {}); await knexInstance?.schema.dropTableIfExists(STAMPED.name).catch(() => {}); + await knexInstance?.schema.dropTableIfExists(UNBACKED.name).catch(() => {}); await driver?.disconnect?.(); }); beforeEach(async () => { await knexInstance(DEFAULTED.name).delete(); await knexInstance(STAMPED.name).delete(); + await knexInstance(UNBACKED.name).delete(); await knexInstance(DEFAULTED.name).insert({ id: KEPT_ID, email: 'unset', title: 'kept' }); await knexInstance(STAMPED.name).insert({ id: KEPT_ID, @@ -210,6 +228,18 @@ function declareEmptyMergeSetPins(cell: DialectCell): void { expect(await createdAtOf(KEPT_ID), 'an insert-only column was rewritten').toBe(createdBefore); }); + it('refuses a conflict target that no unique index backs, and writes nothing', async () => { + const err = (await captureError(() => driver.upsert(UNBACKED.name, { id: 'os8740_unbacked_row' }, ['email']))) as { + code?: string; + status?: number; + } | null; + + expect(err, 'the unbacked conflict target was accepted').not.toBeNull(); + expect(err!.code).toBe('VALIDATION_ERROR'); + expect(err!.status).toBe(400); + expect(await rows(UNBACKED.name), 'a row was written').toEqual([]); + }); + it('still inserts, and answers the row, when the payload collides with nothing', async () => { const written = await driver.upsert(STAMPED.name, { id: 'os8740_fresh_row' }, ['email']); From b4fc36f35e265f6c25471504842838d88f58fe2a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 09:45:16 +0000 Subject: [PATCH 5/5] fix(driver-sql): refuse an unbacked conflict target on the plain-INSERT path, on every dialect Claude-Session: https://claude.ai/code/session_01T6USnvRkjp4BPytcNttAuF Co-authored-by: Claude --- .changeset/8740-upsert-nothing-to-merge.md | 4 +- packages/drivers/driver-sql/src/sql-driver.ts | 70 +++++++++++++++---- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/.changeset/8740-upsert-nothing-to-merge.md b/.changeset/8740-upsert-nothing-to-merge.md index e5e930bb302..c857529cfc6 100644 --- a/.changeset/8740-upsert-nothing-to-merge.md +++ b/.changeset/8740-upsert-nothing-to-merge.md @@ -4,7 +4,7 @@ fix(driver-sql): an `upsert` with nothing left to merge is a plain INSERT, so a collision surfaces as the driver's unique violation instead of merging every column -Clause-②: yes (narrowing) +Clause-②: no (narrowing) Metadata-protocol: none @@ -15,6 +15,6 @@ Metadata-protocol: none Unchanged: a payload with a column to merge, or one that carries its conflict target, upserts exactly as before, and a payload that collides with nothing is inserted and answered as before. -Also changed, on SQLite and PostgreSQL only: a conflict target that no unique index backs is no longer refused when the payload carries nothing to merge and none of the target's columns. There is no conflict clause left for the database to judge, so the row is inserted. MySQL still refuses it before the statement, and every call with something to merge is still refused on all three dialects. +A conflict target that no PRIMARY KEY or UNIQUE index backs is still refused on that path, with the same `VALIDATION_ERROR` / 400 every dialect answers when there is something to merge. A plain INSERT asks the database nothing about the target, so the driver checks it against the table's keys before the statement, on every dialect. **What changes for you.** If you upsert a payload made only of insert-only columns under a business conflict key the payload omits, catch the unique-violation error on a collision, or use `update()` to change a stored row. diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index ab93e7e1a7f..3c5654b0c83 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -2257,6 +2257,30 @@ function refuseUndeclaredFieldType(column: string): never { * matching the twin: the two-argument form needs the ES2022 `ErrorOptions` * overload. */ +/** + * Whether a physical key index covers exactly the conflict target — the same + * columns, order-insensitively, which is how both `ON CONFLICT` dialects infer + * an arbiter index and the only reading under which a composite key means + * anything. Shared by {@link SqlDriver.assertConflictTargetHonoured} and + * {@link SqlDriver.assertConflictTargetBackedWithoutClause}. + */ +function indexCoversConflictTarget(index: PhysicalIndex, mergeKeys: readonly string[]): boolean { + const wanted = new Set(mergeKeys); + return index.columns.length === wanted.size && index.columns.every((c) => wanted.has(c)); +} + +/** + * The `cause` an introspection-based unbacked-target refusal carries, where the + * server's own sentence stands on a server-judged one: schema identifiers only, + * no row values. + */ +function unbackedConflictTargetCause(tableName: string, mergeKeys: readonly string[], keys: PhysicalIndex[]): Error { + const found = keys.map((i) => `${i.name}(${i.columns.join(', ')})`).join(', '); + return new Error( + `no PRIMARY KEY or UNIQUE index on "${tableName}" covers (${mergeKeys.join(', ')}); ` + `keys present: ${found}`, + ); +} + function refuseUnbackedConflictTarget(object: string, mergeKeys: string[], cause: unknown): Error { const keys = mergeKeys.map((k) => `"${k}"`).join(', '); const err = new Error( @@ -9496,9 +9520,7 @@ export class SqlDriver implements IDataDriver { const target = this.rotationWriteTarget(object) ?? object; const tableName = this.physicalTableByObject[target] ?? target; - const wanted = new Set(mergeKeys); - const covers = (i: PhysicalIndex): boolean => - i.columns.length === wanted.size && i.columns.every((c) => wanted.has(c)); + const covers = (i: PhysicalIndex): boolean => indexCoversConflictTarget(i, mergeKeys); type Verdict = | { kind: 'honoured' } @@ -9589,15 +9611,36 @@ export class SqlDriver implements IDataDriver { // `cause` carries the ground truth an operator needs to act. Schema // identifiers only — no row values, matching the payload contract the // refusal already keeps. - const found = keys.map((i) => `${i.name}(${i.columns.join(', ')})`).join(', '); - throw refuseUnbackedConflictTarget( - object, - mergeKeys, - new Error( - `no PRIMARY KEY or UNIQUE index on "${tableName}" covers (${mergeKeys.join(', ')}); ` + - `keys present: ${found}`, - ), - ); + throw refuseUnbackedConflictTarget(object, mergeKeys, unbackedConflictTargetCause(tableName, mergeKeys, keys)); + } + + /** + * [#8740] The unbacked-conflict-target refusal for an upsert statement that + * carries NO conflict clause — the one {@link upsert} sends when nothing is + * left to merge ({@link withUpsertConflictClause}). + * + * With a conflict clause, SQLite and PostgreSQL refuse an unbacked target + * themselves (the planner finds no arbiter index) and MySQL refuses it in + * {@link assertConflictTargetHonoured}. A plain INSERT asks the server + * nothing about the target, so without this check that path would accept on + * two dialects a call every dialect refuses when there is something to merge. + * So the answer is the same refusal, on every dialect, read from the same + * cached introspection MySQL's pre-flight reads, with the same certainty + * rule: a failed read or a table with no keys at all proceeds, and a refusal + * is re-read fresh first, so a stale cache can never refuse a backed target. + */ + private async assertConflictTargetBackedWithoutClause( + object: string, + writeTable: string, + mergeKeys: string[], + ): Promise { + const tableName = this.physicalTableByObject[writeTable] ?? writeTable; + const backed = (keys: PhysicalIndex[] | null): boolean => + keys === null || keys.length === 0 || keys.some((i) => indexCoversConflictTarget(i, mergeKeys)); + if (backed(await this.introspectKeyIndexes(tableName))) return; + const fresh = await this.introspectKeyIndexes(tableName, { fresh: true }); + if (backed(fresh)) return; + throw refuseUnbackedConflictTarget(object, mergeKeys, unbackedConflictTargetCause(tableName, mergeKeys, fresh!)); } /** @@ -9997,6 +10040,9 @@ export class SqlDriver implements IDataDriver { // target. See {@link withUpsertConflictClause}. const noopMergeColumns = Object.keys(formatted).filter((c) => mergeKeys.includes(c)); const columnsToMerge = mergeColumns.length > 0 ? mergeColumns : noopMergeColumns; + // …and that plain INSERT still refuses a conflict target nothing backs, + // as every dialect does when there is something to merge. + if (columnsToMerge.length === 0) await this.assertConflictTargetBackedWithoutClause(object, writeTable, mergeKeys); // [#21185] The fence for THIS attempt: the tenant column, and the value // the row is written under in storage form (what the statement sends).