Skip to content

types: isUniqueViolationError claims SQLite's UNBACKED-conflict-target error — a superstring collision on "UNIQUE constraint", the exact inversion the predicate's own warning names #8590

Description

@os-zhuang

Found while implementing #8567 (measuring the unbacked-conflict-target condition across dialects). Filed unassigned; not fixed there — the fix moves a shared predicate's verdicts, which is its own blast radius.

The defect

isUniqueViolationError (packages/types/src/unique-violation.ts) answers true for the error SQLite raises when an ON CONFLICT target has no backing unique index — i.e. for the exact condition its own docstring warns it must never claim:

⚠️ It must stay a separate predicate: isUniqueViolationError answers the OPPOSITE condition (a unique index exists and the row violated it), and merging them would report a working constraint as a missing one.

Measured on real driver errors (not fixtures), knex 3.3.0, @objectstack/types built from a3b1d264f:

===== sqlite (better-sqlite3) =====
UNBACKED-target error:
msg = "insert into `xtalk_plain` (`email`, `id`, `title`) values ('a@b.com', '1', 'x')
on conflict (`email`) do update set `title` = excluded.`title`
- ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint"
isUnbackedConflictTargetError = true
isUniqueViolationError = true <== WRONG, should be false
===== pg (PostgreSQL 16.13) =====
UNBACKED-target error:
msg = "insert into "xtalk_plain" (...) 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"
isUnbackedConflictTargetError = true
isUniqueViolationError = false <== correct

Cause

The vocabulary's message limb is

message: /uniqueconstraint|uniqueviolation|duplicatekey|duplicateentry/i

and SQLite's sentence for the missing-index condition ends ...any PRIMARY KEY or UNIQUE constraint. The words UNIQUE constraint sit adjacent inside it, so the unique constraint limb matches a sentence that says the constraint is absent.

Postgres escapes only by luck of word order: its unique or exclusion constraint is not adjacent, so the same limb misses it. That asymmetry is the tell — the limb is matching a word pair, not a condition.

This is the same bug class relation-sub-object.ts already documents for a different phrase pair: "Postgres' missing-COLUMN spelling contains a legal missing-TABLE phrase as a substring. Three packages had each repaired that superstring hole separately." Same shape, different vocabulary, and the one that has a named module was fixed centrally.

Reachability today: latent, not live — and worth fixing anyway

SqlDriver.upsert is the only site in driver-sql that compiles a caller-supplied conflict target (grep -n "onConflict(" returns exactly one hit), and #8445's recognition runs first in that catch, so the raw error is enveloped before any unique-violation consumer sees it. The remote face does the same (#8413). So no user-visible 409-instead-of-400 is reachable through the upsert path as it stands.

Two reasons that is not a close:

  1. A comment in sql-driver.ts states the opposite as fact and reasons from it. At the upsert catch:

    // 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 true for this error, measured above. The ordering is therefore load-bearing in a way the comment says it is not: move the recognition after the retry branch and the reservation probe really does run. (drivers(sql): the unbacked-conflict-target refusal is SQLite-only — Postgres and MySQL still answer the raw driver error #8567 corrects the comment in place; it does not touch the predicate.)

  2. Six other packages consume the predicate — rest-server.ts (the 409 UNIQUE_VIOLATION mapping), objectql/engine.ts, rest/import-runner.ts, service-messaging, metadata-protocol/partial-index-probe.ts. Any of them reached by a raw unbacked-target error gets told a value collided when nothing was ever compared, and 409 is a status an SDK will not retry.

What a fix has to be careful about

Narrowing the unique constraint limb changes verdicts of a predicate whose whole point (#6250) was that four divergent copies disagreed. The limb is inherited verbatim from the REST branch it replaced and covers both SQLite's UNIQUE constraint failed: t.c and Postgres' violates unique constraint "...", so it cannot simply be deleted. A negative-lookahead on the missing-index sentence, or requiring the limb to match a violation phrasing (constraint failed, violates), are both plausible — this needs the same measured-per-dialect treatment #8567 gave the other predicate, including MySQL's Duplicate entry path, before anything is changed.

packages/types/src/unbacked-conflict-target.test.ts (landing with #8567) already pins the current behaviour in both directions and names this issue, so whichever way the fix goes, the pin points at itself rather than going quietly green.

Related: #8567 (where this was measured), #6250 / #6543 (the predicate's consolidation), #6615 (the same superstring bug class, already fixed once for a different phrase pair), #8445 / #8413 (the two faces whose ordering currently masks it).

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions