fix(driver-sql): judge unique violations with the shared predicate — and the Postgres hole it uncovered (#6543)

.changeset/driver-sql-unique-violation-predicate.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/driver-sql": patch
+---
+
+fix(driver-sql): judge unique violations with the shared predicate, so a Postgres index build over dirty data no longer takes the boot down (#6543)
+
+`syncDeclaredIndexes` has a branch whose whole job is to keep a database
+BOOTING when existing rows violate a NULL-safe unique it was asked to create
+(the #5030 defect made data): the constraint is logged at `error` as not
+enforced, and the ADR-0120 D4 drift pre-flight reports the exact conflicting
+rows. Taking the process down instead would brick the deployment.
+
+It decided whether it was looking at that case with a private inline regex over
+the stringified message — `unique constraint failed|duplicate entry|duplicate
+key value`, the fourth hand-written spelling of this question #6250
+inventoried. That read one of the two channels drivers use, and on the DDL path
+the missing channel is the whole answer for one shipped dialect:
+
+| dialect | `CREATE UNIQUE INDEX` over duplicate rows says | old regex |
+|:---|:---|:---|
+| SQLite   | `UNIQUE constraint failed: product.code`                | matched |
+| MySQL    | `ER_DUP_ENTRY: Duplicate entry 'DUP' for key 'uniq_…'`  | matched |
+| Postgres | `could not create unique index "uniq_…"`, SQLSTATE 23505 | **missed** |
+
+Postgres does not reuse its DML phrasing for an index build: `duplicate key
+value violates unique constraint` is what a conflicting INSERT says, while a
+conflicting index BUILD says `could not create unique index "…"` and puts the
+verdict on `error.code` (SQLSTATE `23505`) with the offending tuple on
+`error.detail`. None of the three message limbs appear in it — so on Postgres
+the branch never fired, and a database with legacy duplicates failed to start
+rather than booting with the constraint reported as unenforced.
+
+Both discriminators in this file now call `isUniqueViolationError` from
+`@objectstack/types`, passing the **error object** rather than a pre-stringified
+message, so `code`, `errno` and the `cause` chain are read alongside `message`:
+
+- the #5030 boot-survival branch above;
+- the negative limb of the MySQL functional-key-part fallback in
+  `createNullSafeUniqueIndex`, which used a bare `/duplicate/i` to avoid
+  degrading a conflict into a "this server rejects functional key parts"
+  verdict — a message-only exclusion that did not fire on the `errno`-only
+  shape mysql2 can hand back.
+
+`patch` rather than `minor`: no API changes, and the message spellings that
+were recognised before are a strict subset of what the predicate recognises, so
+nothing that was absorbed before is absorbed differently now. The site's own
+business logic — the `nullSafe.size > 0` guard that keeps this absorption
+scoped to the NULL-safe case, and the "already exists" race arm that runs ahead
+of it — is unchanged.

packages/drivers/driver-sql/src/sql-driver-unique-violation-predicate.test.ts

@@ -0,0 +1,272 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `syncDeclaredIndexes` judges "did existing rows violate the NULL-safe unique
+ * I just tried to create?" — the #5030 branch that keeps a dirty database
+ * BOOTING (the constraint is logged as not-enforced and reported by the
+ * ADR-0120 D4 drift pre-flight) instead of taking the process down.
+ *
+ * It used to judge that with a private inline regex over the stringified
+ * message — `unique constraint failed|duplicate entry|duplicate key value` —
+ * the fourth hand-written vocabulary #6250 inventoried. #6543 migrates it onto
+ * `@objectstack/types`' `isUniqueViolationError`, passing the ERROR OBJECT so
+ * the `code` / `errno` channels are read at all.
+ *
+ * ## Why this is a live defect and not only a structural one
+ *
+ * The issue graded the migration `finding`, on the reasoning that "on the three
+ * dialects the repo ships, the message channel happens to carry the words".
+ * That holds for the DML path (a duplicate INSERT), which is what this
+ * package's other tests exercise. It does not hold for the DDL path this
+ * branch is in:
+ *
+ * | dialect | `CREATE UNIQUE INDEX` over duplicate rows says | old regex |
+ * |:---|:---|:---|
+ * | SQLite   | `UNIQUE constraint failed: product.code`                | ✅ matched |
+ * | MySQL    | `ER_DUP_ENTRY: Duplicate entry 'DUP' for key 'uniq_…'`  | ✅ matched |
+ * | Postgres | `could not create unique index "uniq_…"`, SQLSTATE 23505 | ❌ **missed** |
+ *
+ * Postgres does not reuse its DML phrasing here: `duplicate key value violates
+ * unique constraint` is what a conflicting INSERT says, while a conflicting
+ * index BUILD says `could not create unique index "…"` and carries the verdict
+ * on `error.code` (SQLSTATE `23505`, `ERRCODE_UNIQUE_VIOLATION`) with the
+ * offending tuple on `error.detail`. None of the three old message limbs
+ * appear in it — so on Postgres, the one dialect where the boot-survival
+ * branch was needed most, it never fired and `throw e` took the boot down.
+ * Postgres is a first-class shipped dialect for this package
+ * (`description: "… Supports PostgreSQL, MySQL, SQLite via Knex"`).
+ *
+ * The failures are injected rather than driven through a live Postgres because
+ * this package's unit suite boots SQLite only; the shapes below are the wire
+ * shapes `pg`/`mysql2` hand knex, message prefix included.
+ *
+ * ## Why this package's OTHER tests keep their own spelling
+ *
+ * `sql-driver-schema.test.ts`, `sql-driver-unique-tenancy.test.ts` and
+ * `adr0120-three-posture-conformance.test.ts` assert on
+ * `/UNIQUE constraint failed|duplicate key value/`. #6543 asked for a decision
+ * on those rather than leaving them to the next reader. **They stay as they
+ * are, deliberately.**
+ *
+ * They are not discriminators — they are assertions on what a real driver
+ * actually emitted when a real duplicate INSERT was refused, and their job is
+ * to prove the constraint EXISTS in the database. Routing them through
+ * `isUniqueViolationError` would make them strictly weaker in two ways:
+ *
+ *  1. The predicate is deliberately broad (four message limbs, three codes, an
+ *     errno, and a `cause` walk). An assertion through it can no longer
+ *     distinguish "SQLite refused this row on a unique index" from "some error
+ *     the predicate happens to accept", which is the whole content of those
+ *     tests.
+ *  2. A test that judges with the same predicate the production path judges
+ *     with shares that predicate's blind spots — the two stop being
+ *     independent, and a wrong predicate passes its own tests. That
+ *     independence is exactly what caught the Postgres hole above.
+ *
+ * The narrow spelling is therefore the right one THERE, and the shared
+ * predicate the right one in `src/sql-driver.ts`. The rule that reconciles
+ * them: **judge with the predicate, assert on the literal.**
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+import type { DeclaredIndexInput } from '../src/index.js';
+
+/** The NULL-safe unique of the #5030 scenario: `COALESCE(organization_id), code`. */
+const NULL_SAFE_INDEX: DeclaredIndexInput = {
+  name: 'uniq_product_organization_id_code',
+  fields: ['organization_id', 'code'],
+  unique: 'organization',
+  nullSafeColumns: ['organization_id'],
+};
+
+/** The same index with no NULL-safe key part — the `nullSafe.size > 0` guard's false arm. */
+const PLAIN_INDEX: DeclaredIndexInput = {
+  name: 'uniq_product_code',
+  fields: ['code'],
+  unique: true,
+};
+
+const PHYSICAL_COLUMNS = new Set(['id', 'organization_id', 'code']);
+
+/**
+ * What `pg` hands knex when `CREATE UNIQUE INDEX` finds duplicate rows.
+ * knex prefixes the failing statement onto the message; the primary message is
+ * `could not create unique index "…"` and the tuple lives on `detail`.
+ */
+function postgresIndexBuildConflict(): Error {
+  const err = new Error(
+    `create unique index "uniq_product_organization_id_code" on "product" ` +
+      `(COALESCE("organization_id", '__global__'), "code") - ` +
+      `could not create unique index "uniq_product_organization_id_code"`,
+  );
+  Object.assign(err, {
+    code: '23505',
+    detail: `Key (COALESCE(organization_id, '__global__'::text), code)=(__global__, DUP) is duplicated.`,
+    severity: 'ERROR',
+    routine: '_bt_check_unique',
+  });
+  return err;
+}
+
+/** mysql2's numeric channel with prose the old regex could not read. */
+function mysqlErrnoOnlyConflict(): Error {
+  const err = new Error('alter table `product` add unique `uniq_product_organization_id_code` - ER_DUP_ENTRY');
+  Object.assign(err, { errno: 1062, sqlState: '23000' });
+  return err;
+}
+
+/** A failure that is NOT a unique violation and must keep taking the boot down. */
+function unrelatedDdlFailure(): Error {
+  const err = new Error('create unique index "uniq_product_organization_id_code" - permission denied for table product');
+  Object.assign(err, { code: '42501' });
+  return err;
+}
+
+describe('syncDeclaredIndexes unique-violation discriminator (#6543)', () => {
+  let driver: SqlDriver;
+  let realKnex: any;
+  let errors: string[];
+  let warns: string[];
+
+  /** Make the NULL-safe index creation fail with `err`, and capture the log. */
+  function arm(err: Error): void {
+    (driver as any).createNullSafeUniqueIndex = async () => {
+      throw err;
+    };
+    errors = [];
+    warns = [];
+    (driver as any).logger = {
+      warn: (msg: string) => warns.push(String(msg)),
+      error: (msg: string) => errors.push(String(msg)),
+    };
+  }
+
+  /** Drive the branch under test directly — `initObjects` is not needed to reach it. */
+  function sync(indexes: DeclaredIndexInput[]): Promise<void> {
+    return (driver as any).syncDeclaredIndexes('product', indexes, PHYSICAL_COLUMNS, 'organization_id');
+  }
+
+  beforeEach(async () => {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+    realKnex = (driver as any).knex;
+    await realKnex.schema.createTable('product', (t: any) => {
+      t.string('id').primary();
+      t.string('organization_id');
+      t.string('code');
+    });
+  });
+
+  afterEach(async () => {
+    // One test stands in for `knex`; put the real one back so teardown closes it.
+    (driver as any).knex = realKnex;
+    await driver.disconnect();
+  });
+
+  // ── The channels the old message-only read could not see ──────────────────
+
+  it('absorbs a Postgres index-build conflict that names the verdict only on `code` (SQLSTATE 23505)', async () => {
+    arm(postgresIndexBuildConflict());
+
+    // Before #6543 this REJECTED: none of `unique constraint failed`,
+    // `duplicate entry`, `duplicate key value` appears in Postgres' DDL
+    // phrasing, so the branch fell through to `throw e` and the boot died on
+    // exactly the dirty database it exists to survive.
+    await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined();
+
+    // Absorbed the way the branch promises: the durability-degradation
+    // channel, naming the constraint that is NOT enforced and the way out.
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatch(/cannot create NULL-safe unique index/);
+    expect(errors[0]).toMatch(/uniq_product_organization_id_code/);
+    expect(errors[0]).toMatch(/NOT enforced/);
+    expect(errors[0]).toMatch(/#5030/);
+    expect(errors[0]).toMatch(/ADR-0120 D4/);
+  });
+
+  it('absorbs a MySQL conflict carried only on `errno` (1062)', async () => {
+    arm(mysqlErrnoOnlyConflict());
+
+    await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined();
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatch(/#5030/);
+  });
+
+  it('reads the violation through a driver `cause` wrapper', async () => {
+    const wrapped = new Error('index sync failed');
+    Object.assign(wrapped, { cause: postgresIndexBuildConflict() });
+    arm(wrapped);
+
+    await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined();
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatch(/#5030/);
+  });
+
+  // ── Nothing the old regex caught may be narrowed ──────────────────────────
+
+  it.each([
+    ['sqlite', 'UNIQUE constraint failed: product.organization_id, product.code'],
+    ['mysql', "ER_DUP_ENTRY: Duplicate entry 'DUP' for key 'uniq_product_organization_id_code'"],
+    ['postgres dml', 'duplicate key value violates unique constraint "uniq_product_organization_id_code"'],
+  ])('still absorbs the %s message spelling the inline regex used to match', async (_dialect, message) => {
+    arm(new Error(message));
+
+    await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined();
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatch(/#5030/);
+  });
+
+  // ── The site's own business logic, untouched by the migration ─────────────
+
+  it('leaves the `nullSafe.size > 0` guard intact — a plain unique still fails the sync', async () => {
+    arm(postgresIndexBuildConflict());
+    // The plain arm goes through knex's schema builder rather than the
+    // overridden method, and `knex.schema` is a fresh builder on every access
+    // — so the failure is injected by standing in for `knex` itself.
+    (driver as any).getExistingIndexNames = async () => new Set<string>();
+    (driver as any).knex = {
+      schema: {
+        alterTable: () => Promise.reject(postgresIndexBuildConflict()),
+      },
+    };
+
+    // A unique violation on a NON-NULL-safe index is not the #5030 case and
+    // must still surface: absorbing it would silently ship an unenforced
+    // constraint the drift pre-flight was never told about.
+    const rejected: any = await sync([PLAIN_INDEX]).then(
+      () => undefined,
+      (e: unknown) => e,
+    );
+    expect(rejected).toBeInstanceOf(Error);
+    expect(rejected.code).toBe('23505');
+    expect(errors).toHaveLength(0);
+  });
+
+  it('rethrows a failure that is not a unique violation, identity preserved', async () => {
+    const original = unrelatedDdlFailure();
+    arm(original);
+
+    const rejected: any = await sync([NULL_SAFE_INDEX]).then(
+      () => undefined,
+      (e: unknown) => e,
+    );
+    expect(rejected).toBe(original);
+    expect(rejected.code).toBe('42501');
+    expect(errors).toHaveLength(0);
+  });
+
+  it('still treats an "already exists" race as benign, ahead of the conflict branch', async () => {
+    const race = new Error('create unique index - index "uniq_product_organization_id_code" already exists');
+    Object.assign(race, { code: '42P07' });
+    arm(race);
+
+    await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined();
+    // Benign: absorbed WITHOUT the durability-degradation log, because the
+    // constraint IS enforced — a different outcome from the #5030 branch.
+    expect(errors).toHaveLength(0);
+  });
+});

packages/drivers/driver-sql/src/sql-driver.ts

@@ -40,7 +40,7 @@ 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 { resolveTenancyPosture } from '@objectstack/types';
+import { isUniqueViolationError, resolveTenancyPosture } from '@objectstack/types';
 import { postureEnforcesWall } from '@objectstack/spec/security';
 import { nextUtcCalendarDay } from '@objectstack/core';
 import {
@@ -6348,7 +6348,17 @@ export class SqlDriver implements IDataDriver {
         // different name can race us here — both are benign for our intent
         // (the index exists). Anything else is a real failure.
         if (/already exists|duplicate key name|exists/i.test(msg)) continue;
-        if (nullSafe.size > 0 && /unique constraint failed|duplicate entry|duplicate key value/i.test(msg)) {
+        // The ERROR OBJECT, not `msg` (#6543). This used to be a private
+        // inline regex over the message alone, which is the only channel the
+        // SQLite family reliably fills — but Postgres answers this exact
+        // failure with `could not create unique index "…"` and puts the
+        // verdict on `code` (SQLSTATE 23505) instead, so a message-only read
+        // missed the dialect entirely and took the boot down on the very case
+        // the branch below exists to absorb. The shared predicate reads
+        // `code` / `errno` / `message` / `cause`; see
+        // `@objectstack/types`' `unique-violation.ts` for why it is the one
+        // name for this question.
+        if (nullSafe.size > 0 && isUniqueViolationError(e)) {
           // Existing rows violate the NULL-safe unique — the #5030 defect made
           // visible. Do not take the boot down: the declared constraint is not
           // enforced yet, say so at `error` (from the outside everything looks
@@ -6398,8 +6408,16 @@ export class SqlDriver implements IDataDriver {
       await this.knex.raw(sql);
     } catch (e: any) {
       const msg = String(e?.message ?? e);
+      // The positive limb is this site's own question — "does this server
+      // reject functional key parts?" — and stays a message test, because
+      // that is the only channel the answer is on. The NEGATIVE limb was a
+      // seventh spelling of the unique-violation vocabulary (`/duplicate/i`)
+      // and is now the shared predicate (#6543): a conflict must never be
+      // read as a syntax rejection and silently degraded to the bare
+      // composite, and on the `errno`-only shape mysql2 can hand back, a
+      // message-only exclusion did not fire.
       const functionalUnsupported =
-        this.isMysql && /syntax|functional|not supported|near '\(/i.test(msg) && !/duplicate/i.test(msg);
+        this.isMysql && /syntax|functional|not supported|near '\(/i.test(msg) && !isUniqueViolationError(e);
       if (!functionalUnsupported) throw e;
       (this.logger.error ?? this.logger.warn)(
         `[sql-driver] this MySQL/MariaDB server rejects functional key parts — created '${name}' on ` +