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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
34 changes: 34 additions & 0 deletions .changeset/sql-bulk-update-transaction.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `bulkUpdate` applies as one transaction, so a mid-batch refusal rolls back the rows already applied (#13854)

`SqlDriver.bulkUpdate` is a sequential loop of individual `update()` calls — one
per id, because each id carries its own patch and no single statement expresses
N different SET lists. That loop had no transaction around it, so every
`update()` autocommitted its own row. A batch refused partway through — a unique
violation on a later row, a NOT NULL violation, any per-row failure from the
database — left every row processed **before** the refusal permanently
committed. The caller received an exception while the database held a state
nobody declared: neither the pre-image nor the post-image, and a retry of the
same array was not safe.

The loop now runs inside a transaction, so the batch applies as one unit.
`driver-turso` reaches this door through `super.bulkUpdate` and inherits the
repair; no subclass change is needed or was made.

Same defect class #13340/#13435 closed on `driver-memory`'s batch doors, on the
production SQL driver. `bulkDelete` was measured and is untouched — it is a
single `whereIn(...).delete()` per rotation shard, already atomic per shard.

**Behaviour inside a caller's transaction.** When the caller supplies
`options.transaction`, the batch runs in a nested transaction (a `SAVEPOINT`) on
that same transaction rather than opening a competing one. A refused batch is
undone as a unit, the caller's own surrounding work is untouched, and the
caller's transaction stays usable — so a caller that catches the refusal and
commits anyway no longer lands a partial batch.

No API, signature or accepted-input change: every input accepted before is
accepted after, and every refusal that fired before still fires. A missing id is
still skipped rather than refused, unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#13854] `SqlDriver.bulkUpdate` is a sequential `for`-`await` over individual
* `update()` calls. Each of those autocommitted its own row, so a batch refused
* partway through left every row processed BEFORE the refusal permanently
* committed — the caller got an exception and the database held a state nobody
* declared. `driver-turso` reaches the same door through `super.bulkUpdate`,
* which is why the pin lives here, on the parent.
*
* ## What makes this file a measurement rather than a green suite
*
* §1 and §4 each assert three things, and only the THIRD one is this card:
*
* 1. the batch refuses (passes on the broken code too);
* 2. the call throws (passes on the broken code too);
* 3. ⭐ the EARLIER row is unchanged **in the database** — read back through a
* bare knex query against the physical table rather than from the return
* value, because the return value of a throwing call is not evidence about
* storage, and the driver's own read path is not an independent witness.
*
* Reverse verification (direction predicted before running): restoring `main`'s
* body — the bare loop, no transaction — leaves assertions 1 and 2 green and
* turns assertion 3 red in §1 and §4, with the received value being the
* committed post-image (`'first-updated'` where `'first'` is asserted). §2, §3,
* §5 and §6 stay green in both directions: they pin what must NOT change.
*
* ## Which refusal, and why not the obvious one
*
* ⚠️ A missing id does NOT refuse on this door: `update()` issues an UPDATE
* matching zero rows and returns `null`, which `bulkUpdate` skips
* (`if (updated)`). A pin built on one would measure nothing — it never throws.
* §6 pins that skip, since the card must not change it. The refusal used here is
* the database's own: a `unique: 'global'` string column, and a later row in the
* batch assigned a value a THIRD row already holds. Measured on better-sqlite3
* as `SQLITE_CONSTRAINT_UNIQUE` / `UNIQUE constraint failed: account.code`; the
* assertion accepts the Postgres and MySQL spellings too, the vocabulary this
* package's other constraint tests assert on.
*
* `unique: 'global'` rather than `unique: true` on purpose: it materializes a
* single-column index on every tenancy posture (ADR-0120 D1), so the refusal
* this file depends on cannot be re-scoped by a change to the tenancy composite.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** These fixtures are not tenant-scoped; the audit warning is not under test. */
const OPTS = { bypassTenantAudit: true } as any;

describe('SqlDriver.bulkUpdate atomicity (#13854)', () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
} as any);
await driver.initObjects([
{
name: 'account',
fields: {
code: { type: 'string', unique: 'global' },
name: { type: 'string' },
},
},
] as any);
await driver.create('account', { id: 'a', code: 'A-1', name: 'first' }, OPTS);
await driver.create('account', { id: 'b', code: 'B-1', name: 'second' }, OPTS);
await driver.create('account', { id: 'c', code: 'C-1', name: 'third' }, OPTS);
});

afterEach(async () => {
await driver.disconnect();
});

/**
* Read a row straight from the physical table, bypassing the driver's read
* path entirely — the independent witness assertion 3 needs.
*/
const stored = async (id: string): Promise<Record<string, any> | undefined> =>
(driver as any).knex('account').where('id', id).first();

/**
* A batch whose SECOND entry collides with row `c`'s code. The first entry is
* a perfectly good update, and on `main` it committed.
*/
const COLLIDING_BATCH = [
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { code: 'C-1' } },
];

it('§1 rolls the whole batch back when a LATER row refuses (no caller transaction)', async () => {
// 1 + 2 — the batch refuses and the call throws. Both pass on `main` too.
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow(
/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/,
);

// 3 ⭐ — the entire card. The earlier row was applied and then undone.
expect((await stored('a'))?.name).toBe('first');

// The refused row itself, and the row it collided with, are also untouched.
expect((await stored('b'))?.code).toBe('B-1');
expect((await stored('c'))?.code).toBe('C-1');
});

it('§2 leaves rows outside the batch alone', async () => {
await expect(driver.bulkUpdate('account', COLLIDING_BATCH, OPTS)).rejects.toThrow();
expect((await stored('c'))?.name).toBe('third');
});

it('§3 still commits every row when the batch succeeds', async () => {
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'b', data: { name: 'second-updated' } },
],
OPTS,
);

expect(results).toHaveLength(2);
expect((await stored('a'))?.name).toBe('first-updated');
expect((await stored('b'))?.name).toBe('second-updated');
});

/**
* The boundary the card's dispatch flagged as the likeliest surprise: a caller
* can already own a transaction (`beginTransaction()` is public API, exercised
* by `sql-driver-multiwrite-tx.test.ts`), and SQLite's pool hands out exactly
* ONE connection — so the wrapper must ride the caller's transaction rather
* than ask `this.knex` for a second connection, which would dead-lock.
*
* It rides it as a knex NESTED transaction (a `SAVEPOINT`), so the batch is
* undone as a unit while the caller's own earlier write inside the same
* transaction survives and the transaction stays usable. Joining WITHOUT the
* savepoint would leave the caller who catches the refusal and commits anyway
* holding exactly the partial batch this card is about.
*/
it('§4 undoes the batch inside a caller transaction, leaving that transaction usable', async () => {
const trx = await driver.beginTransaction();

// The caller's OWN write, before the batch. It must survive the batch's
// rollback — the savepoint undoes the batch, not the caller's work.
await driver.update('account', 'c', { name: 'caller-own-write' }, { ...OPTS, transaction: trx });

await expect(
driver.bulkUpdate('account', COLLIDING_BATCH, { ...OPTS, transaction: trx }),
).rejects.toThrow(/UNIQUE constraint failed|duplicate key value|ER_DUP_ENTRY/);

// The transaction survived the refusal, so the caller can still commit it.
await driver.commit(trx);

// 3 ⭐ — the batch's earlier row is undone…
expect((await stored('a'))?.name).toBe('first');
expect((await stored('b'))?.code).toBe('B-1');
// …and the caller's own write, which the batch never touched, is committed.
expect((await stored('c'))?.name).toBe('caller-own-write');
});

it('§5 issues nothing for an empty batch', async () => {
await expect(driver.bulkUpdate('account', [], OPTS)).resolves.toEqual([]);
});

it('§6 still skips a missing id rather than refusing the batch', async () => {
// The convention #13435 deferred to, unchanged: no throw, no placeholder in
// the returned array, and the real row is updated.
const results = await driver.bulkUpdate(
'account',
[
{ id: 'a', data: { name: 'first-updated' } },
{ id: 'no-such-row', data: { name: 'ignored' } },
],
OPTS,
);

expect(results).toHaveLength(1);
expect((await stored('a'))?.name).toBe('first-updated');
});
});
79 changes: 70 additions & 9 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7705,17 +7705,78 @@ export class SqlDriver implements IDataDriver {
}

/**
* Batch-update multiple records by ID.
* NOTE: Current implementation performs sequential updates for correctness.
* TODO: Optimize with SQL CASE statements or batched transactions for performance.
* Batch-update multiple records by ID — atomically, as ONE unit.
*
* [#13854] The loop itself is unchanged, and deliberately so: each id carries
* its OWN patch, so there is no single statement that expresses N different
* SET lists (the reason `bulkCreate`'s "send it as one INSERT" shape does not
* transfer). What this card changed is the transaction boundary around it.
*
* ## The defect
*
* With no boundary, every `update()` autocommitted its own row —
* {@link getBuilder} hands back a plain `this.knex` builder whenever
* `options.transaction` is unset — so a batch refused partway through left
* every row processed BEFORE the refusal permanently committed. The caller
* got an exception and the database was left in a state nobody declared:
* neither the pre-image nor the post-image. Same defect class #13340/#13435
* closed on `driver-memory`'s batch doors; `driver-turso` reaches this door
* through `super.bulkUpdate`, which is why the repair belongs to the parent
* and patching the subclass would only wait for the next subclass.
*
* ⚠️ A missing id is NOT the reachable failure on this door, and a pin that
* uses one measures nothing: `update()` issues an UPDATE matching zero rows
* and returns `null`, which this loop skips (`if (updated)`). That skip is
* the established convention #13435's note defers to, and it is unchanged
* here. The reachable refusals are the database's own — a unique violation on
* a later row, a NOT NULL violation, a bind refusal.
*
* {@link bulkDelete} needs no such wrapper: it is one `whereIn(...).delete()`
* per rotation shard, atomic per shard on its own.
*
* ## One runner, two meanings — and why the caller's transaction is fenced
*
* With no caller transaction, `this.knex.transaction()` opens the driver's
* own — the shape {@link upsert}'s `verifyIdentity` branch uses. Inside a
* caller transaction, `trx.transaction()` opens a knex NESTED transaction: a
* `SAVEPOINT`, released on success and rolled back to on failure, the exact
* primitive {@link attemptWithoutPoisoning} is built on and which this file
* has verified on PG 16 and on better-sqlite3 at pool `max: 1`, where the
* savepoint rides the parent's connection and never asks the pool for a
* second one.
*
* ⚠️ Joining the caller's transaction WITHOUT a savepoint — the posture
* {@link upsert} takes at its own boundary — would be wrong here, and the
* difference is N statements versus one. `upsert` reasons that "the statement
* is already transactional", which is true of one statement and false of this
* loop: a caller that catches the refusal and commits its transaction anyway
* lands precisely the partial batch this card is about. That is reachable on
* SQLite and MySQL; Postgres aborts the whole transaction on any statement
* error, so it is the one dialect where the hole was already shut by
* accident. The savepoint makes "every row applied in THIS call is undone"
* true whatever the caller wraps around it, while leaving that transaction
* usable — the rollback decision for the caller's OWN work stays the
* caller's, which is the half of `upsert`'s reasoning that does transfer.
*/
async bulkUpdate(object: string, updates: Array<{ id: string | number; data: Record<string, any> }>, options?: DriverOptions): Promise<Record<string, any>[]> {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, options);
if (updated) results.push(updated);
}
return results;
// An empty batch issues no statement, exactly as before: a BEGIN/COMMIT
// pair — and the pool checkout behind it — buys nothing for zero rows.
if (updates.length === 0) return [];

const applyAll = async (scoped: DriverOptions): Promise<Record<string, any>[]> => {
const results: Record<string, any>[] = [];
for (const { id, data } of updates) {
const updated = await this.update(object, id, data, scoped);
if (updated) results.push(updated);
}
return results;
};

// The `(caller's transaction) ?? this.knex` runner idiom this file already
// uses (see {@link collidingAutoNumberReservations}); here the choice of
// runner is also the choice between a transaction and a savepoint.
const runner: Knex | Knex.Transaction = (options?.transaction as Knex.Transaction | undefined) ?? this.knex;
return runner.transaction(async (trx) => applyAll({ ...options, transaction: trx }));
}

async bulkDelete(object: string, ids: Array<string | number>, options?: DriverOptions): Promise<void> {
Expand Down
Loading