From 91bdacb7062e13f864f5b19c0406847e4114ce7a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 00:15:40 +0000 Subject: [PATCH] fix(drivers): recognise the unbacked conflict target on Postgres too (#8567) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured PostgreSQL 16.13 through the same knex + pg path SqlDriver.upsert uses: SQLSTATE 42P10, routine=infer_arbiter_indexes, and the sentence "there is no unique or exclusion constraint matching the ON CONFLICT specification". #8445's recognition was SQLite-only, so on Postgres the raw error escaped and the caller got statement text with no code to branch on. Recognition graduates to `isUnbackedConflictTargetError` in @objectstack/types, beside — and deliberately separate from — `isUniqueViolationError`, which answers the opposite condition. One measured message limb per dialect; the `code` channel is left unread because it over-matches on Postgres (42P10 is invalid_column_reference, which an out-of-range ORDER BY position also raises) and is generic on SQLite. MySQL cannot raise the condition: knex compiles onConflict().merge() there to ON DUPLICATE KEY UPDATE, which takes no conflict target. Pinned from the compiled statement; the live MySQL cell is declared un-run, not omitted. One clause of the refusal wording moved on BOTH faces — "SQLite refuses the statement" to "the database refuses the statement" — because naming SQLite to a Postgres operator points at the wrong engine. #5240's one-wording rule and #8568's cross-face parity pin are what keep the two faces together. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VoxQqG5FiUHZKCST7KDoZC --- .../unbacked-conflict-target-dialects.md | 55 +++ ...er-upsert-conflict-target-dialects.test.ts | 326 ++++++++++++++++++ ...er-upsert-conflict-target-envelope.test.ts | 9 +- packages/drivers/driver-sql/src/sql-driver.ts | 127 ++++--- .../driver-turso/src/remote-transport.ts | 25 +- packages/types/src/index.ts | 6 + .../src/unbacked-conflict-target.test.ts | 272 +++++++++++++++ .../types/src/unbacked-conflict-target.ts | 215 ++++++++++++ packages/types/src/unique-violation.ts | 22 ++ 9 files changed, 991 insertions(+), 66 deletions(-) create mode 100644 .changeset/unbacked-conflict-target-dialects.md create mode 100644 packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts create mode 100644 packages/types/src/unbacked-conflict-target.test.ts create mode 100644 packages/types/src/unbacked-conflict-target.ts diff --git a/.changeset/unbacked-conflict-target-dialects.md b/.changeset/unbacked-conflict-target-dialects.md new file mode 100644 index 0000000000..3792b1fa02 --- /dev/null +++ b/.changeset/unbacked-conflict-target-dialects.md @@ -0,0 +1,55 @@ +--- +"@objectstack/types": patch +"@objectstack/driver-sql": patch +"@objectstack/driver-turso": patch +--- + +fix(drivers): a `conflictKeys` upsert with no backing unique index refuses legibly on **Postgres** too, not only SQLite (#8567) + +`SqlDriver.upsert` recognised "the `ON CONFLICT` target is not backed by a +PRIMARY KEY or UNIQUE index" on **SQLite only** (#8445). `driver-sql` serves +three dialects, so on Postgres the raw driver error still escaped: `mapDataError` +fell through to its default branch and served the thrown message as the whole +response body — and that message is the **statement**, with no `code` for any +client to branch on. + +**What a Postgres caller got, and now gets.** Measured against a real +PostgreSQL 16.13 through the same knex + `pg` path the driver uses: + +``` +before: code=42P10 status=undefined + message=insert into "plain" ("email", "id", "title") values ($1, $2, $3) + on conflict ("email") do update set "title" = excluded."title" + - there is no unique or exclusion constraint matching the ON CONFLICT specification + +after: code=VALIDATION_ERROR status=400 + message=Cannot upsert into "plain" on conflict keys ("email"): no PRIMARY KEY or + UNIQUE index backs them, … Fix by declaring the column(s) "unique: true" … +``` + +The accept/reject set does not move: the same upserts fail, they fail legibly. +The server's own sentence is preserved on the error's `cause`, which no error +mapper puts on the wire, so an operator debugging the table keeps the ground +truth while the caller stops receiving SQL text. + +**One clause of the refusal wording changed, on both faces.** "…and SQLite +refuses the statement" is now "…and **the database** refuses the statement". +Once recognition covers Postgres, naming SQLite points a Postgres operator at +the wrong engine. `driver-turso`'s remote-face copy moved in the same commit — +the two are held word-for-word identical (#5240) by #8568's cross-face parity +pin. No other sentence of the refusal changed. + +**Recognition is now a named, shared predicate.** +`isUnbackedConflictTargetError` is exported from `@objectstack/types` beside +`isUniqueViolationError`, carrying one measured message limb per dialect that +can raise the condition. ⚠️ It is deliberately a **separate** predicate: +`isUniqueViolationError` answers the opposite condition (an index exists and the +row violated it), and a merged one would report a working constraint as a +missing one. + +**MySQL is unaffected, by measurement rather than omission.** knex compiles +`onConflict(...).merge(...)` on that dialect to `ON DUPLICATE KEY UPDATE`, which +takes no conflict target — the named keys never leave the process, so the server +is never asked to find an index for them and the condition cannot arise. The +compiled statement is pinned; the live MySQL cell is reported as un-run rather +than passing vacuously. 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 new file mode 100644 index 0000000000..df85e6f7a2 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.ts @@ -0,0 +1,326 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8567] The unbacked-conflict-target refusal, across the DIALECTS + * `driver-sql` actually serves — not just the one #8445 could raise it on. + * + * # What this card measured, and why measuring was the deliverable + * + * #8445 landed the refusal against SQLite's sentence and said so in the code: + * the container had no other server, and its dispatch ruled that transcribing + * another dialect's wording from memory is not evidence. That left Postgres + * and MySQL answering the raw driver error — `mapDataError` falling through to + * its default branch and shipping the STATEMENT, bound values included, with no + * `code` for any client to branch on. Two of three dialects, the same payload + * argument #8445 made for the third. + * + * So this file's first job is to be the place a dialect's wording is + * OBSERVED rather than assumed. Postgres was raised for real — system PG 16 + * binaries, `initdb` + `pg_ctl`, no container runtime — through the same + * knex + `pg` path `upsert` takes, and the sentence it produced is now a limb + * of `isUnbackedConflictTargetError` in `@objectstack/types`. + * + * ``` + * # PostgreSQL 16.13, knex 3.3.0 + pg 8.22.0 + * upsert('plain', { email: 'a@b.com', title: 'x' }, ['email']) + * -> name=error (DatabaseError) code=42P10 severity=ERROR status=undefined + * routine=infer_arbiter_indexes constraint=undefined detail=undefined + * msg=insert into "plain" ("email", "id", "title") values ($1, $2, $3) + * on conflict ("email") do update set "title" = excluded."title" + * - there is no unique or exclusion constraint matching the ON CONFLICT specification + * ``` + * + * # The three cells are not symmetric, and pretending otherwise would lie + * + * - **SQLite** runs everywhere, in-process. It is the cell that always + * executes, so a regression in the shared predicate cannot hide behind an + * unprovisioned matrix. + * - **Postgres** runs when `OS_TEST_POSTGRES_URL` is set, and is REPORTED as + * an un-run cell when it is not (`declareUnprovisionedCell`) — never a + * silent pass. This is the cell whose wording this card added. + * - **MySQL** cannot raise the condition at all, which is a measurement, not + * an excuse. knex compiles `onConflict(...).merge(...)` on that dialect to + * `ON DUPLICATE KEY UPDATE`, which takes **no conflict target**: the named + * keys are dropped before the statement leaves the process, so the server is + * never asked to find an index for them. That is checkable with no server at + * all, and the compile pin below checks it. The LIVE MySQL cell is still + * declared un-run rather than dropped, because "the condition cannot arise" + * is a claim about knex's compiler that a real server should eventually be + * held to. + * + * ⚠️ What MySQL does INSTEAD of refusing — merge on whichever unique key the + * row collides with, or insert a second row — is a different defect with a + * different fix, filed separately. This file does not assert it, because + * nobody has watched a MySQL server do it: an assertion written from the + * compiled SQL alone would be exactly the transcribed-from-memory evidence + * this card exists to stop accepting. + * + * # Reverse verification — direction predicted BEFORE it was run + * + * Predicted, removing the Postgres limb from `UNBACKED_CONFLICT_TARGET` in + * `@objectstack/types` with the fix committed first: the Postgres cell's + * envelope pin and leak pin go RED (the raw `DatabaseError` returns, `code` + * `42P10`, `status` undefined, statement text back in the caller-visible + * message), and **every SQLite pin stays GREEN** — the SQLite limb is + * untouched, which is what makes the two limbs independent rather than one + * regex that happens to cover both. The positive controls stay GREEN on both + * cells across that revert: they never depended on recognition, which is + * precisely what makes them controls. Measured, and it matched. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import knex from 'knex'; +import { StandardErrorCode } from '@objectstack/spec/api'; +import { SqlDriver } from '../src/index.js'; +import { DIALECT_CELLS, declareUnprovisionedCell, type DialectCell } from './live-dialect-matrix.testkit.js'; + +/** The shape `mapDataError` / `sendError` read off a thrown driver error. */ +interface WireBearingError extends Error { + code?: string; + status?: number; + cause?: unknown; +} + +/** + * Table names are card-scoped rather than the generic `crm_contact` #8445 uses: + * the live cells share ONE database with every other matrix in this package, so + * a generic name is a cross-suite collision waiting for the first parallel run. + */ +const BACKED = { + name: 'os8567_backed', + fields: { + email: { type: 'string', unique: true }, + title: { type: 'string' }, + }, +} as any; + +/** The same object with the `unique` declaration REMOVED — the unbacked target. */ +const PLAIN = { + name: 'os8567_plain', + fields: { + email: { type: 'string' }, + title: { type: 'string' }, + }, +} as any; + +const captureError = async (run: () => Promise): Promise => { + try { + await run(); + return null; + } catch (e) { + return e as WireBearingError; + } +}; + +/** + * The dialects whose `ON CONFLICT` compilation can carry a target at all. MySQL + * is excluded here and handled on its own terms below — running it through this + * sweep would assert a refusal that cannot happen and would read as a driver + * defect rather than as the dialect fact it is. + */ +const ON_CONFLICT_DIALECTS = new Set(['sqlite', 'pg']); + +for (const cell of DIALECT_CELLS) { + if (!ON_CONFLICT_DIALECTS.has(cell.id)) continue; + if (!cell.available) { + declareUnprovisionedCell(cell, 'unbacked conflict-target refusal'); + continue; + } + declareRefusalSweep(cell); +} + +function declareRefusalSweep(cell: DialectCell): void { + describe(`SqlDriver.upsert — unbacked conflict-target refusal (${cell.label})`, () => { + let driver: SqlDriver; + let knexInstance: any; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + knexInstance = (driver as any).knex; + // Live cells reuse one database, so the sweep starts from dropped tables. + await knexInstance.schema.dropTableIfExists(BACKED.name); + await knexInstance.schema.dropTableIfExists(PLAIN.name); + await driver.initObjects([BACKED, PLAIN]); + }); + + afterAll(async () => { + await knexInstance?.schema.dropTableIfExists(BACKED.name).catch(() => {}); + await knexInstance?.schema.dropTableIfExists(PLAIN.name).catch(() => {}); + await driver?.disconnect?.(); + }); + + // ─────────────────────────────────────────────────────────────────── + // Pin 1 — the envelope. `code` AND `status`, never a bare toThrow(). + // ─────────────────────────────────────────────────────────────────── + + /** + * A bare `rejects.toThrow()` is blind here in both directions: the un-fixed + * driver threw for this input too (that is the whole defect), so it was + * green before and after. The measured pre-fix values are the ones the + * negative assertions name — `SQLITE_ERROR` on one cell, `42P10` on the + * other, `status: undefined` on both. + */ + it('answers a real `code` and `status` — not the raw driver error', async () => { + const err = await captureError(() => driver.upsert(PLAIN.name, { email: 'a@b.com', title: 'x' }, ['email'])); + + expect(err, 'the unbacked conflict target must still be refused, not silently accepted').not.toBeNull(); + expect(err!.code).toBe(StandardErrorCode.enum.VALIDATION_ERROR); + expect(err!.status).toBe(400); + // The two measured raw codes this refusal replaces, one per cell. + expect(err!.code).not.toBe('SQLITE_ERROR'); + expect(err!.code).not.toBe('42P10'); + + // The message names what an operator has to act on. + expect(err!.message).toMatch(new RegExp(PLAIN.name)); + expect(err!.message).toMatch(/email/); + expect(err!.message).toMatch(/unique/i); + }); + + /** + * One condition, one wording (#5240) — and it must not name a dialect. The + * clause read "SQLite refuses the statement" until this card; on a Postgres + * deployment that sentence pointed the reader at the wrong engine. + */ + it('states the refusal without naming a single engine', async () => { + const err = await captureError(() => driver.upsert(PLAIN.name, { email: 'a@b.com', title: 'x' }, ['email'])); + + expect(err!.message.split('. ')[0] + '.').toBe( + `Cannot upsert into "${PLAIN.name}" on conflict keys ("email"): no PRIMARY KEY or UNIQUE ` + + 'index backs them, so the merge target does not exist and the database refuses the statement.', + ); + expect(err!.message).not.toMatch(/SQLite|Postgres|MySQL/i); + }); + + // ─────────────────────────────────────────────────────────────────── + // Pin 2 — the payload: the statement must not travel with the refusal + // ─────────────────────────────────────────────────────────────────── + + it('keeps the SQL statement and its bound values out of the caller-visible message', async () => { + const err = await captureError(() => + driver.upsert(PLAIN.name, { email: 'leaked@example.com', title: 'secret-title' }, ['email']), + ); + + expect(err).not.toBeNull(); + expect(err!.message).not.toMatch(/insert into/i); + expect(err!.message).not.toMatch(/excluded\./i); + expect(err!.message).not.toContain('leaked@example.com'); + expect(err!.message).not.toContain('secret-title'); + + // …while the server's own text stays reachable through `cause`, which no + // error mapper puts on the wire. This is the ground truth a DBA wants, + // and it is the per-dialect sentence — the assertion is deliberately + // loose about WHICH one, because both cells run this same case. + const causeText = String((err!.cause as Error | undefined)?.message); + expect(causeText).toMatch(/insert into/i); + expect(causeText).toMatch( + /ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint|there is no unique or exclusion constraint matching the ON CONFLICT specification/i, + ); + }); + + // ─────────────────────────────────────────────────────────────────── + // Pin 3 — the controls: what recognition must NOT have swallowed + // ─────────────────────────────────────────────────────────────────── + + /** + * Without this, a predicate that matched EVERY upsert failure would pass + * every pin above while having destroyed the capability the driver exists + * to provide. + */ + it('MERGES when a declared unique index does back the conflict target', async () => { + await driver.upsert(BACKED.name, { email: 'ctl@b.com', title: 'first' }, ['email']); + await driver.upsert(BACKED.name, { email: 'ctl@b.com', title: 'second' }, ['email']); + + const rows = await driver.find(BACKED.name, { where: { email: 'ctl@b.com' } }); + expect(rows).toHaveLength(1); + expect(rows[0].title).toBe('second'); + }); + + it('leaves the default `id` merge key working — no conflictKeys, no refusal', async () => { + await driver.upsert(PLAIN.name, { id: 'os8567_fixed', email: 'id@b.com', title: 'first' }); + await driver.upsert(PLAIN.name, { id: 'os8567_fixed', email: 'id@b.com', title: 'second' }); + + const rows = await driver.find(PLAIN.name, { where: { id: 'os8567_fixed' } }); + expect(rows).toHaveLength(1); + expect(rows[0].title).toBe('second'); + }); + + /** + * The specificity control, and the reason the predicate reads the message + * rather than the code on BOTH cells: Postgres answers `42P10` here too for + * an out-of-range `ORDER BY` position, and SQLite answers its generic + * `SQLITE_ERROR` for a missing table. Either would be swallowed by a + * code-based test. + */ + it('does not swallow an unrelated statement failure as this refusal', async () => { + const err = await captureError(() => driver.upsert('os8567_never_created', { id: 'x' })); + + expect(err).not.toBeNull(); + expect(err!.message).not.toMatch(/Cannot upsert into/); + expect(err!.code).not.toBe(StandardErrorCode.enum.VALIDATION_ERROR); + }); + }); +} + +// ───────────────────────────────────────────────────────────────────────── +// MySQL — the condition cannot arise, proven by what knex COMPILES +// ───────────────────────────────────────────────────────────────────────── + +describe('[#8567] MySQL: `onConflict().merge()` compiles the conflict target away', () => { + /** + * No server, no connection — `toSQL()` runs the dialect's compiler alone. + * This is the whole MySQL half of the card's measurement question, and it is + * answerable exactly because it is a claim about knex rather than about a + * server nobody here can start. + */ + const compile = (client: string): string => { + const k = knex({ client, connection: {} } as any); + try { + return k('os8567_plain') + .insert({ id: '1', email: 'a@b.com', title: 'x' }) + .onConflict(['email']) + .merge(['title']) + .toSQL().sql; + } finally { + void k.destroy(); + } + }; + + it('emits ON DUPLICATE KEY UPDATE, which takes no conflict target', () => { + const sql = compile('mysql2'); + + expect(sql).toMatch(/on duplicate key update/i); + // The named key never reaches the server as a TARGET, so the server cannot + // report that no index backs it — there is nothing for it to look up. + expect(sql).not.toMatch(/on conflict/i); + expect(sql).toBe( + 'insert into `os8567_plain` (`email`, `id`, `title`) values (?, ?, ?) ' + + 'on duplicate key update `title` = values(`title`)', + ); + }); + + /** + * The contrast is the argument: the SAME builder call keeps the target on the + * dialects that have `ON CONFLICT`. Without this half, the assertion above + * would be consistent with knex having dropped conflict targets everywhere. + */ + it('keeps the conflict target on the dialects that have ON CONFLICT', () => { + expect(compile('pg')).toMatch(/on conflict \("email"\)/i); + expect(compile('better-sqlite3')).toMatch(/on conflict \(`email`\)/i); + }); +}); + +/** + * The live MySQL cell: declared un-run, never quietly dropped. + * + * The compile pin above proves the refusal cannot arise on MySQL. It does NOT + * prove what happens instead, and that question needs a server this container + * has none of (`mysqld` and `mariadbd` are both absent; only a PHP client + * library is installed, and the docker daemon is unreachable). Reporting the + * cell keeps that gap addressable by anyone who has one, instead of leaving a + * dialect silently uncovered — which is the vacuous-green shape + * `live-dialect-matrix.testkit.ts` exists to prevent. + */ +const MYSQL_CELL = DIALECT_CELLS.find((c) => c.id === 'mysql')!; +if (!MYSQL_CELL.available) { + declareUnprovisionedCell(MYSQL_CELL, 'unbacked conflict-target refusal (behaviour never observed)'); +} diff --git a/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-envelope.test.ts b/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-envelope.test.ts index 225e9944e9..6384297015 100644 --- a/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-envelope.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-upsert-conflict-target-envelope.test.ts @@ -182,13 +182,20 @@ describe('[#8445] SqlDriver.upsert refuses an unbacked conflict target in the AD * of `SqlDriver`, so neither source can hold it. That pin belongs in * `driver-turso`, outside this card's declared file surface, and is filed as * #8568 rather than smuggled in here. + * + * [#8567] The literal moved once, deliberately: "and SQLite refuses the + * statement" became "and the database refuses the statement", because + * recognition now covers Postgres and naming SQLite to a Postgres operator + * points at the wrong engine. `driver-turso`'s copy moved in the same commit, + * and #8568's two-way parity pin is what proves it — a reword that updated + * only one face reddens there, which is that file's whole purpose. */ it('opens with the remote refusal’s first sentence, word for word', async () => { const err = await captureError(() => driver.upsert(PLAIN.name, { email: 'a@b.com', title: 'x' }, ['email'])); expect(err!.message.split('. ')[0] + '.').toBe( 'Cannot upsert into "crm_contact_plain" on conflict keys ("email"): no PRIMARY KEY or UNIQUE ' + - 'index backs them, so the merge target does not exist and SQLite refuses the statement.', + 'index backs them, so the merge target does not exist and the database refuses the statement.', ); }); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 7427409804..df3f37d24d 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -56,7 +56,12 @@ import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts'; import { StandardErrorCode } from '@objectstack/spec/api'; import { StorageNameMapping } from '@objectstack/spec/system'; import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared'; -import { isUniqueViolationError, uniqueViolationColumn, resolveTenancyPosture } from '@objectstack/types'; +import { + isUniqueViolationError, + isUnbackedConflictTargetError, + uniqueViolationColumn, + resolveTenancyPosture, +} from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; import { nextUtcCalendarDay } from '@objectstack/core'; import { @@ -967,56 +972,27 @@ function refuseDateBucketedGroupBy(granularity: string, bucketedHere: string[], throw err; } -/** - * [#8445] Is this the "the conflict target is not a key" failure? - * - * # Which channel answers, and why it is the message - * - * SQLite fills exactly one: the MESSAGE. It raises a plain `SQLITE_ERROR` — - * the same generic code a syntax error carries — so `code` cannot discriminate, - * and a `code`-based test would swallow every other statement failure as an - * unbacked conflict target. Measured on this face, through knex + - * better-sqlite3, before the fix: - * - * ``` - * upsert('plain', { email: 'a@b.com', title: 'x' }, ['email']) - * -> THREW name=SqliteError code=SQLITE_ERROR status=undefined - * msg=insert into `plain` (...) values (...) on conflict (`email`) do update set ... - * - ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint - * ``` - * - * Note what knex does to the message: it prefixes the STATEMENT and keeps - * SQLite's own sentence after ` - `. So the tail this predicate matches is - * present on the local face too, and the regex is anchored on that tail rather - * than on the whole string. The text is SQLite's own, stable since `ON - * CONFLICT` arrived in 3.24, and narrow enough that no other failure shares it. - * - * # What it deliberately does NOT cover — measured, not assumed - * - * `driver-sql` serves three dialects, and only SQLite's wording for this - * condition has been measured (this container has no Postgres or MySQL server - * to raise the other two). Recognition is therefore SQLite-first by decision: - * a Postgres deployment still gets its raw error here, exactly as before, which - * is no worse than today and is a condition nobody has to guess at. Widening it - * — most likely as a dialect-spanning predicate in `@objectstack/types` beside - * `isUniqueViolationError`, whose vocabulary table is the shape it would take — - * is #8567, and it is gated on measuring the other two dialects' text rather - * than transcribing it from memory. - * - * ⚠️ Deliberately NOT `isUniqueViolationError`: that predicate answers the - * OPPOSITE condition (a unique index exists and the row violated it). - * Confusing the two would report a working constraint as a missing one. - * - * The twin lives in `driver-turso`'s `remote-transport.ts` - * ({@link https://github.com/objectstack-ai/objectstack/issues/8413}); the two - * faces state this recognition once each, for the reason that file's header - * gives — the remote transport is deliberately free of knex and of `SqlDriver`, - * so it cannot import this one. +/* + * [#8445 → #8567] `isUnbackedConflictTargetError` — "is this the conflict + * target is not a key failure?" — is imported from `@objectstack/types` + * (`unbacked-conflict-target.ts`), not written here. + * + * It began as a private regex on SQLite's sentence, because that was the only + * dialect #8445's container could raise the condition on and transcribing the + * other two from memory was ruled out as evidence. #8567 raised it on a real + * Postgres 16.13 through this same knex path and found a second, unrelated + * sentence — so the vocabulary is now per-dialect, which is the shape that + * belongs beside `isUniqueViolationError` rather than inside a driver. The + * measurements, the reason the `code` channel is unread on BOTH dialects + * (SQLite's is generic; Postgres' `42P10` also fires for an out-of-range + * `ORDER BY` position), and the reason MySQL has no limb at all are all + * recorded there. + * + * ⚠️ Deliberately NOT `isUniqueViolationError`, which is imported one line + * away: that predicate answers the OPPOSITE condition (a unique index exists + * and the row violated it). Confusing the two would report a working + * constraint as a missing one. */ -function isUnbackedConflictTargetError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error ?? ''); - return /ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint/i.test(message); -} /** * [#8445] A `conflictKeys` upsert whose target no unique index backs. @@ -1057,17 +1033,28 @@ function isUnbackedConflictTargetError(error: unknown): boolean { * that already holds duplicates (`syncDeclaredIndexes` degrades rather than * rewriting rows), so the same table arrives here with the same missing index. * - * ⚠️ The original error is kept as `cause` rather than discarded — the SQLite - * text is the ground truth an operator debugging the table wants, and nothing - * above this layer can recover it once replaced. It is ASSIGNED rather than - * passed to the `Error` constructor, matching the twin: the two-argument form - * needs the ES2022 `ErrorOptions` overload. + * [#8567] One clause moved, on BOTH faces in the same commit: "and SQLite + * refuses the statement" is now "and **the database** refuses the statement". + * This face serves three dialects, and once recognition covers Postgres a + * Postgres operator was being told SQLite had refused their statement — a + * sentence naming the wrong engine, which is worse than a vague one because it + * sends the reader to the wrong manual. #5240 still holds: one condition, one + * wording, so `driver-turso`'s copy moved with it and #8568's cross-face parity + * pin is what proves they moved together rather than drifting apart. + * + * ⚠️ The original error is kept as `cause` rather than discarded — the server's + * own text is the ground truth an operator debugging the table wants (SQLite's + * `ON CONFLICT clause does not match…`, Postgres' `there is no unique or + * exclusion constraint…`), and nothing above this layer can recover it once + * replaced. It is ASSIGNED rather than passed to the `Error` constructor, + * matching the twin: the two-argument form needs the ES2022 `ErrorOptions` + * overload. */ function refuseUnbackedConflictTarget(object: string, mergeKeys: string[], cause: unknown): Error { const keys = mergeKeys.map((k) => `"${k}"`).join(', '); const err = new Error( `Cannot upsert into "${object}" on conflict keys (${keys}): no PRIMARY KEY or UNIQUE index ` + - `backs them, so the merge target does not exist and SQLite refuses the statement. This is ` + + `backs them, so the merge target does not exist and the database refuses the statement. This is ` + `usually a table created before its "unique" declaration was emitted as DDL, or conflict ` + `keys naming columns that were never declared unique. Fix by declaring the column(s) ` + `"unique: true" and re-running schema sync so the unique index is created — if the table ` + @@ -5064,14 +5051,26 @@ export class SqlDriver implements IDataDriver { } catch (error) { // [#8445] Classified BEFORE the autonumber retry logic, for three // reasons that all point the same way. It is not an autonumber - // collision — `collidingAutoNumberReservations` asks - // `isUniqueViolationError`, which is false for this error, so the - // reservation probe would query the sequences table for nothing and - // then rethrow the raw error anyway. It is not transient — the same - // statement fails identically on every attempt, so a retry is a wasted - // round trip by construction. And it is the only placement that - // envelopes BOTH exits below: a refusal recognised after the branch - // would still escape raw whenever `mayRetry` is false. + // collision — `collidingAutoNumberReservations` gates on + // `isUniqueViolationError`, and this error is not a unique violation: + // no index existed for anything to violate. + // + // ⚠️ [#8567] That gate does NOT agree, and this comment used to claim it + // did ("which is false for this error"). Measured on the real SQLite + // error: `isUniqueViolationError` returns TRUE, because SQLite's + // missing-index sentence ends `…PRIMARY KEY or UNIQUE constraint` and + // the shared vocabulary matches the word pair `unique constraint` + // wherever it appears. Filed as #8590. So the ORDERING here is + // load-bearing rather than merely tidy: move this line below the branch + // and the reservation probe really does run — querying the sequences + // table for a collision that cannot exist, then rethrowing the raw + // error. Recognising first is what makes that unreachable. + // + // It is also not transient — the same statement fails identically on + // every attempt, so a retry is a wasted round trip by construction. And + // it is the only placement that envelopes BOTH exits below: a refusal + // recognised after the branch would still escape raw whenever + // `mayRetry` is false. if (isUnbackedConflictTargetError(error)) throw refuseUnbackedConflictTarget(object, mergeKeys, error); if (!mayRetry || attempt >= AUTONUMBER_COLLISION_RETRIES) throw error; const colliding = await this.collidingAutoNumberReservations(error, reservations, options); diff --git a/packages/drivers/driver-turso/src/remote-transport.ts b/packages/drivers/driver-turso/src/remote-transport.ts index aec60661b9..129161ef70 100644 --- a/packages/drivers/driver-turso/src/remote-transport.ts +++ b/packages/drivers/driver-turso/src/remote-transport.ts @@ -579,6 +579,17 @@ function invalidFilterError(message: string): Error { * predicate answers the OPPOSITE condition (a unique index exists and the row * violated it). Confusing the two would report a working constraint as a * missing one. + * + * [#8567] `@objectstack/types` now also exports the dialect-spanning + * `isUnbackedConflictTargetError` — the same question, with a measured Postgres + * limb beside this SQLite one — and `driver-sql` reads it from there. This copy + * deliberately stays: this package does not depend on `@objectstack/types` + * today (`@libsql/client`, `@objectstack/core`, `@objectstack/driver-sql`, + * `@objectstack/spec`, `nanoid`, `zod`), and adding a dependency edge to + * de-duplicate six lines that can never need the Postgres limb — this transport + * speaks libsql/SQLite and only ever will — buys nothing. If this package ever + * takes that dependency for another reason, delete this and import the shared + * one; the wording it feeds is already pinned across faces by #8568. */ function isUnbackedConflictTargetError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error ?? ''); @@ -616,6 +627,18 @@ function isUnbackedConflictTargetError(error: unknown): boolean { * list, so a caller's un-satisfiable upsert stops being logged as an unhandled * SERVER error once per request. * + * # [#8567] Why one clause of the wording moved + * + * "and SQLite refuses the statement" became "and **the database** refuses the + * statement". Nothing about this face changed — the remote transport speaks + * libsql/SQLite and only ever will. The clause moved because #5240 binds this + * refusal to `driver-sql`'s word for word, and THAT face serves Postgres and + * MySQL too: once its recognition covers Postgres, a sentence naming SQLite + * points a Postgres operator at the wrong engine. The two faces are held + * identical by #8568's parity pin, so the correct response to that pin + * reddening is to move both, which is what the same commit did — never to + * update the pin to accept a divergence. + * * ⚠️ The original error is kept as `cause` rather than discarded — the SQLite * text is the ground truth an operator debugging the table will want, and * nothing above this layer can recover it once it is replaced. It is ASSIGNED @@ -627,7 +650,7 @@ function refuseUnbackedConflictTarget(object: string, mergeKeys: string[], cause const keys = mergeKeys.map((k) => `"${k}"`).join(', '); const err = new Error( `Cannot upsert into "${object}" on conflict keys (${keys}): no PRIMARY KEY or UNIQUE index ` + - `backs them, so the merge target does not exist and SQLite refuses the statement. This is ` + + `backs them, so the merge target does not exist and the database refuses the statement. This is ` + `usually a table created before its "unique" declaration was emitted as DDL, or conflict ` + `keys naming columns that were never declared unique. Fix by declaring the column(s) ` + `"unique: true" and re-running schema sync so the unique index is created — if the table ` + diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 279704029f..32bf0759d6 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -23,6 +23,12 @@ export * from './relation-sub-object.js'; // Four hand-written vocabularies used to answer it and disagreed about MySQL, // which is why every MySQL conflict came back 500 instead of 409. export * from './unique-violation.js'; +// [#8567] The OPPOSITE question, kept deliberately separate: "is this the +// database refusing an ON CONFLICT target that no unique index backs?" One +// measured limb per dialect that can raise it (SQLite, Postgres); MySQL cannot, +// because knex compiles the conflict target away. Never merge the two — a +// merged predicate reports a working constraint as a missing one. +export * from './unbacked-conflict-target.js'; // [ADR-0120 D5e] The `isolated`-posture install gate for `'global'` uniques — // the pure enumerator both the hard stop (install seam) and the advisories // (`os doctor` / `os migrate plan`) read, so the three cannot drift apart. diff --git a/packages/types/src/unbacked-conflict-target.test.ts b/packages/types/src/unbacked-conflict-target.test.ts new file mode 100644 index 0000000000..22b28a0543 --- /dev/null +++ b/packages/types/src/unbacked-conflict-target.test.ts @@ -0,0 +1,272 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8567] `isUnbackedConflictTargetError` — the dialect-spanning "no unique + * index backs this `ON CONFLICT` target" predicate, and the pin that keeps it + * from ever becoming `isUniqueViolationError`. + * + * # Every recognised string here was measured, not composed + * + * The SQLite fixture is #8445's transcript; the Postgres fixtures are #8567's, + * read off the thrown `DatabaseError` from a real PostgreSQL 16.13 raised + * through knex 3.3.0 + pg 8.22.0 — the same path `SqlDriver.upsert` takes. The + * card that filed this work ruled that transcribing a dialect's wording from + * memory is not evidence, so a fixture nobody observed a server emit does not + * belong in this file. + * + * # The disjointness suite is the reason this file exists + * + * A boolean predicate is cheap to test and cheap to get catastrophically + * wrong in exactly one way: by answering the NEIGHBOURING question. + * `isUniqueViolationError` says "a unique index exists and the row violated + * it"; this one says "no unique index exists at all". Both fire while a caller + * is upserting, both are about uniqueness, and knex prefixes both with the same + * statement text — so a limb borrowed from one into the other produces a + * confident, plausible, inverted answer. + * + * The pins below therefore run every measured text through BOTH predicates and + * record both verdicts. A one-directional pin would miss the likelier drift: + * not this predicate growing a `duplicate entry` limb, but the unique-violation + * vocabulary growing an `ON CONFLICT` one, because that is the file people + * extend. + * + * ⚠️ Running it that way is what found **#8590**: on SQLite the separation is + * ALREADY broken in the pre-existing direction — `isUniqueViolationError` + * claims the unbacked-target error, because SQLite's missing-index sentence + * ends `…PRIMARY KEY or UNIQUE constraint` and that vocabulary matches the word + * pair `unique constraint` wherever it appears. Not fixed here (it moves + * verdicts in six packages); pinned as measured, per dialect, so the fix + * announces itself. See the suite below. + */ + +import { describe, expect, it } from 'vitest'; +import { isUnbackedConflictTargetError } from './unbacked-conflict-target.js'; +import { isUniqueViolationError } from './unique-violation.js'; + +/** + * The condition, as each dialect that can raise it actually words it. + * + * `knexPrefixed` is what a caller really catches: knex builds the message as + * STATEMENT + ` - ` + the server's sentence, so the recognised text is always a + * tail. `bare` is the same sentence as the server alone would give it — the + * shape a driver that does not wrap, or a caller that already unwrapped, hands + * over. Both must be recognised, or the predicate's verdict would depend on how + * many layers the error passed through. + */ +const UNBACKED = { + sqlite: { + label: 'sqlite (better-sqlite3, #8445)', + bare: 'ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint', + knexPrefixed: + 'insert into `crm_contact_plain` (`created_at`, `email`, `id`, `title`) values ' + + "('2026-08-13T00:00:00.000Z', 'a@b.com', 'ib21mSZ', 'x') on conflict (`email`) do " + + 'update set `email` = excluded.`email` - ON CONFLICT clause does not match any ' + + 'PRIMARY KEY or UNIQUE constraint', + code: 'SQLITE_ERROR', + }, + postgres: { + label: 'postgres 16.13 (pg 8.22.0, #8567)', + bare: 'there is no unique or exclusion constraint matching the ON CONFLICT specification', + knexPrefixed: + 'insert into "plain" ("email", "id", "title") values ($1, $2, $3) on conflict ' + + '("email") do update set "title" = excluded."title" - there is no unique or ' + + 'exclusion constraint matching the ON CONFLICT specification', + code: '42P10', + }, +} as const; + +/** + * The OPPOSITE condition, in each dialect's words — a unique index that EXISTS + * and was violated. Sourced from `unique-violation.ts`'s own measured table. + */ +const UNIQUE_VIOLATIONS = [ + ['sqlite', 'UNIQUE constraint failed: sys_user.email'], + ['postgres', 'duplicate key value violates unique constraint "sys_user_email_key"'], + ['mysql', "Duplicate entry 'acme@example.com' for key 'idx_email_unique'"], +] as const; + +describe('[#8567] isUnbackedConflictTargetError — the measured dialect vocabulary', () => { + for (const dialect of Object.values(UNBACKED)) { + it(`recognises ${dialect.label}, knex-prefixed`, () => { + expect(isUnbackedConflictTargetError(new Error(dialect.knexPrefixed))).toBe(true); + }); + + it(`recognises ${dialect.label}, bare server sentence`, () => { + expect(isUnbackedConflictTargetError(new Error(dialect.bare))).toBe(true); + }); + + it(`recognises ${dialect.label} as a plain string`, () => { + expect(isUnbackedConflictTargetError(dialect.bare)).toBe(true); + }); + } + + /** + * The condition is the same on both dialects, so a caller must not be able + * to tell them apart by the verdict — that is what "dialect-spanning" + * means, and it is the property #8445 could not have. + */ + it('answers the same on both dialects — Postgres is no longer the odd one out', () => { + const verdicts = Object.values(UNBACKED).map((d) => isUnbackedConflictTargetError(new Error(d.knexPrefixed))); + expect(verdicts).toEqual([true, true]); + }); +}); + +describe('[#8567] the `code` channel is deliberately unread — measured over-match', () => { + /** + * Postgres' `42P10` is `invalid_column_reference`, NOT "unbacked conflict + * target". Both messages below were raised on the same PG 16.13 cluster + * that produced the fixture above and carry the identical code. A predicate + * that read `code` would answer "add a unique index" to a caller whose + * actual mistake is an out-of-range sort position. + */ + const OTHER_42P10 = [ + ['ORDER BY position', 'select id from plain order by 7 - ORDER BY position 7 is not in select list'], + ['GROUP BY position', 'select id from plain group by 9 - GROUP BY position 9 is not in select list'], + ] as const; + + for (const [label, message] of OTHER_42P10) { + it(`does not claim a 42P10 raised by an out-of-range ${label}`, () => { + const err = Object.assign(new Error(message), { code: '42P10', severity: 'ERROR' }); + expect(isUnbackedConflictTargetError(err)).toBe(false); + }); + } + + /** + * The mirror: a code alone must not carry the verdict either. If someone + * later adds a `codes` set, this goes red — which is the intended alarm, + * not an obstacle. The reason lives in the module head. + */ + it('does not claim an error whose only unbacked-looking signal is `code: 42P10`', () => { + const err = Object.assign(new Error('something else went wrong'), { code: '42P10' }); + expect(isUnbackedConflictTargetError(err)).toBe(false); + }); + + /** + * SQLite's side of the same argument: the generic code it raises here is + * the code it raises for a missing table, which `driver-sql`'s suite + * already requires to come back as itself. + */ + it('does not claim an unrelated SQLITE_ERROR', () => { + const err = Object.assign(new Error('insert into `never_created` ... - no such table: never_created'), { + code: 'SQLITE_ERROR', + }); + expect(isUnbackedConflictTargetError(err)).toBe(false); + }); +}); + +describe('[#8567] ⚠️ separation from isUniqueViolationError — the inverse condition', () => { + /** + * ⚠️ This suite was written expecting clean disjointness in both + * directions. It went RED on the first run, and the measurement won: on + * SQLite, `isUniqueViolationError` ALREADY claims the unbacked-target + * error. Filed as **#8590**, deliberately not fixed here — narrowing that + * predicate moves verdicts in six consuming packages and needs its own + * measured pass. + * + * The cause is a superstring collision, not a judgement call. Its message + * limb is `/unique constraint|…/i`, and SQLite's sentence for the MISSING + * index ends `…any PRIMARY KEY or UNIQUE constraint` — the two words sit + * adjacent inside a sentence that says the constraint is absent. Postgres + * escapes only on word order (`unique or exclusion constraint` is not + * adjacent), which is the tell that a word pair is being matched rather + * than a condition. + * + * So the pins below record the state as MEASURED, per dialect, rather than + * as hoped. When #8590 lands, the SQLite row goes red and points straight + * at itself — which is the entire reason to pin a known defect instead of + * leaving the direction untested. + */ + const UNIQUE_VIOLATION_VERDICT_ON_UNBACKED: Record = { + // ⚠️ THE DEFECT (#8590). Correct value is `false`; flip it when #8590 lands. + sqlite: true, + // Correct today, and only by luck of word order — see above. + postgres: false, + }; + + for (const [key, dialect] of Object.entries(UNBACKED)) { + it(`${dialect.label}: this predicate claims it, and isUniqueViolationError's verdict is pinned as measured`, () => { + expect(isUnbackedConflictTargetError(new Error(dialect.knexPrefixed))).toBe(true); + expect( + isUniqueViolationError(new Error(dialect.knexPrefixed)), + 'if this changed, #8590 either landed (SQLite → false: delete the exception) or ' + + 'regressed (Postgres → true: a new limb is matching the missing-index sentence)', + ).toBe(UNIQUE_VIOLATION_VERDICT_ON_UNBACKED[key]); + }); + } + + /** + * The direction this card CAN break, and therefore the one that carries no + * exceptions: nothing that is a real unique violation may be claimed as an + * unbacked target. A false positive here sends an operator to create an + * index that already exists and is doing its job, while the duplicate that + * actually failed goes unexplained. + */ + for (const [dialect, message] of UNIQUE_VIOLATIONS) { + it(`${dialect}: a real unique violation is NOT an unbacked target`, () => { + expect(isUniqueViolationError(new Error(message))).toBe(true); + // If this flips, an operator is sent to create an index that + // already exists and is doing its job. + expect(isUnbackedConflictTargetError(new Error(message))).toBe(false); + }); + } + + /** + * The knex-prefixed unique violation is the sharpest case: its statement + * text contains the literal words `on conflict`, so a predicate loosened to + * match the clause rather than the server's sentence would claim it — and + * would invert the answer on the one input where the two questions are + * hardest to tell apart by eye. + */ + it('does not claim a unique violation that arrived with `on conflict` in its statement text', () => { + const message = + 'insert into `crm_contact` (`email`, `id`) values (?, ?) on conflict (`email`) do update ' + + 'set `email` = excluded.`email` - UNIQUE constraint failed: crm_contact.email'; + + expect(isUniqueViolationError(new Error(message))).toBe(true); + expect(isUnbackedConflictTargetError(new Error(message))).toBe(false); + }); +}); + +describe('[#8567] the `cause` chain and non-error inputs', () => { + it('follows a wrapped driver error down the cause chain', () => { + const raw = new Error(UNBACKED.postgres.knexPrefixed); + const wrapped = Object.assign(new Error('upsert failed'), { cause: raw }); + expect(isUnbackedConflictTargetError(wrapped)).toBe(true); + }); + + /** + * The driver's own refusal keeps the raw error as `cause`, so re-asking the + * predicate about a refusal already built still answers `true`. That is the + * correct answer — the refusal is not a different condition — and pinning it + * documents that a second envelope pass cannot mislabel it. + */ + it('still recognises the condition through the refusal that already enveloped it', () => { + const refusal = Object.assign( + new Error('Cannot upsert into "crm_contact_plain" on conflict keys ("email"): no PRIMARY KEY or UNIQUE index backs them.'), + { code: 'VALIDATION_ERROR', status: 400, cause: new Error(UNBACKED.sqlite.knexPrefixed) }, + ); + expect(isUnbackedConflictTargetError(refusal)).toBe(true); + }); + + it('stops following the cause chain rather than recursing without bound', () => { + const nest = (depth: number): Error => { + let err = new Error(UNBACKED.sqlite.bare); + for (let i = 0; i < depth; i++) err = Object.assign(new Error(`wrap ${i}`), { cause: err }); + return err; + }; + expect(isUnbackedConflictTargetError(nest(4))).toBe(true); + expect(isUnbackedConflictTargetError(nest(9))).toBe(false); + }); + + it('never throws on a self-referential cause', () => { + const err = new Error('wrapped') as Error & { cause?: unknown }; + err.cause = err; + expect(isUnbackedConflictTargetError(err)).toBe(false); + }); + + for (const value of [null, undefined, 0, 42, true, false, {}, [], new Error('boom')]) { + it(`is false for ${JSON.stringify(value) ?? String(value)}`, () => { + expect(isUnbackedConflictTargetError(value)).toBe(false); + }); + } +}); diff --git a/packages/types/src/unbacked-conflict-target.ts b/packages/types/src/unbacked-conflict-target.ts new file mode 100644 index 0000000000..fc1ac6dbf8 --- /dev/null +++ b/packages/types/src/unbacked-conflict-target.ts @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The one named predicate for "did the database refuse this `ON CONFLICT` + * target because no PRIMARY KEY or UNIQUE index backs it?" (#8567). + * + * ## ⚠️ This is NOT `isUniqueViolationError` — it is the OPPOSITE condition + * + * Read this before touching either predicate. They are one file apart and one + * word apart in English, and they answer inverse questions: + * + * | predicate | the index | the row | + * |:---|:---|:---| + * | {@link isUniqueViolationError} | **exists** | violated it | + * | `isUnbackedConflictTargetError` | **does not exist** | never got compared | + * + * Merging them — or reaching for whichever one autocomplete offers — reports a + * *working* constraint as a missing one, which sends an operator to add an + * index that is already there while the real duplicate goes unexplained. The + * warning is repeated at both call sites and in `unique-violation.ts` because + * it is the most expensive mistake available anywhere near this question. + * + * ⚠️ The separation is **not clean today, in the pre-existing direction**, and + * pinning it is what found that: `isUniqueViolationError` claims SQLite's + * unbacked-target error, because that sentence ends `…PRIMARY KEY or UNIQUE + * constraint` and its vocabulary matches the word pair `unique constraint` + * wherever it appears — including inside a sentence saying the constraint is + * ABSENT. Filed as #8590; not fixed by #8567, which would have moved verdicts + * in six consuming packages on a card that measured a different question. + * `unbacked-conflict-target.test.ts` records both predicates' verdicts on every + * measured text, per dialect, so neither the fix nor a fresh drift can land + * silently. Nothing below may take a limb from that vocabulary, or give one to + * it, while #8590 is open. + * + * ## What each dialect actually says — measured, never transcribed + * + * #8445 landed this recognition for SQLite alone and said so: the container + * that implemented it had no other server, and transcribing another dialect's + * wording from memory was ruled out as evidence. #8567 raised the condition on + * a real Postgres 16.13 (system PG16 binaries, `initdb` + `pg_ctl`, no + * container runtime) through the same knex + `pg` path `SqlDriver.upsert` + * uses, and read the fields off the thrown error object: + * + * ``` + * # POSTGRES 16.13, knex 3.3.0 + pg 8.22.0 + * upsert('plain', { email: 'a@b.com', title: 'x' }, ['email']) + * -> name=error (DatabaseError) code=42P10 severity=ERROR status=undefined + * routine=infer_arbiter_indexes constraint=undefined detail=undefined + * msg=insert into "plain" ("email", "id", "title") values ($1, $2, $3) + * on conflict ("email") do update set "title" = excluded."title" + * - there is no unique or exclusion constraint matching the ON CONFLICT specification + * + * # SQLITE 3.x, knex 3.3.0 + better-sqlite3 (#8445's measurement, unchanged) + * upsert('plain', { email: 'a@b.com', title: 'x' }, ['email']) + * -> name=SqliteError code=SQLITE_ERROR status=undefined + * msg=insert into `plain` (...) values (...) on conflict (`email`) do update set ... + * - ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint + * ``` + * + * Two dialects, two unrelated sentences, and the same envelope from knex: the + * STATEMENT, then ` - `, then the server's own text. That tail is what both + * limbs below are anchored on, so a knex-prefixed message and a bare driver + * message are recognised identically. + * + * ## Why the `code` channel is unused — also measured + * + * The obvious predicate is `code === '42P10'`, and it is wrong in both + * directions: + * + * - **SQLite has no code to read.** It answers plain `SQLITE_ERROR`, the same + * generic code a syntax error or a missing table carries. `driver-sql`'s + * own suite pins that: an upsert against a table that was never created + * must come back as itself, and it is a `SQLITE_ERROR` too. + * - **Postgres' code OVER-matches.** `42P10` is `invalid_column_reference`, + * not "unbacked conflict target". Measured on the same cluster, same + * session: + * + * ``` + * select id from plain order by 7 -> code=42P10 "ORDER BY position 7 is not in select list" + * select id from plain group by 9 -> code=42P10 "GROUP BY position 9 is not in select list" + * ``` + * + * A code-only limb would answer `VALIDATION_ERROR` "no unique index backs + * your conflict keys" to a caller whose real defect is an out-of-range sort + * position — a refusal pointing at the wrong thing entirely. So the message + * is not a fallback for a missing code here; it is the only channel that + * identifies the condition, on both dialects, and the code channel is + * deliberately left unread rather than ANDed in for a narrowing it does not + * provide. + * + * The Postgres limb is safe to match on prose because the sentence has exactly + * one source: `infer_arbiter_indexes` (`plancat.c`), reached only while + * planning an `ON CONFLICT` inference, which the measured `routine` field + * confirms. The SQLite limb has the same property, stated at #8445. + * + * ## MySQL: the condition cannot arise, and that is measured too + * + * MySQL has no `ON CONFLICT` syntax. knex compiles the driver's exact call to + * `ON DUPLICATE KEY UPDATE`, which takes **no conflict target** — the named + * keys are dropped from the statement before it leaves the process, so the + * server is never asked to find an index for them and cannot complain that + * none exists. Compiled with knex 3.3.0 on the `mysql2` dialect, no server + * needed (`.toSQL()`), and pinned by + * `sql-driver-upsert-conflict-target-dialects.test.ts`: + * + * ``` + * knex('plain').insert({...}).onConflict(['email']).merge(['title']).toSQL() + * mysql2 -> insert into `plain` (`email`, `id`, `title`) values (?, ?, ?) + * on duplicate key update `title` = values(`title`) ← no `email` target + * pg -> insert into "plain" (...) values ($1, $2, $3) + * on conflict ("email") do update set "title" = excluded."title" + * ``` + * + * So there is no MySQL limb to write, and its absence is a finding rather than + * a gap. ⚠️ What MySQL does *instead* — merge on whichever unique key the row + * happens to collide with, or insert a second row — is a different defect with + * a different fix, and is NOT this predicate's business. + * + * ## Home + * + * `@objectstack/types`, beside {@link isUniqueViolationError}, for the reason + * that module records: every consumer of the question already depends on this + * package, so naming it here never adds an edge, and this module deliberately + * imports nothing. The alternative — a second private regex in each driver + * that meets the condition — is exactly the state `unique-violation.ts` was + * written to retire, where four hand-written vocabularies disagreed about + * MySQL and nobody could see it. + */ + +/** + * One dialect vocabulary for this condition, in the channel that carries it. + * + * Deliberately **message-only**, unlike `UniqueViolationSignature`'s + * three-channel table — the module head records the measurements: SQLite's + * `code` is the generic `SQLITE_ERROR`, and Postgres' `42P10` is + * `invalid_column_reference`, which an out-of-range `ORDER BY` position also + * raises. Neither channel narrows anything, and a `codes` set standing empty + * beside them would read as "nobody has filled this in yet" rather than as the + * decision it is. + */ +interface UnbackedConflictTargetSignature { + /** + * `error.message` — matched on the server's own sentence, which knex leaves + * as the tail after the statement and ` - `. + */ + readonly message: RegExp; +} + +/** + * Every wording measured for this condition, one limb per dialect that can + * raise it. Nothing here is inferred: each limb was read off a thrown error + * object, and the transcript is in the module head above. + * + * - SQLite: `ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE + * constraint` — stable since `ON CONFLICT` arrived in 3.24 (#8445). + * - Postgres: `there is no unique or exclusion constraint matching the ON + * CONFLICT specification` — `infer_arbiter_indexes`, PG 16.13 (#8567). + * + * Deliberately NOT here: any limb for MySQL (the condition cannot reach the + * server — see the module head), and any bare `ON CONFLICT` fragment. A limb + * loose enough to match `on conflict` alone would match the driver's own + * *statement* text, which knex prefixes onto every upsert failure — including + * a unique violation, which is the opposite condition. + */ +const UNBACKED_CONFLICT_TARGET: UnbackedConflictTargetSignature = { + message: + /ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint|there is no unique or exclusion constraint matching the ON CONFLICT specification/i, +}; + +/** How far to follow an `error.cause` chain — drivers wrap, but not deeply. */ +const MAX_CAUSE_DEPTH = 4; + +/** + * Whether a thrown driver error says the `ON CONFLICT` target it was given is + * backed by no PRIMARY KEY or UNIQUE index. + * + * Reads the message channel, then one step at a time down the `cause` chain — + * pool and query-builder layers re-throw with the original attached, and the + * refusal this predicate gates keeps the raw error as its own `cause`. A plain + * string is judged directly, so a caller that already unwrapped `err.message` + * can pass it in. + * + * **Unrecognised is always `false`.** A false positive is the expensive + * direction: it tells a caller to go add an index when the real failure was a + * syntax error, a missing table, or — worst — a genuine unique violation on an + * index that exists and works. A false negative costs only the raw error that + * was the status quo before recognition existed. + * + * @param error - the thrown value, of any shape. + * + * @example + * ```ts + * catch (error) { + * // ⚠️ NOT isUniqueViolationError — that is the opposite condition. + * if (isUnbackedConflictTargetError(error)) throw refuseUnbackedConflictTarget(object, keys, error); + * throw error; + * } + * ``` + */ +export function isUnbackedConflictTargetError(error: unknown): boolean { + return matchesUnbackedConflictTarget(error, 0); +} + +function matchesUnbackedConflictTarget(error: unknown, depth: number): boolean { + if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return false; + + if (typeof error === 'string') return UNBACKED_CONFLICT_TARGET.message.test(error); + if (typeof error !== 'object') return false; + + const err = error as { message?: unknown; cause?: unknown }; + + if (typeof err.message === 'string' && UNBACKED_CONFLICT_TARGET.message.test(err.message)) return true; + + return matchesUnbackedConflictTarget(err.cause, depth + 1); +} diff --git a/packages/types/src/unique-violation.ts b/packages/types/src/unique-violation.ts index 00abcfe516..662d6890e2 100644 --- a/packages/types/src/unique-violation.ts +++ b/packages/types/src/unique-violation.ts @@ -63,6 +63,28 @@ * 2026-08-08 ruling. Read its doc comment before touching either — the two are * gated on each other and the column answer is deliberately narrower than the * boolean. + * + * ## ⚠️ The INVERSE question lives next door — do not merge them + * + * `isUnbackedConflictTargetError` (`unbacked-conflict-target.ts`, #8567) asks + * whether the database refused an `ON CONFLICT` target because **no unique + * index exists** for it. This predicate asks whether one **exists and was + * violated**. Same neighbourhood, same vocabulary, inverse verdicts: + * answering an unbacked target with a 409 `UNIQUE_VIOLATION` tells the client + * to change a value when nothing collided, and answering a real conflict with + * "add a unique index" sends an operator after an index that is already there. + * Neither predicate may grow a limb belonging to the other. + * + * ⚠️ This predicate is ALREADY on the wrong side of that line for one dialect: + * `message`'s `unique constraint` limb matches SQLite's *missing*-index + * sentence, which ends `…any PRIMARY KEY or UNIQUE constraint`, so an unbacked + * conflict target is reported here as a violation of a constraint that does not + * exist. Measured on the real driver error and filed as **#8590** — read it + * before touching `UNIQUE_VIOLATION.message`, because the naive narrowing also + * drops Postgres' `violates unique constraint "..."`, which this limb has + * covered since it was inherited verbatim from the REST branch it replaced. + * `unbacked-conflict-target.test.ts` pins both predicates' verdicts per dialect + * so the fix cannot land silently in either direction. */ /**