fix(driver-sql): the first concurrent autonumber insert from two tenants no longer fails on Postgres with 25P02

.changeset/autonumber-cold-race-savepoint.md

@@ -0,0 +1,58 @@
+---
+"@objectstack/driver-sql": patch
+---
+
+fix(driver-sql): the first concurrent autonumber insert from two tenants no longer fails on Postgres with `25P02` (#8269)
+
+On Postgres, two tenants inserting into the same autonumber-bearing object **for
+the first time concurrently** failed the whole batch with `25P02 current
+transaction is aborted, commands ignored until end of transaction block`. The
+counters advanced anyway, so the numbers that attempt had reserved were lost —
+a permanent gap at the start of both tenants' sequences. The user-visible story
+was *"creating the first records failed; I retried, it worked, and my numbering
+starts at 0004."*
+
+`getNextSequenceValue` handled the first-insert race the way the idiom is
+usually written — catch the unique violation, then recover **on the same
+transaction**:
+
+```ts
+try {
+ await trx(SEQUENCES_TABLE).insert({ ...insertRow, last_value: initial });
+} catch (err) {
+ existing = await trx(SEQUENCES_TABLE).where(key).forUpdate().first(); // 25P02
+}
+```
+
+On Postgres any statement error aborts the entire transaction, so the recovery
+`SELECT … FOR UPDATE` **is** the statement that raises the error — the recovery
+path could never run there. SQLite and MySQL do not abort on a statement error,
+which is why the pattern looked correct, and why the SQLite-backed autonumber
+suite could not catch it.
+
+Both speculative statements now run under a `SAVEPOINT`, released on success and
+rolled back to on failure, so a failed attempt leaves the surrounding
+transaction usable on every dialect. The race handler that was written for this
+case now actually runs: the loser of the first-insert race blocks on the
+winner's row, reads the committed counter, and takes its number from the UPDATE
+path.
+
+**Scope of the second site.** The `SELECT … FOR UPDATE` fallback a few lines
+above had the same shape and the same consequence. Its comment attributed it to
+dialects that "reject `.forUpdate()` on a missing row" — measured on
+`postgres:16`, that does not happen (a missing row returns zero rows), but the
+catch is reachable for lock-level failures (deadlock `40P01`, lock/statement
+timeout `55P03`/`57014`), and each of those was being masked as an
+uninformative `25P02` by the fallback read. It is now under the same savepoint.
+
+**Not multi-org-only.** The report measured single-tenant bursts as safe; they
+are only *flakier*. Measured before the fix, 5 rounds each of one tenant × N
+cold concurrent inserts failed 0/5 (N=2), 1/5 (N=4), 3/5 (N=6) and 2/5 (N=12)
+with the same `25P02`. Two tenants means two cold counter rows, which makes the
+window near-certain to be hit rather than occasional — an amplifier, not a
+precondition. Single-organization deployments were exposed too.
+
+Unchanged: what happens to numbers on a failed attempt. The reservation still
+commits in its own transaction and is not rolled back with the caller's insert,
+which is ordinary sequence semantics. SQLite and MySQL behaviour is unchanged —
+the savepoint makes Postgres behave the way they already did.

packages/drivers/driver-sql/src/sql-driver-autonumber-cold-race.test.ts

@@ -0,0 +1,199 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #8269 — the first CONCURRENT autonumber insert into a COLD object fails on
+ * Postgres with `25P02 current transaction is aborted`.
+ *
+ * `getNextSequenceValue` handled the first-insert race by catching the unique
+ * violation and recovering **inside the same transaction**. On Postgres any
+ * statement error aborts the whole transaction, so the recovery `SELECT … FOR
+ * UPDATE` was itself the statement that raised the error — the recovery path
+ * could never run there. The fix runs each speculative statement under a
+ * SAVEPOINT (`attemptWithoutPoisoning`).
+ *
+ * ## Why this file needs a live Postgres, and all four conditions
+ *
+ * The reporter measured that dropping any single condition hides the defect,
+ * which is why the (SQLite-backed, single-tenant, warm) autonumber suite never
+ * caught it in the first place:
+ *
+ * - **Postgres**: SQLite and MySQL do not abort a transaction on a statement
+ * error, so the buggy recovery path works there. SQLite is not a weaker
+ * version of this test — it is a test that cannot fail.
+ * - **cold**: a counter row that already exists takes the UPDATE path and
+ * never reaches the speculative INSERT. Warm bursts pass unfixed.
+ * - **concurrent**: serial writers never race the first INSERT.
+ * - **cross-tenant**: two tenants means two cold counter rows, which is what
+ * makes the race window near-certain to be hit rather than occasional.
+ *
+ * That last point is a REFINEMENT of the report, measured here on `postgres:16`
+ * before the fix: single-tenant cold bursts are not safe, only *flaky* — 5
+ * rounds each of 1 tenant × N cold concurrent inserts failed 0/5 (N=2), 1/5
+ * (N=4), 3/5 (N=6) and 2/5 (N=12) with the same `25P02`. The tenant boundary is
+ * not a precondition of the defect, only an amplifier, so single-org
+ * deployments were exposed too. The guard below is still written cross-tenant
+ * because that is the shape that fails deterministically.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+import { DIALECT_CELLS, declareUnprovisionedCell, type DialectCell } from './live-dialect-matrix.testkit.js';
+
+const TABLE = 'os8269_cold_race';
+const SEQUENCES_TABLE = '_objectstack_sequences';
+
+const SHAPE = {
+ name: TABLE,
+ fields: {
+ organization_id: { type: 'string' },
+ code: { type: 'autonumber', format: 'TK-{0000}' },
+ name: { type: 'string' },
+ },
+} as any;
+
+/** Rows of a raw result, across knex's three dialect shapes. */
+function rowsOf(res: any): any[] {
+ if (Array.isArray(res) && Array.isArray(res[0])) return res[0]; // mysql2: [rows, fields]
+ if (Array.isArray(res)) return res; // better-sqlite3
+ return res?.rows ?? []; // pg
+}
+
+function coldRaceSuite(cell: DialectCell, role: string) {
+ describe(`sql-driver — autonumber cold cross-tenant race (${cell.label}) [#8269]`, () => {
+ let driver: SqlDriver;
+
+ beforeEach(async () => {
+ driver = new SqlDriver(cell.config());
+ // COLD is the whole point: drop the data table AND the counter rows, so
+ // every run starts with no sequence row for this object.
+ await driver.execute(`drop table if exists ${TABLE}`).catch(() => {});
+ await driver
+ .execute(`delete from ${SEQUENCES_TABLE} where "object" = '${TABLE}'`)
+ .catch(() => {});
+ await driver.initObjects([SHAPE]);
+ });
+
+ afterEach(async () => {
+ await driver.execute(`drop table if exists ${TABLE}`).catch(() => {});
+ await driver
+ .execute(`delete from ${SEQUENCES_TABLE} where "object" = '${TABLE}'`)
+ .catch(() => {});
+ await driver.disconnect();
+ });
+
+ it(`issues a contiguous band per tenant on a cold concurrent cross-tenant burst (${role})`, async () => {
+ const tenants = ['os8269_orgA', 'os8269_orgB'];
+ const per = 6;
+
+ // One burst, both tenants interleaved, nothing warmed up first. Unfixed,
+ // this rejects with 25P02 on Postgres.
+ const rows = await Promise.all(
+ tenants.flatMap((t) =>
+ Array.from({ length: per }, (_, i) =>
+ driver.create(TABLE, { organization_id: t, name: `${t}-${i}` }),
+ ),
+ ),
+ );
+
+ expect(rows).toHaveLength(tenants.length * per);
+ for (const t of tenants) {
+ const issued = rows
+ .filter((r: any) => r.organization_id === t)
+ .map((r: any) => r.code)
+ .sort();
+ // Each tenant gets its own counter, so each band starts at 1 and is
+ // contiguous — no duplicates, and no numbers burned by a failed attempt
+ // (the permanent gap the report saw after retrying a failed burst).
+ expect(issued).toEqual(
+ Array.from({ length: per }, (_, i) => `TK-${String(i + 1).padStart(4, '0')}`),
+ );
+ }
+ });
+
+ it(`advances each tenant's counter exactly once per issued number (${role})`, async () => {
+ const tenants = ['os8269_orgC', 'os8269_orgD'];
+ const per = 4;
+ await Promise.all(
+ tenants.flatMap((t) =>
+ Array.from({ length: per }, (_, i) =>
+ driver.create(TABLE, { organization_id: t, name: `${t}-${i}` }),
+ ),
+ ),
+ );
+
+ const counters = rowsOf(
+ await driver.execute(
+ `select tenant_id, last_value from ${SEQUENCES_TABLE} where "object" = '${TABLE}' order by tenant_id`,
+ ),
+ );
+ // `last_value` is a bigint — pg returns it as a string.
+ expect(counters.map((r: any) => [String(r.tenant_id), Number(r.last_value)])).toEqual(
+ tenants.map((t) => [t, per]),
+ );
+ });
+ });
+}
+
+const pgCell = DIALECT_CELLS.find((c) => c.id === 'pg')!;
+if (pgCell.available) {
+ // THE regression guard. Postgres is the only dialect that aborts a
+ // transaction on a statement error, so it is the only one that can fail.
+ coldRaceSuite(pgCell, 'regression guard');
+} else {
+ declareUnprovisionedCell(pgCell, 'autonumber cold cross-tenant race');
+}
+
+// A control, not coverage: SQLite passed this before the fix (the report
+// measured it) and must still pass after, which is what pins "the savepoint
+// changed nothing on the dialects that were already correct". Deleting the
+// Postgres cell above and leaving this one would be a suite that cannot fail.
+coldRaceSuite(
+ DIALECT_CELLS.find((c) => c.id === 'sqlite')!,
+ 'unaffected control — passes before AND after the fix',
+);
+
+describe(`sql-driver — attemptWithoutPoisoning (${pgCell.available ? 'live postgres' : 'skipped'}) [#8269]`, () => {
+ // The mechanism itself, pinned directly: this is what makes the SECOND
+ // speculative site (`SELECT … FOR UPDATE`, which has no `ON CONFLICT` form)
+ // safe as well. Postgres-only for the same reason as above.
+ it.skipIf(!pgCell.available)(
+ 'leaves the surrounding transaction usable after a statement error',
+ async () => {
+ const driver = new SqlDriver(pgCell.config());
+ const probe = 'os8269_poison_probe';
+ try {
+ const knex = (driver as any).knex;
+ await knex.schema.dropTableIfExists(probe);
+ await knex.schema.createTable(probe, (t: any) => {
+ t.string('k').primary();
+ t.integer('v');
+ });
+
+ await knex.transaction(async (trx: any) => {
+ await trx(probe).insert({ k: 'a', v: 1 });
+
+ const failed = await (driver as any).attemptWithoutPoisoning(trx, (scoped: any) =>
+ scoped(probe).insert({ k: 'a', v: 2 }),
+ );
+ // The original error is preserved, not swallowed — the caller needs it
+ // to tell "another writer raced me" from anything else.
+ expect(failed.ok).toBe(false);
+ expect((failed.error as any).code).toBe('23505');
+
+ // Unfixed, this read is where 25P02 surfaced.
+ const row = await trx(probe).where({ k: 'a' }).forUpdate().first();
+ expect(Number(row.v)).toBe(1);
+
+ const ok = await (driver as any).attemptWithoutPoisoning(trx, (scoped: any) =>
+ scoped(probe).insert({ k: 'b', v: 3 }),
+ );
+ expect(ok.ok).toBe(true);
+ });
+
+ await knex.schema.dropTableIfExists(probe);
+ } finally {
+ await driver.disconnect();
+ }
+ },
+ );
+});

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

@@ -4261,6 +4261,66 @@ export class SqlDriver implements IDataDriver {
  return maxN;
  }
 
+ /**
+ * Run a SPECULATIVE statement — one whose failure is an expected outcome the
+ * caller recovers from — so that its error cannot poison the surrounding
+ * transaction.
+ *
+ * # Why this exists (#8269)
+ *
+ * On Postgres, ANY statement error aborts the whole transaction: every
+ * subsequent statement returns `25P02 current transaction is aborted` until
+ * rollback. So the `try { … } catch { …recover… }` idiom, where the recovery
+ * itself issues SQL on the same transaction, can never run there — the
+ * recovery statement is the one that raises the error you observe. SQLite and
+ * MySQL do not abort the transaction on a statement error, which is why the
+ * idiom looked correct and why the SQLite-backed autonumber suite never
+ * caught it.
+ *
+ * Measured on `postgres:16`, two tenants inserting into the same cold
+ * autonumber object concurrently: `select … from "_objectstack_sequences"
+ * where "key_hash" = $1 limit $2 for update - current transaction is aborted,
+ * commands ignored until end of transaction block`. The whole batch failed
+ * while the counters advanced, so the reserved numbers were lost — a
+ * permanent gap at the start of both tenants' sequences.
+ *
+ * A knex nested transaction is a `SAVEPOINT`, released on success and rolled
+ * back to on failure, so the outer transaction stays usable on every dialect
+ * (verified: PG 16, and better-sqlite3 at pool `max: 1`, where the savepoint
+ * rides the parent's connection and never asks the pool for a second one —
+ * the deadlock #4250 exists to prevent).
+ *
+ * Chosen over `INSERT … ON CONFLICT DO NOTHING` on two pieces of evidence:
+ *
+ * 1. It is the only form that covers BOTH speculative sites here — the other
+ * one is a `SELECT … FOR UPDATE`, and there is no `ON CONFLICT` for a
+ * read.
+ * 2. `ON CONFLICT (object, tenant_id, field)` — the columns the legacy key
+ * shape uses when `sequencesHasKeyHash` is false — raises `42P10 there is
+ * no unique or exclusion constraint matching the ON CONFLICT
+ * specification` against an INTERIM table whose primary key is the FOUR
+ * columns `(object, tenant_id, field, scope)`. That pairing is reachable:
+ * it is exactly what a failed {@link ensureSequencesKeyHashShape}
+ * migration leaves behind. It would have replaced this bug with a harder
+ * one on the deployments least able to absorb it.
+ *
+ * Returns a discriminated result rather than rethrowing, so the caller keeps
+ * the ORIGINAL error to inspect or rethrow — an `ON CONFLICT DO NOTHING` that
+ * silently affects zero rows cannot tell "another writer raced me" apart from
+ * "the row was rejected for some other reason".
+ */
+ protected async attemptWithoutPoisoning<T>(
+ trx: Knex.Transaction,
+ attempt: (scoped: Knex.Transaction) => Promise<T> | Knex.QueryBuilder<any, T>,
+ ): Promise<{ ok: true; value: T } | { ok: false; error: unknown }> {
+ try {
+ const value = await trx.transaction(async (scoped) => attempt(scoped));
+ return { ok: true, value: value as T };
+ } catch (error) {
+ return { ok: false, error };
+ }
+ }
+
  /**
  * Atomically reserve and return the next sequence value for
  * `(object, tenantId, field)`. Bootstraps from the data-table MAX on
@@ -4349,13 +4409,21 @@ export class SqlDriver implements IDataDriver {
 
  return runner.transaction(async (trx) => {
  // Lock the row (no-op on SQLite, real lock on Postgres/MySQL).
+ //
+ // Speculative, so it runs under a savepoint: `.forUpdate()` on a MISSING
+ // row does NOT throw on Postgres (measured: returns zero rows), so the
+ // fallback below is unreachable for the reason the old comment gave. It
+ // IS reachable for lock-level failures — deadlock (40P01), lock/statement
+ // timeout (55P03/57014) — and without the savepoint each of those turned
+ // into a `25P02` from the fallback SELECT, masking the real error.
  let existing: any;
- try {
- existing = await trx(SEQUENCES_TABLE).where(key).forUpdate().first();
- } catch {
- // Some dialects/versions reject .forUpdate() on a missing row in
- // weird ways; fall back to plain SELECT then rely on transaction
- // isolation. Postgres/MySQL behave normally here.
+ const locked = await this.attemptWithoutPoisoning(trx, (scoped) =>
+ scoped(SEQUENCES_TABLE).where(key).forUpdate().first(),
+ );
+ if (locked.ok) {
+ existing = locked.value;
+ } else {
+ // Fall back to a plain SELECT then rely on transaction isolation.
  existing = await trx(SEQUENCES_TABLE).where(key).first();
  }
 
@@ -4370,15 +4438,15 @@ export class SqlDriver implements IDataDriver {
  suffix,
  );
  const initial = seedMax + 1;
-try {
-await trx(SEQUENCES_TABLE).insert({ ...insertRow, last_value: initial });
- return initial;
-} catch (err) {
-// Another writer raced us to the first INSERT. Fall through to
-// the UPDATE path with the now-present row.
- existing = await trx(SEQUENCES_TABLE).where(key).forUpdate().first();
- if (!existing) throw err;
-}
+const inserted = await this.attemptWithoutPoisoning(trx, (scoped) =>
+scoped(SEQUENCES_TABLE).insert({ ...insertRow, last_value: initial }),
+);
+if (inserted.ok) return initial;
+ // Another writer raced us to the first INSERT. Fall through to
+ // the UPDATE path with the now-present row. This `SELECT … FOR UPDATE`
+// blocks until the winner commits, so it sees the committed counter.
+existing = await trx(SEQUENCES_TABLE).where(key).forUpdate().first();
+if (!existing) throw inserted.error;
  }
 
  const next = Number(existing.last_value) + 1;