Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions .changeset/unique-violation-absence-sentence-superstring.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
---
"@objectstack/types": patch
---

fix(types): `isUniqueViolationError` stops claiming the sentences that say a unique constraint is ABSENT (#8590)

The shared predicate's message limb was a bare `unique constraint`, and a word
pair is not a condition. Every dialect that can say "this row violated a unique
constraint" can also say "there is no unique constraint here", and the same two
words sit adjacent in both — so the predicate answered **true** for errors
meaning the exact opposite of what it detects. `rest-server.ts` maps that
verdict to `409 UNIQUE_VIOLATION`, which tells a client to change a value when
nothing was ever compared, on a status an SDK will not retry.

**Measured on live servers for this fix, all three supported dialect families**
— SQLite via better-sqlite3, PostgreSQL 16.13 via `pg` 8.22.0, MariaDB 10.11.14
via `mysql2` 3.23.1, all through knex 3.3.0 — driving each dialect through both
conditions plus the NOT NULL / FOREIGN KEY near misses:

```
sqlite ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint
-> was true, WRONG (the reported defect, #8590)
postgres there is no unique constraint matching given keys for referenced table "t"
-> was true, WRONG (42830 — found by this fix's dialect sweep)
postgres there is no unique or exclusion constraint matching the ON CONFLICT specification
-> false (the pair is not adjacent here)
mysql the condition cannot arise: knex compiles to ON DUPLICATE KEY UPDATE,
which carries no conflict target (confirmed against a live server)
```

**Postgres was not clean either, and that chose the fix.** #8590 was filed
reading the collision as SQLite-only, with Postgres escaping "by luck of word
order". The sweep raised **42830** — a `FOREIGN KEY` referencing a non-unique
column — where Postgres puts `unique constraint` adjacent in its own absence
sentence. The card offered two candidate fixes; only one survives 42830. A
negative lookahead on SQLite's missing-index sentence is a blocklist that can
only enumerate absence sentences somebody already tripped over, and it answers
`true` on 42830. So the limb now requires a **violation phrasing** —
`unique constraint failed` (SQLite) or `violates unique constraint` (Postgres) —
which restores the module's own stated default, *unrecognised is `false`*, to
the message channel.

**Both spellings the retired limb covered are preserved exactly**, which was the
constraint on the fix: the limb was inherited verbatim from the REST branch
#6250 replaced and covered SQLite's `UNIQUE constraint failed: t.c` *and*
Postgres' `... violates unique constraint "..."`. The `unique violation`,
`duplicate key` and `duplicate entry` limbs are untouched, as are the `code` and
`errno` channels — MySQL's `Duplicate entry` path never went through the
narrowed limb at all.

**No user-visible behaviour changes today; this closes a latent inversion.** The
one site compiling a caller-supplied conflict target (`SqlDriver.upsert`)
recognises the unbacked target *first* in its catch and throws a refusal
declaring `status: 400`, and `mapDataError` reads `declaredHttpStatus` before it
reaches the unique-violation branch — so the 409 was gated off the wire by
ordering, not by the verdict. That ordering was the only thing standing between
this and a wrong status, which is why the verdict is now pinned rather than left
to it. A repo-wide scan of every string literal whose verdict moves found no
consumer relying on the old answer: all of them are prose, a different
predicate's vocabulary (`looksLikeInternalErrorLeak` keeps its own list), or
fixtures asserted through the status-passthrough path.

`unbacked-conflict-target.test.ts`'s pin — written by #8567 to point at itself
rather than go quietly green — is **inverted, not deleted**, and
`unique-violation-absence-sentences.test.ts` pins the absence sentences per
dialect in both directions, including the code channel, so re-reading `code`
cannot undo the message-side fix from the other side.
69 changes: 41 additions & 28 deletions packages/types/src/unbacked-conflict-target.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,13 +30,16 @@
* 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.
* ⚠️ Running it that way is what found **#8590**: on SQLite the separation was
* broken in the pre-existing direction — `isUniqueViolationError` claimed the
* unbacked-target error, because SQLite's missing-index sentence ends
* `…PRIMARY KEY or UNIQUE constraint` and that vocabulary matched the word pair
* `unique constraint` wherever it appeared. #8567 pinned it as measured rather
* than fixing it (the fix moves verdicts in six consuming packages); **#8590
* has since closed it** by requiring a violation phrasing in that limb, and the
* pin below was inverted rather than deleted — which is what a pin written to
* point at itself is for. The separation is now clean on both dialects, in both
* directions, and the suite below is what keeps it that way.
*/

import { describe, expect, it } from 'vitest';
Expand DownExpand Up@@ -157,29 +160,38 @@ describe('[#8567] the `code` channel is deliberately unread — measured over-ma
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.
* directions. It went RED on the first run and the measurement won: on
* SQLite, `isUniqueViolationError` claimed the unbacked-target error.
* #8567 filed that as **#8590** and pinned the wrong verdict as measured
* rather than fixing it, because narrowing that predicate moves verdicts in
* six consuming packages and needed 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.
* **#8590 has since landed, and this pin was INVERTED — that is the pin
* working, not an obstacle to route around.** The cause was a superstring
* collision, not a judgement call: the limb was a bare `unique constraint`,
* and SQLite's sentence for the MISSING index ends `…any PRIMARY KEY or
* UNIQUE constraint`, so the two words sit adjacent inside a sentence that
* says the constraint is ABSENT. The limb now requires a violation
* phrasing (`unique constraint failed` / `violates unique constraint`), so
* mentioning a unique constraint is no longer enough to be claimed as one.
*
* 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.
* ⚠️ Postgres was believed to escape "by luck of word order" — its
* `unique or exclusion constraint` is not adjacent. That reading was too
* kind: #8590's own dialect sweep raised PG 42830,
* `there is no unique constraint matching given keys for referenced table`,
* where Postgres puts the pair adjacent in its own ABSENCE sentence. Both
* dialects had the collision; only SQLite's instance was on the path this
* file measures. The absence sentences are pinned per dialect in
* `unique-violation-absence-sentences.test.ts`.
*
* Both rows are therefore `false` now, and the map is kept per dialect
* rather than collapsed to a constant so a regression names the dialect it
* came back on.
*/
const UNIQUE_VIOLATION_VERDICT_ON_UNBACKED: Record<string, boolean> = {
// ⚠️ 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.
// [#8590] Was `true` — the defect. Inverted when the fix landed.
sqlite: false,
// Correct before #8590 on this sentence, and now correct by rule.
postgres: false,
};

Expand All@@ -188,8 +200,9 @@ describe('[#8567] ⚠️ separation from isUniqueViolationError — the inverse
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)',
'both dialects are `false` since #8590. A `true` here means the unique-violation ' +
'vocabulary has regrown a limb that matches a sentence saying the constraint is ' +
'ABSENT — the superstring collision #8590 closed, back on this dialect',
).toBe(UNIQUE_VIOLATION_VERDICT_ON_UNBACKED[key]);
});
}
Expand Down
39 changes: 30 additions & 9 deletions packages/types/src/unbacked-conflict-target.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,17 +20,38 @@
* 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
* ⛔ **Nothing below may take a limb from that vocabulary, or give one to it.**
* Unconditional, and permanent: the two predicates answer inverse questions, so
* a limb that travels between them produces a confident inverted answer. This
* prohibition was once written as holding "while #8590 is open", which was
* wrong twice over — it reads as expiring, and #8590 has since closed.
*
* ⚠️ The separation **was** broken in the pre-existing direction, and pinning
* it is what found that: `isUniqueViolationError` claimed 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.
* constraint` and its vocabulary matched the word pair `unique constraint`
* wherever it appeared — including inside a sentence saying the constraint is
* ABSENT. #8567 filed that as #8590 and pinned it rather than fixing it, which
* would have moved verdicts in six consuming packages on a card that measured a
* different question. **#8590 has since closed it**: that predicate's message
* limb now requires a VIOLATION phrasing — `unique constraint failed` (SQLite)
* or `violates unique constraint` (Postgres) — so merely mentioning a unique
* constraint no longer answers yes.
*
* ⚠️ Postgres was believed to escape that collision "by luck of word order",
* its `unique or exclusion constraint` not being adjacent. #8590's dialect
* sweep disproved it: PG **42830**, `there is no unique constraint matching
* given keys for referenced table "t"` — a FOREIGN KEY referencing a non-unique
* column — puts the pair adjacent in Postgres' own ABSENCE sentence. Both
* dialects had the collision; only SQLite's instance sat on the path this file
* measures. That is why the fix is an allowlist of violation phrasings and not
* a negative lookahead on SQLite's sentence, which would still answer `true`
* there.
*
* `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.
* measured text, per dialect, and `unique-violation-absence-sentences.test.ts`
* pins the absence sentences on both sides — so neither a fix nor a fresh drift
* can land silently in either direction.
*
* ## What each dialect actually says — measured, never transcribed
*
Expand Down
Loading
Loading