diff --git a/.changeset/driver-sql-batch-autonumber-resync.md b/.changeset/driver-sql-batch-autonumber-resync.md new file mode 100644 index 0000000000..cbff357087 --- /dev/null +++ b/.changeset/driver-sql-batch-autonumber-resync.md @@ -0,0 +1,72 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): `bulkCreate` and `upsert` re-seed a stale autonumber counter instead of burning the whole batch (#6943) + +#5495 taught `create()` to re-seed a stale autonumber counter and retry instead +of burning one number per failed insert. `bulkCreate()` and `upsert()` call the +same `fillAutoNumberFields` and did not get that fix. They are not, however, the +same defect as each other — measured on `main` @ `c8ff269`, on a fresh database +with seeded rows above the counter (the one-time-storm repro constraint #5495 +established): + +**`upsert` is `create()`'s old shape exactly.** Single row, so a stale counter +costs it one burned number per call: `last_value` walked 1 → 2 → 3 across two +refused upserts. Its `ON CONFLICT (mergeKeys) DO UPDATE` absorbs a conflict on +the merge key only; the tenanted autonumber lives under a *different* unique +index, so that violation is still raised and still reaches the caller. + +**`bulkCreate` is worse.** Each row reserves its number in its own committed +transaction and the batch then goes in as ONE insert, so a single colliding row +burns *every* number the batch reserved and fails the whole request: + +| 3-row `bulkCreate`, counter at 10, rows 11–39 already present | before | after | +|:---|:---|:---| +| caller-visible failures | both calls threw | **0** | +| rows written | **0** | 3 | +| `last_value` | 10 → 13, then 13 → 16 | 10 → 42, by one re-seed | + +And it is the worst path to leave without recovery: framework#2678 made +`bulkCreate` the common case for seed/import, and seed/import is exactly what +*creates* the staleness — an `isSystem` replay or a `preserveAudit` import keeps +its explicit numbers and never enters `fillAutoNumberFields` (#5495/#5503). + +Both paths now reuse #5495's machinery unchanged — `collidingAutoNumberReservations` +for the three-state routing, `autoNumberValueExists` for the data-based +discriminator (the conflicting column is never determinable for a tenanted +autonumber), and the forward-only `resyncSequenceToDataMax`. A collision that is +not provably this counter's is still rethrown untouched, so a duplicate on a +value the caller supplied still reaches them as its own error. + +**Batch semantics are unchanged, and that is a measurement rather than a +choice.** `insert(rows[])` is a single statement, so the batch was already +all-or-nothing — the failed batch above left the table exactly as it found it. +Re-issuing and retrying the whole batch therefore preserves the existing +contract: no partial success is introduced, no transaction is opened, and no +"does a failed row roll back its siblings" question arises, because siblings +already fail together. Per-row retry inside the batch was rejected for the +opposite reason — it would have had to split the one statement into N and invent +partial success where none existed. + +One thing the batch may not borrow from `create()`: `create()` keeps a +reservation that did not collide, to avoid burning a second number. A batch +cannot. One that straddles the seeded range has its low rows collide and its +high rows not, and re-issuing only the collided ones would hand them numbers +*above* the kept ones — an intra-batch duplicate the driver would have +manufactured itself. Re-issue is therefore per counter: every row drawn from a +counter that went stale is re-issued, and counters that did not go stale keep +their values, so a co-tenant's rows in the same batch are undisturbed. + +As with #5495, retrying is confined to the no-caller-transaction case. Inside a +caller's transaction the sequence `UPDATE` rolls back with the refused `INSERT`, +so nothing is burned and there is nothing to repair (measured on both paths), and +on Postgres a constraint failure aborts the transaction outright. The caller owns +that retry. + +`TursoDriver` (local/replica) and `SqliteWasmDriver` inherit both fixes, each +pinned by its own test rather than assumed from the base class — Turso +*overrides* `bulkCreate`/`upsert` to route remote traffic away, so inheritance +there is a routing fact, not a class fact. Turso's remote transport builds its +own INSERT and generates no autonumber at all, so it neither has this defect nor +receives this fix (that gap is #6944). diff --git a/packages/drivers/driver-sql/src/sql-driver-autonumber-batch-resync.test.ts b/packages/drivers/driver-sql/src/sql-driver-autonumber-batch-resync.test.ts new file mode 100644 index 0000000000..67aade25d5 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-autonumber-batch-resync.test.ts @@ -0,0 +1,328 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6943] `bulkCreate` and `upsert` re-seed a stale autonumber counter, the way + * `create()` began to at #5495. + * + * # Two different shapes, measured rather than assumed + * + * The card said these two "burn a number per collision, like `create()`". Only + * one of them does. + * + * **`upsert` does** — it is single-row, so a stale counter costs it exactly one + * burned number per call, which is `create()`'s pre-#5495 shape exactly. Its + * `ON CONFLICT (mergeKeys) DO UPDATE` absorbs a conflict on the merge key only; + * the tenanted autonumber lives under a *different* unique index, so its + * violation is still raised. Measured on `main` @ `c8ff269`: `last_value` + * 1 → 2 → 3 across two refused upserts. + * + * **`bulkCreate` does not — it is worse.** Every row reserves its own number in + * its own committed transaction, then the whole batch goes in as ONE insert. So + * one colliding row burns *every* number the batch reserved and fails the whole + * request. Measured on the same commit, counter at 10 with rows 11–39 landed by + * a bypass path: + * + * | 3-row `bulkCreate` | before | after | + * |:---|:---|:---| + * | caller-visible failures | **2 of 2 calls threw** | **0** | + * | rows written | **0** | 3 | + * | `last_value` | 10 → 13, then 13 → 16 | 10 → 42, by one re-seed | + * + * And this is the worst path to have no recovery on: framework#2678 made + * `bulkCreate` the common case for seed/import, and seed/import is precisely + * what *creates* the staleness — an `isSystem` replay or a `preserveAudit` + * import keeps its explicit numbers and never enters `fillAutoNumberFields` + * (#5495/#5503). + * + * # Batch semantics are unchanged, and that is a measurement, not a taste + * + * The card expected a decision about partial success and transaction + * boundaries. There is none to make: `insert(rows[])` is a SINGLE statement, so + * the batch is already all-or-nothing — the failed batch above left the table + * exactly as it found it (31 rows before, 31 after). Re-issuing and retrying + * the whole batch therefore preserves the existing contract exactly. The + * alternatives that would have changed it — per-row retry, a driver-opened + * transaction — are rejected in `bulkCreate`'s own comment with the reasons. + * + * # The one thing the batch may NOT borrow from `create()` + * + * `create()` keeps a reservation that did not collide, to avoid burning a + * second number. A batch cannot: one that straddles the seeded range has its + * low rows collide and its high rows not, and regenerating only the low ones + * hands them numbers *above* the kept ones — an intra-batch duplicate the + * driver would have manufactured itself. So re-issue is per *counter*: every + * row drawn from a counter that went stale is re-issued. Pinned below. + * + * # On the assertion style + * + * Same as #5495's file, and for the same reason: ADR-0112's `code`/`status` + * envelope is a *wire* contract, and what a refused INSERT throws here is the + * driver's raw error, which carries neither. These assert something stronger + * than a bare `toThrow()`: the specific constraint text, `isUniqueViolationError` + * agreeing, and — for the cases that must NOT be retried — that the error names + * the CALLER's field and that no row landed. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { isUniqueViolationError } from '@objectstack/types'; +import { SqlDriver } from './index.js'; + +const SEQUENCES_TABLE = '_objectstack_sequences'; + +const CRM_CASE = { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, +} as any; + +describe('[#6943] batch and upsert re-seed a stale autonumber counter', () => { + let driver: SqlDriver; + + const knex = () => (driver as any).knex; + + const lastValue = async (tenant: string): Promise => { + const rows = await knex()(SEQUENCES_TABLE).select('*'); + const row = rows.find((r: any) => r.object === 'crm_case' && r.field === 'case_number' && r.tenant_id === tenant); + return row ? Number(row.last_value) : undefined; + }; + + /** Land rows by a path that bypasses `fillAutoNumberFields`, as a seed replay does. */ + const bypassInsert = async (org: string, from: number, to: number) => { + const rows = []; + for (let n = from; n <= to; n++) { + rows.push({ id: `${org}-s${n}`, organization_id: org, case_number: `CASE-${String(n).padStart(5, '0')}`, title: `seed ${n}` }); + } + await knex()('crm_case').insert(rows); + }; + + const rowCount = async () => Number((await knex()('crm_case').count({ c: '*' }).first()).c); + + /** Every `case_number` in the table, for the duplicate check. */ + const allNumbers = async (): Promise => + (await knex()('crm_case').select('case_number')).map((r: any) => r.case_number); + + beforeEach(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + // ---------------------------------------------------------------- bulkCreate + + it('serves a batch that used to fail outright, and burns the batch only once', async () => { + await driver.initObjects([CRM_CASE]); + + await bypassInsert('orgA', 9, 9); + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); // counter -> 10 + await bypassInsert('orgA', 11, 39); // seeds land ABOVE the counter; nothing tells the sequence + + const created = await driver.bulkCreate('crm_case', [ + { organization_id: 'orgA', title: 'b1' }, + { organization_id: 'orgA', title: 'b2' }, + { organization_id: 'orgA', title: 'b3' }, + ]); + + // Before this change the same call threw and wrote nothing. + expect(created.map((r: any) => r.case_number)).toEqual(['CASE-00040', 'CASE-00041', 'CASE-00042']); + expect(await rowCount()).toBe(34); + + // And the counter is simply +1 from here — re-seeding is not a mode the + // driver stays in. + const next = await driver.bulkCreate('crm_case', [{ organization_id: 'orgA', title: 'b4' }]); + expect(next[0].case_number).toBe('CASE-00043'); + }); + + it('re-issues the WHOLE batch drawn from a stale counter, so it cannot duplicate itself', async () => { + // The straddle case: the batch is longer than the stale gap, so its low + // rows collide and its high rows do not. Re-issuing only the collided ones + // would hand them numbers above the kept ones. + await driver.initObjects([CRM_CASE]); + + await bypassInsert('orgA', 9, 9); + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); // counter -> 10 + await bypassInsert('orgA', 11, 39); + + const batch = Array.from({ length: 60 }, (_, i) => ({ organization_id: 'orgA', title: `straddle ${i}` })); + const created = await driver.bulkCreate('crm_case', batch); + + expect(created).toHaveLength(60); + const numbers = await allNumbers(); + expect(new Set(numbers).size).toBe(numbers.length); // no duplicate anywhere in the table + // Every row of the batch sits above the seeded range it straddled. + for (const r of created) expect(Number(r.case_number.slice('CASE-'.length))).toBeGreaterThan(39); + }); + + it('re-seeds only the counter that went stale, leaving a co-tenant in the same batch alone', async () => { + await driver.initObjects([CRM_CASE]); + + await driver.create('crm_case', { organization_id: 'orgA', title: 'a1' }); // A -> 1 + await driver.create('crm_case', { organization_id: 'orgB', title: 'b1' }); // B -> 1 + await bypassInsert('orgA', 2, 20); // only A goes stale + + const created = await driver.bulkCreate('crm_case', [ + { organization_id: 'orgA', title: 'a2' }, + { organization_id: 'orgA', title: 'a3' }, + { organization_id: 'orgB', title: 'b2' }, + ]); + + expect(created.map((r: any) => r.case_number)).toEqual(['CASE-00021', 'CASE-00022', 'CASE-00002']); + // B never collided, so its reservation was kept rather than burned and + // re-issued: one number consumed, not two. + expect(await lastValue('orgB')).toBe(2); + }); + + it('does not swallow a duplicate the CALLER supplied, and still writes nothing', async () => { + await driver.initObjects([ + { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + email: { type: 'string', unique: 'global' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, + } as any, + ]); + + await driver.create('crm_case', { organization_id: 'orgA', email: 'dup@example.com', title: 'one' }); + const before = await rowCount(); + + const caught = await driver + .bulkCreate('crm_case', [ + { organization_id: 'orgA', email: 'fresh@example.com', title: 'ok' }, + { organization_id: 'orgA', email: 'dup@example.com', title: 'the caller’s 409' }, + ]) + .then(() => undefined, (e: unknown) => e); + + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toMatch(/UNIQUE constraint failed|duplicate key value/); + expect(isUniqueViolationError(caught)).toBe(true); + // The 409 is the client's: not retried away, and the batch is still + // all-or-nothing — the sibling row did not sneak in. + expect(await rowCount()).toBe(before); + }); + + it('leaves the caller-transaction path exactly as it was', async () => { + await driver.initObjects([CRM_CASE]); + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); + await bypassInsert('orgA', 2, 2); + + const before = await lastValue('orgA'); + const caught = await knex() + .transaction(async (trx: any) => { + await driver.bulkCreate( + 'crm_case', + [{ organization_id: 'orgA', title: 't1' }, { organization_id: 'orgA', title: 't2' }], + { transaction: trx } as any, + ); + }) + .then(() => undefined, (e: unknown) => e); + + // Still refused, and nothing burned: the sequence UPDATE shares the + // caller's transaction and rolls back with the refused INSERT. On Postgres + // a constraint failure aborts the transaction outright, so a retry issued + // on it could not succeed — the caller owns that retry. + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toMatch(/UNIQUE constraint failed|duplicate key value/); + expect(await lastValue('orgA')).toBe(before); + }); + + it('leaves a batch with no autonumber field completely untouched', async () => { + await driver.initObjects([ + { name: 'note', fields: { organization_id: { type: 'string' }, body: { type: 'string' } } } as any, + ]); + const created = await driver.bulkCreate('note', [ + { organization_id: 'orgA', body: 'one' }, + { organization_id: 'orgA', body: 'two' }, + ]); + expect(created).toHaveLength(2); + }); + + // -------------------------------------------------------------------- upsert + + it('upsert: serves the insert that used to be refused by a stale counter', async () => { + await driver.initObjects([CRM_CASE]); + + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); // counter -> 1 + await bypassInsert('orgA', 2, 20); + + const upserted = await driver.upsert('crm_case', { organization_id: 'orgA', title: 'u1' }); + expect(upserted.case_number).toBe('CASE-00021'); + + // +1 from here, like every other path. + expect((await driver.upsert('crm_case', { organization_id: 'orgA', title: 'u2' })).case_number).toBe('CASE-00022'); + }); + + it('upsert: a duplicate on a CALLER-supplied unique field still reaches the caller', async () => { + await driver.initObjects([ + { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + email: { type: 'string', unique: 'global' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, + } as any, + ]); + + await driver.create('crm_case', { organization_id: 'orgA', email: 'dup@example.com', title: 'one' }); + + const caught = await driver + .upsert('crm_case', { organization_id: 'orgA', email: 'dup@example.com', title: 'two' }) + .then(() => undefined, (e: unknown) => e); + + expect(caught).toBeInstanceOf(Error); + expect(isUniqueViolationError(caught)).toBe(true); + expect((caught as Error).message).toMatch(/email/); + }); + + it('upsert: an explicit autonumber value is preserved, never regenerated', async () => { + await driver.initObjects([CRM_CASE]); + await driver.create('crm_case', { organization_id: 'orgA', title: 'first' }); + + const caught = await driver + .upsert('crm_case', { organization_id: 'orgA', case_number: 'CASE-00001', title: 'explicit duplicate' }) + .then(() => undefined, (e: unknown) => e); + + expect(caught).toBeInstanceOf(Error); + expect(isUniqueViolationError(caught)).toBe(true); + // A caller-supplied value is not a reservation, so no probe and no retry + // can apply to it — and no number was consumed. + expect(await lastValue('orgA')).toBe(1); + }); + + it('upsert: leaves the caller-transaction path exactly as it was', async () => { + await driver.initObjects([CRM_CASE]); + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); + await bypassInsert('orgA', 2, 2); + + const before = await lastValue('orgA'); + const caught = await knex() + .transaction(async (trx: any) => { + await driver.upsert('crm_case', { organization_id: 'orgA', title: 'in-trx' }, undefined, { transaction: trx } as any); + }) + .then(() => undefined, (e: unknown) => e); + + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toMatch(/UNIQUE constraint failed|duplicate key value/); + expect(await lastValue('orgA')).toBe(before); + }); + + it('upsert: still merges onto an existing row rather than inserting a second', async () => { + // The merge branch is untouched by this change — kept here so a future + // edit to the retry loop cannot quietly turn an update into an insert. + await driver.initObjects([CRM_CASE]); + const first = await driver.create('crm_case', { organization_id: 'orgA', title: 'original' }); + + const merged = await driver.upsert('crm_case', { id: first.id, organization_id: 'orgA', title: 'edited' }); + expect(merged.id).toBe(first.id); + expect(merged.title).toBe('edited'); + expect(await rowCount()).toBe(1); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 5613898b71..0f6551c614 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -3898,6 +3898,24 @@ export class SqlDriver implements IDataDriver { }); } + /** + * Identity of the counter a reservation was drawn from — the same + * `(table, tenant, field, scope)` hash {@link getNextSequenceValue} keys the + * sequences row by (#6943). + * + * `create()` never needed this: one row draws at most one reservation per + * field, so "which counter" and "which reservation" are the same question. + * A batch breaks that — many rows draw from ONE counter — and both of the + * batch's collision decisions are per *counter*, not per reservation: re-seed + * each stale counter once rather than once per row, and re-issue every row + * that drew from it. See {@link bulkCreate}. + */ + protected autoNumberCounterKey(reservation: AutoNumberReservation): string { + const resolvedTenantId = + reservation.tenantField && reservation.tenantId ? String(reservation.tenantId) : GLOBAL_TENANT; + return this.sequenceKeyHash(reservation.tableName, resolvedTenantId, reservation.field, reservation.scope); + } + /** * Stamp the builtin audit timestamps to one canonical ISO-8601-with-`Z` * instant on the SQLite write paths (`create`/`bulkCreate`/`upsert`), so @@ -3977,23 +3995,56 @@ export class SqlDriver implements IDataDriver { this.auditMissingTenant(object, 'upsert', options); this.injectTenantOnInsert(object, toUpsert, options); - await this.fillAutoNumberFields(object, toUpsert, options); - const formatted = this.applyWriteColumnMap(object, this.formatInput(object, toUpsert)); - this.stampInsertTimestamps(object, formatted); const mergeKeys = conflictKeys && conflictKeys.length > 0 ? conflictKeys : ['id']; - // Rotation: conflict-merge is scoped to the CURRENT shard (telemetry is - // effectively append-only; a cross-shard upsert would need a probe-first - // strategy nothing on the platform requires today). - const builder = this.getBuilder(this.rotationWriteTarget(object) ?? object, options); - // `created_at` is insert-only — never overwrite it when an existing row is - // merged on conflict (the stamped/seeded value belongs to the original - // insert). Everything else (incl. `updated_at`) merges as before, so an - // upsert that updates a row still advances `updated_at`. - const mergeColumns = Object.keys(formatted).filter((c) => c !== 'created_at'); - const insertion = builder.insert(formatted).onConflict(mergeKeys); - await (mergeColumns.length > 0 ? insertion.merge(mergeColumns) : insertion.merge()); + // #6943. Measured: `upsert` does NOT share `bulkCreate`'s shape. It is + // single-row, so a stale counter costs it exactly one burned number per + // call — the same shape `create()` had before #5495, for the same reason + // (no re-seed, no retry). `ON CONFLICT (mergeKeys) DO UPDATE` absorbs a + // conflict on the merge key only; the tenanted autonumber lives under a + // DIFFERENT unique index, so its violation is still raised and still + // reaches here. The remedy is therefore `create()`'s, verbatim: re-seed and + // re-issue, but only when the collision is provably this counter's + // ({@link collidingAutoNumberReservations}), and only outside a caller + // transaction (inside one the sequence UPDATE rolls back with the refused + // INSERT, so nothing is burned and there is nothing to repair — measured). + const mayRetry = options?.transaction === undefined; + for (let attempt = 0; ; attempt++) { + const reservations = await this.fillAutoNumberFields(object, toUpsert, options); + + const formatted = this.applyWriteColumnMap(object, this.formatInput(object, toUpsert)); + this.stampInsertTimestamps(object, formatted); + + // Rotation: conflict-merge is scoped to the CURRENT shard (telemetry is + // effectively append-only; a cross-shard upsert would need a probe-first + // strategy nothing on the platform requires today). + const builder = this.getBuilder(this.rotationWriteTarget(object) ?? object, options); + // `created_at` is insert-only — never overwrite it when an existing row is + // merged on conflict (the stamped/seeded value belongs to the original + // insert). Everything else (incl. `updated_at`) merges as before, so an + // upsert that updates a row still advances `updated_at`. + const mergeColumns = Object.keys(formatted).filter((c) => c !== 'created_at'); + const insertion = builder.insert(formatted).onConflict(mergeKeys); + + try { + await (mergeColumns.length > 0 ? insertion.merge(mergeColumns) : insertion.merge()); + break; + } catch (error) { + if (!mayRetry || attempt >= AUTONUMBER_COLLISION_RETRIES) throw error; + const colliding = await this.collidingAutoNumberReservations(error, reservations, options); + if (colliding.length === 0) throw error; + for (const reservation of colliding) { + await this.resyncSequenceToDataMax(reservation); + // Clear only what collided, so the next pass regenerates exactly + // those fields — a reservation that did NOT collide keeps its value + // rather than burning a second number for nothing. (Safe here in a + // way it is NOT for a batch: one row cannot collide with itself. + // See {@link bulkCreate}.) + delete toUpsert[reservation.field]; + } + } + } const readback = this.getBuilder(object, options).where('id', toUpsert.id); this.applyTenantScope(readback, object, options); @@ -4043,33 +4094,126 @@ export class SqlDriver implements IDataDriver { return toInsert; }); for (const row of rows) { - if (row && typeof row === 'object') { - this.injectTenantOnInsert(object, row, options); - // Reserve a persistent sequence value for each row's autonumber - // field(s) — the engine no longer pre-fills these (see #1603). - await this.fillAutoNumberFields(object, row, options); + if (row && typeof row === 'object') this.injectTenantOnInsert(object, row, options); + } + + // #6943. A stale counter costs this path more than it costs `create()`. + // Each row reserves its own number in its own committed transaction, and + // then the whole batch goes in as ONE insert — so a single colliding row + // does not burn one number, it burns every number the batch reserved and + // fails the whole request. Measured on `main` @ `c8ff269` (counter at 10, + // rows 11–39 landed by a bypass path): a 3-row `bulkCreate` threw, wrote + // ZERO rows and moved `last_value` 10 → 13; the next call moved it 13 → 16 + // and threw again. And the exposure is the worst available, because + // framework#2678 made this the common path for seed/import — which is + // precisely what *creates* the staleness (`isSystem` replay and + // `preserveAudit` import keep their explicit numbers and never enter + // `fillAutoNumberFields`, #5495/#5503). + // + // ## Batch semantics are deliberately NOT changed + // + // The card anticipated a decision about partial success and transaction + // boundaries. Measurement dissolves it: `insert(rows[])` is a SINGLE + // statement (`… select … union all select …` on SQLite, multi-row VALUES + // elsewhere), so the batch is ALREADY all-or-nothing — the failed batch + // above left the table exactly as it found it. Re-issuing and retrying the + // WHOLE batch therefore preserves the existing contract byte for byte: no + // partial success is introduced, no transaction is opened, no sibling- + // rollback question arises because siblings already fail together. + // Rejected instead: + // - **Per-row retry inside the batch** (the shape the card sketched). + // It requires splitting the one statement into N to learn which row + // failed, which INVENTS partial success where none exists, costs N + // round-trips on the hot seed/import path, and is what the engine's + // own #6806 note warns about ("re-writing the batch could DUPLICATE + // the rows that did land"). Nothing lands here, so nothing can. + // - **Wrapping the batch in a driver-opened transaction.** It would move + // the boundary the card was worried about, for no gain (the statement + // is already atomic), and it defeats the reservation model: each + // `getNextSequenceValue` commits on purpose, which is what makes a + // forward-only re-seed meaningful. On SQLite (pool max 1) it is also + // the deadlock `ensureSequencesTable` documents. + // - **Re-seed but do not re-issue** (what the engine does at #6806). + // Correct there, because the engine cannot know whether an arbitrary + // driver applied the batch partially. Inside THIS driver that is + // measurably false, so the caller can be handed a successful batch + // instead of an error it has to retry itself. + // - **Blanket retry on any unique violation.** Rejected for #5495's + // reason: it silently eats the caller's own 409. + const mayRetry = options?.transaction === undefined; + // What each row currently holds, ACROSS attempts. A retry regenerates only + // the fields it cleared, so `fillAutoNumberFields` reports only those; a + // value kept from the previous pass is still a live reservation and has to + // stay described here, or a second collision on a counter that did not go + // stale the first time would find nothing to route and rethrow blind. + const reservationsPerRow: AutoNumberReservation[][] = rows.map(() => []); + for (let attempt = 0; ; attempt++) { + // Reserve a persistent sequence value for each row's autonumber + // field(s) — the engine no longer pre-fills these (see #1603). + for (let i = 0; i < rows.length; i++) { + const row = rows[i]; + if (!row || typeof row !== 'object') continue; + const reissued = await this.fillAutoNumberFields(object, row, options); + if (reissued.length === 0) continue; + const replaced = new Set(reissued.map((r) => r.field)); + reservationsPerRow[i] = [...reservationsPerRow[i].filter((r) => !replaced.has(r.field)), ...reissued]; + } + + // Same write-side marshaling as create() (#2735): JSON-typed and + // object-valued fields must be serialized per row before they reach the + // knex binder — the raw batch used to hand `{lat, lng}` objects straight + // to SQLite ("Wrong API use: tried to bind a value of an unknown type"), + // silently failing the whole seed batch. Timestamp stamping runs on the + // FORMATTED copy, mirroring create(). + const formattedRows = rows.map((row) => { + if (!row || typeof row !== 'object') return row; + const formatted = this.applyWriteColumnMap(object, this.formatInput(object, row)); + this.stampInsertTimestamps(object, formatted); + return formatted; + }); + const builder = this.getBuilder(this.rotationWriteTarget(object) ?? object, options); + + try { + const result = await builder.insert(formattedRows).returning('*'); + // Read-back parity with create(): JSON columns come back as their stored + // strings from `returning('*')` — decode them so batch callers see the + // same shapes single-insert callers do. + return Array.isArray(result) + ? result.map((r) => this.formatOutput(object, r)) + : result; + } catch (error) { + if (!mayRetry || attempt >= AUTONUMBER_COLLISION_RETRIES) throw error; + const colliding = await this.collidingAutoNumberReservations(error, reservationsPerRow.flat(), options); + if (colliding.length === 0) throw error; + + // Re-seed each stale counter ONCE, not once per row that drew from it: + // the scan is identical for every row sharing a counter and the update + // is forward-only, so the repeats are pure cost. + const stale = new Set(); + for (const reservation of colliding) { + const key = this.autoNumberCounterKey(reservation); + if (stale.has(key)) continue; + stale.add(key); + await this.resyncSequenceToDataMax(reservation); + } + + // Re-issue EVERY row drawn from a stale counter — including rows whose + // own value did not collide. This is where the batch genuinely differs + // from `create()`, which keeps a non-colliding value to avoid burning a + // second number. Keeping them here is unsound: a batch that straddles + // the seeded range (say the counter is at 10, rows 11–39 exist, and the + // batch reserved 11–70) has its low rows collide and its high rows not, + // and regenerating only the low ones hands them numbers ABOVE the kept + // ones — an intra-batch duplicate this driver would have created + // itself. Counters that did NOT go stale keep their values, so another + // tenant's rows in the same batch are undisturbed. + for (let i = 0; i < rows.length; i++) { + for (const reservation of reservationsPerRow[i]) { + if (stale.has(this.autoNumberCounterKey(reservation))) delete rows[i][reservation.field]; + } + } } } - // Same write-side marshaling as create() (#2735): JSON-typed and - // object-valued fields must be serialized per row before they reach the - // knex binder — the raw batch used to hand `{lat, lng}` objects straight - // to SQLite ("Wrong API use: tried to bind a value of an unknown type"), - // silently failing the whole seed batch. Timestamp stamping runs on the - // FORMATTED copy, mirroring create(). - const formattedRows = rows.map((row) => { - if (!row || typeof row !== 'object') return row; - const formatted = this.applyWriteColumnMap(object, this.formatInput(object, row)); - this.stampInsertTimestamps(object, formatted); - return formatted; - }); - const builder = this.getBuilder(this.rotationWriteTarget(object) ?? object, options); - const result = await builder.insert(formattedRows).returning('*'); - // Read-back parity with create(): JSON columns come back as their stored - // strings from `returning('*')` — decode them so batch callers see the - // same shapes single-insert callers do. - return Array.isArray(result) - ? result.map((r) => this.formatOutput(object, r)) - : result; } /** diff --git a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-autonumber-batch-resync.test.ts b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-autonumber-batch-resync.test.ts new file mode 100644 index 0000000000..4788b0a32e --- /dev/null +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-autonumber-batch-resync.test.ts @@ -0,0 +1,79 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6943] The wasm face re-seeds on the batch and upsert paths too. + * + * `SqliteWasmDriver extends SqlDriver` and overrides neither `bulkCreate` nor + * `upsert`, so the re-seed is inherited. What this pins is that the inheritance + * actually DELIVERS it: this driver swaps knex's transport for a custom sql.js + * dialect, and the collision path depends on three things surviving that swap — + * the refused INSERT still throwing an error `isUniqueViolationError` + * recognises, the probe read (`autoNumberValueExists`) seeing the row that + * caused the refusal, and — new on the batch path — a multi-row + * `insert(rows[])` still failing as ONE statement, which is what makes + * re-issuing the whole batch safe rather than duplicating rows. + * + * "It inherits the base class, therefore it is fine" is the assumption #4405 + * and #5240 exist to disprove on this driver, and #6203 is the shape where one + * fix landed on one face and left the other answering differently. So the + * fourth backend is verified rather than assumed. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqliteWasmDriver } from './index.js'; + +describe('[#6943] driver-sqlite-wasm inherits the batch/upsert autonumber re-seed', () => { + let driver: SqliteWasmDriver; + + const knex = () => (driver as any).knex; + + const bypassInsert = async (from: number, to: number) => { + const rows = []; + for (let n = from; n <= to; n++) { + rows.push({ id: `s${n}`, organization_id: 'orgA', case_number: `CASE-${String(n).padStart(5, '0')}`, title: `seed ${n}` }); + } + await knex()('crm_case').insert(rows); + }; + + beforeEach(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, + } as any, + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('bulkCreate serves a batch that a stale counter used to fail outright', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); + await bypassInsert(2, 30); + + const created = await driver.bulkCreate('crm_case', [ + { organization_id: 'orgA', title: 'b1' }, + { organization_id: 'orgA', title: 'b2' }, + ]); + + expect(created.map((r: any) => r.case_number)).toEqual(['CASE-00031', 'CASE-00032']); + // Nothing was left behind by the failed first pass — the batch is one + // statement on this transport too. + const all = (await knex()('crm_case').select('case_number')).map((r: any) => r.case_number); + expect(new Set(all).size).toBe(all.length); + }); + + it('upsert serves the insert a stale counter used to refuse', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }); + await bypassInsert(2, 30); + + const upserted = await driver.upsert('crm_case', { organization_id: 'orgA', title: 'u1' }); + expect(upserted.case_number).toBe('CASE-00031'); + }); +}); diff --git a/packages/drivers/driver-turso/src/turso-autonumber-batch-resync.test.ts b/packages/drivers/driver-turso/src/turso-autonumber-batch-resync.test.ts new file mode 100644 index 0000000000..38d5dab945 --- /dev/null +++ b/packages/drivers/driver-turso/src/turso-autonumber-batch-resync.test.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6943] The Turso faces, held against each other on the batch/upsert re-seed. + * + * `TursoDriver extends SqlDriver` but picks its engine from the `url` it was + * constructed with, and #6203 is the shape where that costs a fix half its + * reach: one driver, two answers. So both faces are stated here rather than one + * being assumed from the other — and `bulkCreate` needs saying out loud even + * more than `create()` did, because Turso **overrides** it (`create` and + * `upsert` too) rather than merely inheriting: each override routes remote to + * `RemoteTransport` and everything else to `super`, so "the base class was + * fixed" is not on its own an answer about this face. + * + * - **LOCAL (and replica, same local engine)** falls through to + * `super.bulkCreate` / `super.upsert` and with them the re-seed. Asserted + * below on rows. + * - **REMOTE** hands the batch to `RemoteTransport.bulkCreate`, which builds + * its own INSERT and never enters `fillAutoNumberFields` — so it has neither + * the defect nor the fix. On that face `auto_number` is only a column-type + * mapping and no sequence machinery exists to be stale; the absence is + * pinned as an ASSERTION rather than a comment, so wiring autonumber into + * the remote transport later cannot silently inherit this file's green. + * (That gap is #6944's, not this card's.) + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { TursoDriver } from './index.js'; + +describe('[#6943] TursoDriver batch/upsert autonumber re-seed', () => { + let driver: TursoDriver; + + const knex = () => (driver as any).knex; + + const bypassInsert = async (org: string, from: number, to: number) => { + const rows = []; + for (let n = from; n <= to; n++) { + rows.push({ id: `${org}-s${n}`, organization_id: org, case_number: `CASE-${String(n).padStart(5, '0')}`, title: `seed ${n}` }); + } + await knex()('crm_case').insert(rows); + }; + + beforeEach(async () => { + driver = new TursoDriver({ url: ':memory:' }); + expect(driver.transportMode).toBe('local'); + await driver.initObjects([ + { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, + } as any, + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('LOCAL: bulkCreate serves a batch that a stale counter used to fail outright', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }, { bypassTenantAudit: true }); + await bypassInsert('orgA', 2, 30); + + const created = await driver.bulkCreate( + 'crm_case', + [ + { organization_id: 'orgA', title: 'b1' }, + { organization_id: 'orgA', title: 'b2' }, + ], + { bypassTenantAudit: true } as any, + ); + + expect(created.map((r: any) => r.case_number)).toEqual(['CASE-00031', 'CASE-00032']); + }); + + it('LOCAL: upsert serves the insert a stale counter used to refuse', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'warm' }, { bypassTenantAudit: true }); + await bypassInsert('orgA', 2, 30); + + const upserted = await driver.upsert( + 'crm_case', + { organization_id: 'orgA', title: 'u1' }, + undefined, + { bypassTenantAudit: true } as any, + ); + + expect(upserted.case_number).toBe('CASE-00031'); + }); + + it('REMOTE: the transport that bypasses this path has no autonumber machinery to re-seed', async () => { + const remote = new TursoDriver({ url: 'libsql://example.turso.io', authToken: 'placeholder' }); + expect(remote.transportMode).toBe('remote'); + + // The boundary, stated as a fact about the code rather than about a live + // connection: `RemoteTransport` has no autonumber surface at all, on the + // batch path any more than the single-row one. When one is added (#6944), + // this assertion is what has to be revisited. + const transportSurface = Object.getOwnPropertyNames( + Object.getPrototypeOf((remote as any).remoteTransport), + ); + expect(transportSurface).toContain('bulkCreate'); + expect(transportSurface.some((m) => /autonumber|sequence/i.test(m))).toBe(false); + }); +});