Skip to content

Commit 2452967

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/project-thread-w7hign
2 parents 2865357 + fbc1656 commit 2452967

8 files changed

Lines changed: 985 additions & 256 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): a predicate delete walks the rows it matches as one set: each relation pointing at the object is asked once per 500 matched rows, not once per row (#27004)
6+
7+
Clause-②: yes (widening) — a predicate delete (`engine.delete(object, { where, multi: true })`, a flow's `delete_record` with `multi: true`) whose matched rows are held only by OTHER matched rows (a report and its manager through a `restrict` self-lookup, both matched) now deletes them; the row-by-row walk refused it, or not, depending on which row it met first. Nothing is narrowed.
8+
Metadata-protocol: none
9+
10+
- **Before.** The predicate delete ran the by-id referential walk once per matched row, so its cost was matched rows times referencing relations: 1,001 rows of an object three relations point at took 4,004 dependents probes, about 1.7 s on SQLite.
11+
- **Now.** One walk, rooted at the whole matched set (a by-id delete is a set of one). Each referencing relation is probed once per 500 matched rows with `$in` (a multiple lookup ORs each id's `$contains`), and each row is then judged and written as before: `restrict` refuses, `cascade` deletes (each child through its own delete, hooks and all), `set_null` clears, a multiple lookup loses only the deleted members and an emptied set is written as `[]`. The same 1,001-row delete takes 12 probes.
12+
- **Unchanged.** A refusal's code and wording: it still names one record and counts the rows that refuse over that record (for a required multiple lookup, the rows the removal would empty). The atomic and split plans, the 500-id `$in` delete batches, the roll-up recompute after the delete, and the filter of every probe a by-id delete sends.
13+
- **Who sees it.** A bulk delete that removes rows holding each other: a matched row that another matched row holds is in the delete's own set, so it neither refuses nor is cleared first (the rule a cascade already applies to the rows it deletes). A holder outside the matched rows still refuses.

‎packages/objectql/src/engine-inspect-cascade-delete.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ function makeDriver() {
3333
if (!where || typeof where !== 'object') return true;
3434
for (const [k, v] of Object.entries(where as Row)) {
3535
if (k.startsWith('$')) continue;
36+
// [#27004] The collection probes the records of one depth together.
37+
if (v && typeof v === 'object' && Array.isArray((v as Row).$in)) {
38+
if (!((v as Row).$in as unknown[]).some((x) => String(x) === String(row[k]))) return false;
39+
continue;
40+
}
3641
const exp = v && typeof v === 'object' && '$eq' in (v as Row) ? (v as Row).$eq : v;
3742
if ((row[k] ?? null) !== (exp ?? null)) return false;
3843
}

‎packages/objectql/src/engine.ts‎

Lines changed: 429 additions & 244 deletions
Large diffs are not rendered by default.
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* A `delete_record` node with `multi: true` walks the rows its filter matches
5+
* as ONE set: each relation pointing at the deleted object is asked once per
6+
* batch of matched ids, not once per matched row, and every row still gets
7+
* the outcome its declared `deleteBehavior` gives it.
8+
*
9+
* The engine's predicate arm used to run the by-id walk once per matched id,
10+
* so its cost was matched rows times referencing relations: deleting 1,001
11+
* rows of an object three relations point at made 4,004 dependents probes and
12+
* took about 1.7 s on SQLite, where the same delete took about 0.1 s before
13+
* the walk existed. The walk now takes a root set; a by-id delete is a set of
14+
* one.
15+
*
16+
* One walk of one set also means the matched rows are members of it: a
17+
* matched row held only by other matched rows refuses nothing and is cleared
18+
* of nothing, the #22305 set exemption, where the row-by-row walk refused or
19+
* not depending on which row it met first.
20+
*
21+
* Composition: the real stack `delete-record-predicate-referential-walk`
22+
* boots (`ObjectKernel`, `ObjectQLPlugin`, `driver-sql` on better-sqlite3
23+
* `:memory:`, the real `delete_record` executor). Driver calls are counted on
24+
* the driver itself; rows are read out of the driver, not through the engine.
25+
*/
26+
27+
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
28+
import type { FilterCondition } from '@objectstack/spec/data';
29+
import { ObjectKernel } from '@objectstack/core';
30+
import { ObjectQLPlugin, type ObjectQL } from '@objectstack/objectql';
31+
import { SqlDriver } from '@objectstack/driver-sql';
32+
import { AutomationServicePlugin } from '../plugin.js';
33+
import { AutomationEngine } from '../engine.js';
34+
import { registerCrudNodes } from './crud-nodes.js';
35+
import { installDefaultOrganization } from '../default-organization.testkit.js';
36+
37+
const SYS = { isSystem: true } as const;
38+
39+
/** Matched rows: more than two of the walk's 500-id batches. */
40+
const N = 1_001;
41+
42+
const PARENT = {
43+
name: 'pin_batch_parent',
44+
label: 'Pin batch parent',
45+
fields: {
46+
title: { name: 'title', label: 'Title', type: 'text' },
47+
grp: { name: 'grp', label: 'Group', type: 'text' },
48+
},
49+
};
50+
/** The three relations of the rest cap test: a master_detail, a lookup and a multiple lookup. */
51+
const DETAIL = {
52+
name: 'pin_batch_detail',
53+
label: 'Pin batch detail',
54+
fields: {
55+
title: { name: 'title', label: 'Title', type: 'text' },
56+
parent: { name: 'parent', label: 'Parent', type: 'master_detail', reference: 'pin_batch_parent' },
57+
},
58+
};
59+
const LINK = {
60+
name: 'pin_batch_link',
61+
label: 'Pin batch link',
62+
fields: {
63+
title: { name: 'title', label: 'Title', type: 'text' },
64+
parent: { name: 'parent', label: 'Parent', type: 'lookup', reference: 'pin_batch_parent' },
65+
},
66+
};
67+
const TAGS = {
68+
name: 'pin_batch_tags',
69+
label: 'Pin batch tags',
70+
fields: {
71+
title: { name: 'title', label: 'Title', type: 'text' },
72+
parents: { name: 'parents', label: 'Parents', type: 'lookup', reference: 'pin_batch_parent', multiple: true },
73+
},
74+
};
75+
76+
/** A second parent with a REQUIRED multiple lookup pointing at it: the per-row escalation. */
77+
const HOLDER = {
78+
name: 'pin_batch_holder',
79+
label: 'Pin batch holder',
80+
fields: {
81+
title: { name: 'title', label: 'Title', type: 'text' },
82+
grp: { name: 'grp', label: 'Group', type: 'text' },
83+
},
84+
};
85+
const MEMBERS = {
86+
name: 'pin_batch_members',
87+
label: 'Pin batch members',
88+
fields: {
89+
title: { name: 'title', label: 'Title', type: 'text' },
90+
holders: { name: 'holders', label: 'Holders', type: 'lookup', reference: 'pin_batch_holder', multiple: true, required: true },
91+
},
92+
};
93+
94+
/** A self-reference with an authored `restrict`: a report holds its manager. */
95+
const STAFF = {
96+
name: 'pin_batch_staff',
97+
label: 'Pin batch staff',
98+
fields: {
99+
title: { name: 'title', label: 'Title', type: 'text' },
100+
grp: { name: 'grp', label: 'Group', type: 'text' },
101+
manager: { name: 'manager', label: 'Manager', type: 'lookup', reference: 'pin_batch_staff', deleteBehavior: 'restrict' },
102+
},
103+
};
104+
105+
function makeLogger(): any {
106+
const l: any = { info() {}, warn() {}, error() {}, debug() {}, trace() {}, fatal() {} };
107+
l.child = () => l;
108+
return l;
109+
}
110+
111+
type Executor = { execute: (node: any, variables: Map<string, unknown>, context: any) => Promise<any> };
112+
113+
describe('[#27004] a multi delete_record walks its matched rows as a set', () => {
114+
let kernel: ObjectKernel;
115+
let ql: ObjectQL;
116+
let driver: SqlDriver;
117+
let deleteRecord: Executor;
118+
119+
beforeAll(async () => {
120+
kernel = new ObjectKernel({ logger: { level: 'fatal' } });
121+
await kernel.use(new ObjectQLPlugin());
122+
await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' }));
123+
await kernel.bootstrap();
124+
ql = kernel.getService<ObjectQL>('objectql');
125+
126+
driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
127+
await driver.connect();
128+
ql.registerDriver(driver, true);
129+
for (const o of [PARENT, DETAIL, LINK, TAGS, HOLDER, MEMBERS, STAFF]) ql.registry.registerObject(o as any, 'pin-batch-delete', 'pin-batch-delete');
130+
await ql.syncSchemas();
131+
await installDefaultOrganization(ql);
132+
133+
const executors = new Map<string, Executor>();
134+
const capture = new Proxy(new AutomationEngine(makeLogger()), {
135+
get(target, key, receiver) {
136+
if (key === 'registerNodeExecutor') return (e: any) => { executors.set(e.type, e); };
137+
return Reflect.get(target, key, receiver);
138+
},
139+
});
140+
registerCrudNodes(capture as AutomationEngine, { logger: makeLogger(), getService: (n: string) => (n === 'data' ? ql : undefined) } as any);
141+
deleteRecord = executors.get('delete_record')!;
142+
expect(deleteRecord, 'control: the delete_record executor was registered').toBeDefined();
143+
}, 60_000);
144+
145+
afterAll(async () => {
146+
try { await kernel?.shutdown(); } catch { /* best-effort teardown */ }
147+
});
148+
149+
const insert = async (object: string, data: Record<string, unknown>): Promise<string> =>
150+
((await ql.insert(object, data, { context: SYS })) as any).id;
151+
/** `count` rows of `object` in one group, inserted 100 at a time; their ids in order. */
152+
const insertMany = async (object: string, grp: string, count: number): Promise<string[]> => {
153+
const ids: string[] = [];
154+
for (let i = 0; i < count; i += 100) {
155+
const batch = Array.from({ length: Math.min(100, count - i) }, (_, j) => ({ id: `${grp}_${i + j}`, title: `${grp} ${i + j}`, grp }));
156+
await ql.insert(object, batch, { context: SYS });
157+
ids.push(...batch.map((r) => r.id));
158+
}
159+
return ids;
160+
};
161+
/** Rows of `object`, read from the driver with no engine in between. */
162+
const rows = async (object: string, where: FilterCondition = {}): Promise<any[]> =>
163+
(await driver.find(object, { where })) as any[];
164+
const sweep = (objectName: string, grp: string) => deleteRecord.execute(
165+
{ id: 'sweep', type: 'delete_record', config: { objectName, filter: { grp }, multi: true } },
166+
new Map(),
167+
{ runAs: 'system', flowName: 'pin_batch_sweep' },
168+
);
169+
170+
it('asks each referencing relation once per 500 matched ids, not once per matched row', async () => {
171+
await insertMany(PARENT.name, 'counted', N);
172+
const finds = vi.spyOn(driver, 'find');
173+
const deletes = vi.spyOn(driver, 'deleteMany');
174+
try {
175+
const res: any = await sweep(PARENT.name, 'counted');
176+
expect(res.success, JSON.stringify(res)).toBe(true);
177+
const probesOf = (object: string) => finds.mock.calls.filter((call) => call[0] === object).length;
178+
// ⌈1,001 / 500⌉ = 3 batches. The master_detail is read twice per batch
179+
// (the cascade set is collected, then walked); the lookup and the
180+
// multiple lookup once each. Per row it was 2,002, 1,001 and 1,001.
181+
expect({ detail: probesOf(DETAIL.name), link: probesOf(LINK.name), tags: probesOf(TAGS.name) })
182+
.toEqual({ detail: 6, link: 3, tags: 3 });
183+
expect(deletes.mock.calls.filter((call) => call[0] === PARENT.name)).toHaveLength(3);
184+
} finally {
185+
finds.mockRestore();
186+
deletes.mockRestore();
187+
}
188+
expect(await rows(PARENT.name, { grp: 'counted' })).toEqual([]);
189+
});
190+
191+
it('every matched row still gets its own outcome, across batch boundaries', async () => {
192+
const ids = await insertMany(PARENT.name, 'mixed', N);
193+
const kept = await insert(PARENT.name, { title: 'k', grp: 'kept' });
194+
// Details of rows in the first and the second batch, and of the kept row.
195+
await insert(DETAIL.name, { title: 'd-0', parent: ids[0] });
196+
await insert(DETAIL.name, { title: 'd-700', parent: ids[700] });
197+
const keptDetail = await insert(DETAIL.name, { title: 'd-k', parent: kept });
198+
const l600 = await insert(LINK.name, { title: 'l-600', parent: ids[600] });
199+
const l1000 = await insert(LINK.name, { title: 'l-1000', parent: ids[1000] });
200+
const keptLink = await insert(LINK.name, { title: 'l-k', parent: kept });
201+
// One set spanning two batches and the kept row; one emptied by the delete.
202+
const spanning = await insert(TAGS.name, { title: 't-span', parents: [ids[1], ids[900], kept] });
203+
const emptied = await insert(TAGS.name, { title: 't-empty', parents: [ids[2], ids[3]] });
204+
205+
const res: any = await sweep(PARENT.name, 'mixed');
206+
expect(res.success, JSON.stringify(res)).toBe(true);
207+
208+
expect(await rows(PARENT.name, { grp: 'mixed' })).toEqual([]);
209+
expect((await rows(PARENT.name, { grp: 'kept' })).map((r) => r.id)).toEqual([kept]);
210+
// master_detail: cascaded on both sides of the batch boundary.
211+
expect((await rows(DETAIL.name)).map((r) => r.id)).toEqual([keptDetail]);
212+
// set_null lookup: cleared, the kept row's untouched.
213+
const links = new Map((await rows(LINK.name)).map((r) => [r.id, r.parent ?? null]));
214+
expect([links.get(l600), links.get(l1000), links.get(keptLink)]).toEqual([null, null, kept]);
215+
// set_null multiple lookup: every deleted member removed, the kept one
216+
// stays, and an emptied set is written as `[]`, never `null`.
217+
const tags = new Map((await rows(TAGS.name)).map((r) => [r.id, r.parents]));
218+
expect(tags.get(spanning)).toEqual([kept]);
219+
expect(tags.get(emptied)).toEqual([]);
220+
});
221+
222+
it('a required multiple lookup emptied only by the whole set refuses, naming the row whose removal empties it', async () => {
223+
const ids = await insertMany(HOLDER.name, 'held', 600);
224+
const other = await insert(HOLDER.name, { title: 'o', grp: 'other' });
225+
// Keeps `other`: takes the member removal, refuses nothing.
226+
await insert(MEMBERS.name, { title: 'm-keeps', holders: [ids[10], other] });
227+
// Every member is matched, one in each batch: emptied by the delete.
228+
await insert(MEMBERS.name, { title: 'm-empties', holders: [ids[20], ids[550]] });
229+
230+
const node: any = await sweep(HOLDER.name, 'held');
231+
expect(node.success).toBe(false);
232+
expect(node.innerRefusal).toMatchObject({ code: 'DELETE_RESTRICTED' });
233+
// The engine's own error, for its structured half: the count is the rows
234+
// the removal would empty, and the record named is the one whose removal
235+
// empties the set, as when the rows were walked one by one.
236+
const err: any = await ql.delete(HOLDER.name, { where: { grp: 'held' }, multi: true, context: SYS }).catch((e: unknown) => e);
237+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: MEMBERS.name, dependentCount: 1 });
238+
expect(err.developerMessage).toContain(`Cannot delete ${HOLDER.name} (${ids[550]}): 1 dependent ${MEMBERS.name} record(s)`);
239+
// One unit of work: nothing deleted, nothing cleared.
240+
expect(await rows(HOLDER.name, { grp: 'held' })).toHaveLength(600);
241+
const members = new Map((await rows(MEMBERS.name)).map((r) => [r.title, r.holders]));
242+
expect(members.get('m-keeps')).toEqual([ids[10], other]);
243+
expect(members.get('m-empties')).toEqual([ids[20], ids[550]]);
244+
});
245+
246+
it('a matched row held only by other matched rows is in the set: the delete goes through, whatever the order', async () => {
247+
// The manager first, so a walk of one row at a time met the report while
248+
// it still stood beside a set of one, and refused.
249+
const manager = await insert(STAFF.name, { title: 'm', grp: 'team' });
250+
const report = await insert(STAFF.name, { title: 'r', grp: 'team', manager });
251+
const removed = await ql.delete(STAFF.name, { where: { grp: 'team' }, multi: true, context: SYS });
252+
expect(removed).toBe(2);
253+
expect(await rows(STAFF.name, { id: { $in: [manager, report] } })).toEqual([]);
254+
255+
// A holder outside the matched rows still refuses, and nothing is deleted.
256+
const held = await insert(STAFF.name, { title: 'h', grp: 'team' });
257+
await insert(STAFF.name, { title: 'o', grp: 'elsewhere', manager: held });
258+
const err: any = await ql.delete(STAFF.name, { where: { grp: 'team' }, multi: true, context: SYS }).catch((e: unknown) => e);
259+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', dependentObject: STAFF.name, dependentCount: 1 });
260+
expect((await rows(STAFF.name, { grp: 'team' })).map((r) => r.id)).toEqual([held]);
261+
});
262+
});

0 commit comments

Comments
 (0)