fix(objectql)!: a predicate delete honours deleteBehavior for every matched row: restrict refuses, master_detail cascades, set_null clears (#26871)

.changeset/26871-predicate-delete-referential-walk.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql)!: a predicate delete (`multi: true` with a filter) honours `deleteBehavior` for every row it matches: a `restrict` refuses, a `master_detail` cascades, a `set_null` clears (#26871)
+
+Clause-②: no (narrowing) — a predicate delete (`engine.delete(object, { where, multi: true })`, which a flow's `delete_record` node with `multi: true` and a filter reaches) refuses with 409 `DELETE_RESTRICTED` where it succeeded: when a row it matches is held by a `restrict` reference (authored, or a required lookup's `set_null`, which is refused as `restrict`). Nothing is widened.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a declared deleteBehavior is now honoured on the predicate delete path too. The refusal is the existing DELETE_RESTRICTED, so there is nothing for a migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing at the predicate delete (`@objectstack/objectql`), served by every caller of `delete` with `multi: true` and a filter, the flow `delete_record` node among them.
+
+- **Before.** A predicate delete went straight to the driver's bulk delete and ran no referential walk. No `restrict` refused it, the matched rows' `master_detail` children were left behind, and `set_null` lookups kept pointing at deleted rows. The same rows deleted one at a time by id (`DELETE /api/v1/data/:object/:id`, the bulk `deleteManyData` door) refused or cascaded.
+- **Now.** When any relation of the delete family (`master_detail`, `lookup`, `tree`) points at the object, the predicate delete reads the rows it matches under the caller's scope, runs the by-id delete's referential walk for each, and then deletes exactly those rows, all in one unit of work on a single datasource. A `restrict` on any matched row refuses the whole delete with the existing `409 DELETE_RESTRICTED`, and nothing is deleted; `master_detail` children (and every `cascade`) are deleted; `set_null` references are cleared. The answer is still the count of rows the delete removed.
+- **Unchanged.** An object no relation points at keeps the single bulk delete: no row is read and no transaction is opened. A cascade that reaches another datasource runs without a transaction and warns once, as the by-id delete does. A predicate delete of `sys_organization` is still refused.
+- **Who sees it.** A flow (or in-process caller) that bulk-deletes rows held by a `restrict` reference now fails instead of deleting them; one that bulk-deletes masters now deletes their details and clears their lookups instead of leaving them dangling.
+
+**The remedy.** Delete or reassign the restricting rows first, then run the bulk delete. If the referencing rows are meant to go with the rows deleted, declare `deleteBehavior: 'cascade'` on that reference.

packages/objectql/src/engine-cascade-delete-atomic.test.ts

@@ -68,6 +68,11 @@ function makeDriver(opts: { transactional?: boolean } = {}) {
     if (!where || typeof where !== 'object') return true;
     for (const [k, v] of Object.entries(where)) {
       if (k.startsWith('$')) continue;
+      // [#26871] The predicate delete removes the rows it walked by id set.
+      if (v && typeof v === 'object' && Array.isArray((v as any).$in)) {
+        if (!(v as any).$in.some((x: unknown) => String(x) === String(row[k]))) return false;
+        continue;
+      }
       const exp = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v;
       if ((row[k] ?? null) !== (exp ?? null)) return false;
     }
@@ -404,16 +409,168 @@ describe('a plain non-cascade delete is unchanged (#7413 control)', () => {
     expect(rec.at('warn')).toHaveLength(0);
   });
 
-  it('opens no transaction when the object HAS dependents declared but the delete is by predicate', async () => {
-    // The predicate branch never called `cascadeDeleteRelations` and still does
-    // not — the wrap is the by-id branch's alone.
-    const { engine, driver } = await makeEngine([parent, kid]);
-    await engine.insert('parent', { name: 'P' });
+  it('opens no transaction for a predicate delete when no relation points at the object', async () => {
+    // [#26871] The predicate twin of the control above: nothing to walk, so the
+    // predicate arm keeps its single `deleteMany` — no id resolution, no wrap.
+    const { engine, driver, rec } = await makeEngine([loner]);
+    await engine.insert('loner', { name: 'solo' });
+    await engine.insert('loner', { name: 'solo' });
     driver.writes.length = 0;
 
-    await engine.delete('parent', { where: { name: 'P' }, multi: true } as any);
+    const removed = await engine.delete('loner', { where: { name: 'solo' }, multi: true });
 
+    expect(removed).toBe(2);
     expect(driver.begun).toHaveLength(0);
+    expect(driver.rowsOf('loner')).toHaveLength(0);
+    for (const w of driver.writes) expect(w.transaction).toBeUndefined();
+    expect(rec.at('warn')).toHaveLength(0);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 3b. [#26871] A predicate delete walks every matched row, as one unit of work.
+// ---------------------------------------------------------------------------
+
+/** A `master_detail` detail of `parent`: the default behaviour is `cascade`. */
+const detail = {
+  name: 'detail',
+  label: 'Detail',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    parent: { name: 'parent', type: 'master_detail' as const, reference: 'parent' },
+  },
+};
+/** An authored `restrict` lookup to `parent`. */
+const guard = {
+  name: 'guard',
+  label: 'Guard',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    parent: { name: 'parent', type: 'lookup' as const, reference: 'parent', deleteBehavior: 'restrict' },
+  },
+};
+
+describe('[#26871] a predicate delete runs the referential walk for every matched row', () => {
+  it('a restrict on one matched row refuses the whole delete, and nothing is deleted', async () => {
+    const { engine, driver } = await makeEngine([parent, detail, guard]);
+    const p1 = await engine.insert('parent', { name: 'doomed' });
+    const p2 = await engine.insert('parent', { name: 'doomed' });
+    await engine.insert('parent', { name: 'kept' });
+    await engine.insert('detail', { parent: p1.id });
+    await engine.insert('guard', { parent: p2.id });
+
+    await expect(engine.delete('parent', { where: { name: 'doomed' }, multi: true }))
+      .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, object: 'parent', dependentObject: 'guard' });
+
+    // p1's detail was cascaded before p2's restrict refused; the one unit of
+    // work puts it back, so the 409 means nothing changed.
+    expect(driver.rowsOf('parent')).toHaveLength(3);
+    expect(driver.rowsOf('detail')).toHaveLength(1);
+    expect(driver.rowsOf('guard')).toHaveLength(1);
+    expect(driver.begun).toHaveLength(1);
+    expect(driver.rolledBack).toHaveLength(1);
+    expect(driver.committed).toHaveLength(0);
+  });
+
+  it('cascades master_detail children and clears set_null references of every matched row', async () => {
+    const { engine, driver } = await makeEngine([parent, detail, link]);
+    const p1 = await engine.insert('parent', { name: 'doomed' });
+    const p2 = await engine.insert('parent', { name: 'doomed' });
+    const kept = await engine.insert('parent', { name: 'kept' });
+    await engine.insert('detail', { parent: p1.id });
+    await engine.insert('detail', { parent: p2.id });
+    const keptDetail = await engine.insert('detail', { parent: kept.id });
+    const l1 = await engine.insert('link', { parent: p1.id });
+    const l2 = await engine.insert('link', { parent: p2.id });
+    const keptLink = await engine.insert('link', { parent: kept.id });
+
+    const removed = await engine.delete('parent', { where: { name: 'doomed' }, multi: true });
+
+    expect(removed).toBe(2);
+    expect(driver.rowsOf('parent').map((r: any) => r.id)).toEqual([kept.id]);
+    expect(driver.rowsOf('detail').map((r: any) => r.id)).toEqual([keptDetail.id]);
+    const links = new Map(driver.rowsOf('link').map((r: any) => [r.id, r.parent]));
+    expect(links.get(l1.id)).toBeNull();
+    expect(links.get(l2.id)).toBeNull();
+    expect(links.get(keptLink.id)).toBe(kept.id);
+    expect(driver.begun).toHaveLength(1);
+    expect(driver.committed).toHaveLength(1);
+  });
+
+  it('routes the walk and the predicate delete onto ONE transaction handle', async () => {
+    const { engine, driver } = await makeEngine([parent, detail, link]);
+    const p = await engine.insert('parent', { name: 'doomed' });
+    await engine.insert('detail', { parent: p.id });
+    await engine.insert('link', { parent: p.id });
+    driver.writes.length = 0;
+
+    await engine.delete('parent', { where: { name: 'doomed' }, multi: true });
+
+    expect(driver.begun).toHaveLength(1);
+    const handle = driver.begun[0];
+    expect(driver.writes.some((w: any) => w.object === 'parent' && w.op === 'delete')).toBe(true);
+    expect(driver.writes.some((w: any) => w.object === 'detail' && w.op === 'delete')).toBe(true);
+    expect(driver.writes.some((w: any) => w.object === 'link' && w.op === 'update')).toBe(true);
+    for (const w of driver.writes) expect(w.transaction).toBe(handle);
+  });
+
+  it('deletes only the rows it walked, even when the walk makes another row match the predicate', async () => {
+    // A self-referencing set_null lookup: clearing `boss` on the report makes
+    // it match `{ boss: null }`. The delete removes the rows it resolved and
+    // walked, never a row the walk itself brought into the predicate.
+    const node = {
+      name: 'node',
+      label: 'Node',
+      fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        name: { name: 'name', type: 'text' as const },
+        boss: { name: 'boss', type: 'lookup' as const, reference: 'node' },
+      },
+    };
+    const { engine, driver } = await makeEngine([node]);
+    const root = await engine.insert('node', { name: 'root', boss: null });
+    const report = await engine.insert('node', { name: 'report', boss: root.id });
+
+    const removed = await engine.delete('node', { where: { boss: null }, multi: true });
+
+    expect(removed).toBe(1);
+    expect(driver.rowsOf('node').map((r: any) => [r.id, r.boss ?? null])).toEqual([[report.id, null]]);
+  });
+
+  it('a detail that also restricts its own master refuses the predicate delete, as it does by id (#26718)', async () => {
+    // The predicate arm runs the by-id walk itself, so every rule the walk
+    // judges holds here too: the detail is in its master's cascade set by its
+    // own master_detail, and its restrict to that master still refuses.
+    const pledge = {
+      name: 'pledge',
+      label: 'Pledge',
+      fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        parent: { name: 'parent', type: 'master_detail' as const, reference: 'parent' },
+        guard: { name: 'guard', type: 'lookup' as const, reference: 'parent', deleteBehavior: 'restrict' },
+      },
+    };
+    const { engine, driver } = await makeEngine([parent, pledge]);
+    const p = await engine.insert('parent', { name: 'doomed' });
+    await engine.insert('pledge', { parent: p.id, guard: p.id });
+
+    await expect(engine.delete('parent', { where: { name: 'doomed' }, multi: true }))
+      .rejects.toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'pledge' });
+    expect(driver.rowsOf('parent')).toHaveLength(1);
+    expect(driver.rowsOf('pledge')).toHaveLength(1);
+  });
+
+  it('a matched set of zero rows walks nothing and deletes nothing', async () => {
+    const { engine, driver } = await makeEngine([parent, detail]);
+    const p = await engine.insert('parent', { name: 'kept' });
+    await engine.insert('detail', { parent: p.id });
+    driver.writes.length = 0;
+
+    const removed = await engine.delete('parent', { where: { name: 'nobody' }, multi: true });
+
+    expect(removed).toBe(0);
+    expect(driver.writes).toEqual([]);
+    expect(driver.rowsOf('detail')).toHaveLength(1);
   });
 });
 

packages/objectql/src/engine-nested-relation-lowering.test.ts

@@ -203,15 +203,21 @@ describe('[#20802] the nested-relation form is lowered at the engine\'s where se
       expect(outer.length, verb).toBeGreaterThan(0);
       for (const read of outer) expect(read.ast.where, verb).toEqual(lowered);
     }
-    for (const [verb, call] of [
-      ['update', () => engine.update(OBJECT, { title: 'x' }, { where, multi: true })],
-      ['delete', () => engine.delete(OBJECT, { where, multi: true })],
-    ] as const) {
-      rec.writes.length = 0;
-      await call();
-      expect(rec.writes, verb).toHaveLength(1);
-      expect(rec.writes[0].ast.where, verb).toEqual(lowered);
-    }
+    rec.writes.length = 0;
+    await engine.update(OBJECT, { title: 'x' }, { where, multi: true });
+    expect(rec.writes, 'update').toHaveLength(1);
+    expect(rec.writes[0].ast.where, 'update').toEqual(lowered);
+    // [#26871] Relations point at this ledger (the owner's `account`, its own
+    // `parent` tree), so a predicate delete first reads the rows it will walk
+    // and then deletes those rows by id. The predicate reaches the driver on
+    // that read, lowered; nothing matches here, so nothing is written.
+    rec.reads.length = 0;
+    rec.writes.length = 0;
+    await engine.delete(OBJECT, { where, multi: true });
+    const resolution = rec.reads.filter((r) => r.object === OBJECT && Array.isArray(r.ast?.fields) && r.ast.fields.includes('id'));
+    expect(resolution, 'delete').toHaveLength(1);
+    expect(resolution[0].ast.where, 'delete').toEqual(lowered);
+    expect(rec.writes, 'delete').toHaveLength(0);
     expect(engine.judgeFilter(OBJECT, where)).toEqual({ ok: true });
     expect(engine.judgeFilter(OBJECT, { owners: { region: 'NA' } })).toEqual({ ok: true });
   });