Skip to content

Commit 693a729

Browse files
fix(objectql): a predicate delete walks the rows it matches as one set, one probe per relation per 500 rows (#27004) (#27073)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XKbBySzMyyJ7bHduW2KYXj8)_ Fixes #27004 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - **改了什么:** 按条件批量删除(`multi: true` 加过滤条件,流程里的批量 `delete_record`)不再对每条命中记录各跑一遍引用关系检查,而是把命中的记录当成一个集合,每个引用关系每 500 条只查一次。按 id 单条删除是“一条记录的集合”,走同一条路径。 - **为什么改:** #26930 之后,删 1,001 条被 3 个关系引用的记录要发 4,004 次依赖查询,约 1.7 秒(之前约 0.1 秒),rest 的 cap 测试在合并队列里逼近 5 秒超时,卡住了 #25521 cut 19/20。现在同样的删除只发 12 次查询。 - **风险与回滚:** restrict 拒绝、cascade 级联、set_null 清空、多值查找移除成员并写 `[]`、必填多值查找按行升级拒绝,这些结果逐条不变。唯一变化:命中的记录之间互相引用时(例如经理和下属都被这次删除命中,下属以 restrict 引用经理),现在视为同一个删除集合,不再拒绝;集合外的引用照旧拒绝。回滚 = revert 本 PR。 - **先看:** `packages/objectql/src/engine.ts` 的 `walkReferencingRelations`(按集合探查后逐条判定,`cleared` 记录已移除的多值成员)、`probeReferencingRowsOfSet`(分批探查与按 id 归属)、`collectCascadeDeleteSet`(按层批量收集)。 - **两分钟验证:** `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/builtin/delete-record-predicate-walk-batched.integration.test.ts`,预期 4 个测试全绿,第一个钉住依赖查询次数为 detail 6、link 3、tags 3。 - **需要你做的:** 无,席位落地。 ## What changes - `cascadeDeleteRelations` takes a ROOT SET. The by-id branch of `delete()` passes `[id]`; the predicate arm passes the ids it matched, once, with no per-row loop left. There is one walk; no new option on `engine.delete`. - `collectCascadeDeleteSet` collects breadth-first one depth at a time: the records found at one depth are probed together, per object and per cascading relation. Members and edges are the ones the record-by-record collection found, and `refuseRestrictsInsideOwnCascade` reads its members in the same discovery order from the edges. - `probeReferencingRowsOfSet` (new, private) is the dependents probe for a set: one read per 500 ids (`REFERENCE_PROBE_BATCH`, now also the predicate arm's `$in` delete batch), `$in` on a single-valued reference and an OR of each id's own `$contains` limbs on a multiple lookup (the spelling the nested-relation lowering already sends). Each row goes to the ids it references, by the #9362 element reading; a batch of one sends exactly the by-id filter. A single-valued row whose value names none of the batch makes that batch fall back to one probe per id. - `walkReferencingRelations` probes each relation once for the set, then judges and writes per record, in the set's order, with every rule as before (#22305 set exemption, #9688 per-row escalation with its count, #18408 member removal writing `[]`, the #12166 elevation records one per record per referenced object, the #24948 organization check per record). New `CascadeDeleteSet.cleared` records, per relation, the members whose walk has finished it, so a multiple-lookup row read once for the whole set has the earlier members left out of the remainder it is judged on and written with (`remainderAfterMemberRemoval(stored, id, cleared)`). - `refuseRestrictsInsideOwnCascade` batches its probes per relation over the members asking it, then judges in the same member and relation order as before. ## Tests - `packages/services/service-automation/src/builtin/delete-record-predicate-walk-batched.integration.test.ts` (new; real `SqlDriver` on better-sqlite3, the real `delete_record` executor), 4 cases: - **the count** (red first): 1,001 matched rows, the cap test's three relations (master_detail, lookup, multiple lookup). On `acc9c57d` the driver saw 2,002 / 1,001 / 1,001 dependents probes; on this head 6 / 3 / 3, and 3 `deleteMany` calls. - every matched row's outcome across the 500 boundary: details cascaded on both sides, lookups cleared, a multiple lookup spanning two batches keeps only its live member, an emptied one is written `[]`. - a required multiple lookup emptied only by two matched rows in different batches refuses with `DELETE_RESTRICTED`, `dependentCount: 1`, naming the record whose removal empties it (the same record the row-by-row walk named); nothing is deleted or cleared. - a matched row held only by another matched row through an authored `restrict` self-lookup: the delete goes through (count 2); a holder outside the matched rows still refuses. - Cases 2 and 3 pass on `acc9c57d` too (they pin unchanged outcomes); case 1 fails there; case 4 fails there (it pins the widening below). - `packages/objectql/src/engine-inspect-cascade-delete.test.ts`: its test double now answers `$in`, because the collection probes the records of one depth together. No assertion changed. ## Acceptance notes - **The widening (why `Clause-②: yes (widening)`).** One walk of one set makes the matched rows members of it. A matched row that only other matched rows hold (through a `restrict`, or a required lookup's escalated `set_null`) refuses nothing, and a matched row another matched row's `set_null` points at is not cleared before it is deleted: the #22305 rule a cascade already applies to the rows it deletes. The row-by-row walk refused the first case whenever it met the held row first, and cleared the second with an `update` (hooks and all) on a row about to go. A holder outside the matched rows still refuses, unchanged. The #26871 changeset (still unreleased) said only that a `restrict` on a matched row refuses. - **Which refusal is reported when several apply.** Unchanged for a by-id delete. For a predicate delete the relations are now taken in order and the matched records inside each relation, so with refusals on two different relations the first relation's is reported, where the row-by-row walk reported the first matched record's. Code, wording rule and count rule are the same: one record named, the rows refusing over that record counted. - **A matched row another matched row cascades into** (a self master_detail where parent and child both match) is now deleted by the predicate delete itself rather than by a nested cascade delete, so its own delete hooks run once, from the predicate delete's per-row dispatch, and the answer counts it. Read from the code (every root is marked entered before the walk), not pinned by a test. - **Cascade children still go through their own `delete()`, one per row,** with their hooks and their own walk, as before; only the probes are batched. - **Cost that remains.** The #12166 elevation ledger still files one record per deleted record per referenced object (1,002 log lines in the cap test, on base and head alike), by design. - **Overlap.** No open PR touches `packages/objectql/src/engine*.ts` (read before `pr_create`); #26831 merged and is in this head. ## Verification Final head: `cec640f0` (merge of `origin/main` at `cb8988ee`). - **Driver-call count** (the new pin, red first): `acc9c57d` 2,002 / 1,001 / 1,001 dependents probes on the three relations; this head 6 / 3 / 3. On the base dist the pin file reads `Tests 2 failed | 2 passed (4)` (the count and the set-exemption case); on this head `4 passed`. - **objectql suite** on this head: `Test Files 476 passed (476)`, `Tests 8888 passed (8888)`. Every existing walk pin (`engine-cascade-delete-atomic`, `engine-cascade-delete*`, `engine-cascade-delete-sibling-restrict`, `summary-rollup`, `engine-nested-relation-lowering`) is green with no assertion changed. - **Flow files** (service-automation, real SqlDriver): the new file, the #26871 `delete-record-predicate-referential-walk` and the #27000 `predicate-write-summary-recompute`: `Tests 12 passed (12)`. - **Dependents** (pre-merge head `ef03ae64`, built closures): the 40 test files in 13 packages that delete through a relation: metadata 22, metadata-protocol 246, plugin-audit 138, plugin-auth 47, plugin-hono-server 49, plugin-security 433, plugin-sharing 9, rest 221 (+12 skipped live cells), runtime 9, service-automation 69, service-messaging 20, service-settings 8, service-storage 141: all exit 0. - **Typecheck / lint:** `pnpm --filter @objectstack/objectql typecheck` and `pnpm --filter @objectstack/service-automation typecheck` exit 0; eslint on the three touched files exit 0. - **Gates:** the 65 commands `dispatch-gates.mjs --commands` derives, run on this head, every one exit 0; `--ran` reconciliation: `65 derived famil(ies) accounted for — 65 run, 0 NOT-MEASURED`. `check-plugin-teardown-shape --self-test` needed the documented shallow-clone fetch first. <details><summary>The rest cap test (data-nested-object-door.test.ts "the cap: …"), alone, --maxWorkers=2, base and head objectql dist alternated, 5 runs each (shared box, gate loop running beside it)</summary> ``` base-r1 exit 0 cap passed 2632 ms · file tests 18 sum 2844 ms head-r1 exit 0 cap passed 306 ms · file tests 18 sum 505 ms base-r2 exit 0 cap passed 2451 ms · file tests 18 sum 2664 ms head-r2 exit 0 cap passed 308 ms · file tests 18 sum 524 ms base-r3 exit 0 cap passed 2526 ms · file tests 18 sum 2730 ms head-r3 exit 0 cap passed 419 ms · file tests 18 sum 638 ms base-r4 exit 0 cap passed 2503 ms · file tests 18 sum 2710 ms head-r4 exit 0 cap passed 341 ms · file tests 18 sum 615 ms base-r5 exit 0 cap passed 2893 ms · file tests 18 sum 3129 ms head-r5 exit 0 cap passed 358 ms · file tests 18 sum 581 ms ``` Base 2,451 to 2,893 ms, head 306 to 419 ms: about 7.5× less. Before #26930 the test took about 0.1 s; what is left is the elevation ledger's 1,002 log lines and the matched-id read. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5ktxGoJfsedLmkFjdk)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5ktxGoJfsedLmkFjdk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cbb3a1a commit 693a729

4 files changed

Lines changed: 709 additions & 244 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
}

0 commit comments

Comments
 (0)