fix(drivers): recognise the unbacked conflict target on Postgres too, from a measured PG 16.13 (#8567)

.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.

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.',
  );
  });
 

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);

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 ` +

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.