Skip to content

Commit 4c9d0d3

Browse files
docs(changeset): correct the objectql release note for the set-rooted predicate delete walk (#27288)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgKURKuRGF1ewEVVZKx7U1i4)_ Fixes #27201 Clause-②: yes (widening) — unchanged from the landed changeset; the review 6466053903 found the line right and this card keeps it. Metadata-protocol: none ## 维护者速读 - **改了什么**:改正 `@objectstack/objectql` 待发布说明 `.changeset/27004-predicate-delete-walk-as-set.md` 里三处与代码不符的说法,并加一条测试钉住其中一项行为。引擎代码一行未动。 - **为什么改**:这份 changeset 会在 Version Packages(#26517)合并时变成 objectql 的 CHANGELOG;合并后复核(6466053903)判定其中三句失实,必须在发版前改正。 - **风险与回滚**:只改发版说明文字与一个测试;发布级别 `patch` 和两行声明保持不变。回滚即 revert 本 PR,说明退回旧的错误文本。 - **先看**:① `.changeset/27004-predicate-delete-walk-as-set.md` 新增的三条要点和 `Clause-②:` 行里改过的例子;② `packages/objectql/src/engine-cascade-delete-atomic.test.ts` 新增的一条用例。 - **两分钟验证**:`pnpm --filter @objectstack/objectql exec vitest run src/engine-cascade-delete-atomic.test.ts`,预期 20 条全过。 - **需要你做的**:无,席位落地。`Check Changeset` 对只改现有 changeset 的 PR 按设计保持红(非必需检查,不打 `skip-changeset`),改正的书面确认由席位写在本 PR 上。 ## What changes The pending changeset of #27073 (card #27004) becomes the `@objectstack/objectql` release note when Version Packages (#26517) lands. The post-landing contract review of #27073 (comment 6466053903) found the engine right and three statements of the note false. This PR corrects those statements and changes nothing else in the note: the `patch` level, the title, the `Clause-②: yes (widening)` and `Metadata-protocol: none` declarations, and the Before, Now (one qualifier added) and Who-sees-it bullets stay. 1. **By-id probe filters.** The note listed "the filter of every probe a by-id delete sends" as unchanged. That is gone from the Unchanged bullet. A new bullet, "A by-id delete's probes, too", states what changed. `collectCascadeDeleteSet` probes the records found at one depth together, per object and relation, and `refuseRestrictsInsideOwnCascade` probes together the members asking one relation. With two or more records this sends `$in`, one probe per 500 records, or on a multiple lookup the OR of each id's `$contains` (`referenceProbeFilterOfSet`). The root's own probe and each cascaded child's own walk still send that one record's filter: a batch of one is `referenceProbeFilter`, and a child is deleted through its own by-id `delete()`, which joins the set and walks one id. 2. **Two outputs the note left out.** Each has its own bullet now: - A matched row that another matched row's cascade reaches is now removed by the predicate delete's own id-set `deleteMany`. The set walk enters every matched id before the first nested `delete()`, so the cascade skips it. The count the predicate delete returns now includes that row, and its `beforeDelete`/`afterDelete` hooks fire once, from the per-row dispatch. Before, the cascade's nested `delete()` removed the row ahead of `deleteMany`, so the count left it out, and its hooks fired a second time from that nested delete. The new bullet also notes that a flow's `delete_record` hands this count on as its `result` (`crud-nodes.ts`). - Which refusal is reported. The set walk asks each relation about every matched row before it moves on to the next relation. It also runs the own-cascade `restrict` check for every matched root before the walk starts. So when two relations refuse different matched rows, it reports the first relation's refusal. Before, it reported the first matched row's refusal. 3. **The example.** The `Clause-②:` line said the row-by-row walk refused a report and its manager (a `restrict` self-lookup, both matched) "or not, depending on which row it met first". It refused in either order, and the line now says so. The `Clause-②: yes (widening)` declaration at the start of the line is unchanged. The Now bullet's "`cascade` deletes (each child through its own delete, hooks and all)" now says "each child the predicate did not match". Without that qualifier it would contradict item 2. ## Tests - New in `packages/objectql/src/engine-cascade-delete-atomic.test.ts`: *counts a matched row another matched row cascades into, and fires its delete hooks once (#27004)*. An object holds a `cascade` lookup to itself, and parent and child are both matched. The test asserts the delete returns `2`, only the unmatched row is left, and each matched row gets exactly one `beforeDelete` and one `afterDelete`. This is the optional pin the card offered. - Before writing a sentence, I measured each before/after claim in the new text once, against the old per-row predicate arm. The arm's single `cascadeDeleteRelations(object, matchedIds, …)` was swapped for the loop it replaced, one root per matched id, through `scripts/ablation-replace.mjs`. The mutation landed (blob `3dcaaed4af74` → `b887e2f85237`) and was restored to HEAD (`git diff HEAD` empty). Temporary probe cases were appended to the test file and removed afterwards; the file's blob equals HEAD. | Reading (in-memory test driver) | Set walk (this tree) | Old per-row walk (mutation) | |---|---|---| | Cascaded matched row: count returned | `2` | `1` | | Cascaded matched row: its hooks | before ×1, after ×1 | before ×2, after ×2 | | Report + manager, `restrict` self-lookup, manager matched first | deleted (`2`) | `DELETE_RESTRICTED` | | Same, report matched first | deleted (`2`) | `DELETE_RESTRICTED` | | `aa_guard` restricts p2, `zz_guard` restricts p1 | refuses p2 via `aa_guard` | refuses p1 via `zz_guard` | The new pin test fails on the mutation (`expected 1 to be 2`) and passes on this tree. ## Acceptance notes - **`Check Changeset` stays red, by design.** This PR edits a pending changeset and adds none, so the job reds twice: its count of added changesets is 0, and `check-empty-changeset.mjs` refuses a change to a changeset that exists on the merge base. This is that gate's DELIBERATE CORRECTION class. The route it prescribes: do not restore the note, do not apply `skip-changeset`, name the note and what changed under it on the PR, and get the correction confirmed there in writing. The note is `.changeset/27004-predicate-delete-walk-as-set.md`. What changed under it is the engine change #27073 already landed: this PR changes no code, and the note was wrong about that change from the start. The order for this edit is the card's triage ruling (6467217349), verbatim: "⛔ Edit the pending changeset text only. The engine code was judged right." The written confirmation on this PR is still owed. `Check Changeset` is not one of the ten required contexts. - The note must land before #26517 is merged; that is the card's deadline. - No engine code changes. `packages/objectql/src/engine.ts` is byte-identical to `origin/main`. ## Verification All readings are from the worktree of this branch. Head is `cbc268d7` unless a line says otherwise. <details><summary>Derived gate families: 59 derived, 59 run, 58 exit 0, 1 exit 1 (by design)</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 59 families at `cbc268d7` from 2 changed paths (+38/-3). - I ran each one and recorded its exit code. The `--ran` reconciliation reads: `✓ dispatch-gates --ran: 59 derived famil(ies) accounted for — 59 run, 0 NOT-MEASURED`. - The one non-zero exit is `node scripts/check-empty-changeset.mjs --base origin/main` (exit 1). It names `.changeset/27004-predicate-delete-walk-as-set.md` as existing on the merge base and offers the DELIBERATE CORRECTION route. See Acceptance notes. - The first run, at `00ca77b4`, had three exit-3 readings: `check-plugin-teardown-shape --self-test` (a shallow clone; I fetched its pinned control commit), `check:lean-entry-closure` (needed objectql `dist`; I built it) and `check:dual-build-cjs-loads` (needed built packages). All three exit 0 in the `cbc268d7` run. </details> <details><summary>@objectstack/objectql package runs</summary> - `pnpm --filter '@objectstack/objectql^...' build`: VERDICT command-exit 0. - `pnpm --filter @objectstack/objectql typecheck`: VERDICT command-exit 0. Output: `check:test-typecheck: OK … 39 file(s) / 233 error(s) / 64 pinned signature(s) held`. - `pnpm --filter @objectstack/objectql test` at `00ca77b4`: `Test Files 477 passed (477)`, `Tests 8900 passed (8900)`, VERDICT command-exit 0. The only commit after it (`cbc268d7`) changes two lines of the changeset, so `packages/objectql` is byte-identical. - `vitest run src/engine-cascade-delete-atomic.test.ts`: 20 passed. Under the old-walk mutation, the new case fails with `expected 1 to be 2`. </details> <details><summary>Not run locally</summary> - `pnpm lint` over the whole repo: CI runs it. I ran `pnpm exec eslint --no-inline-config packages/objectql/src/engine-cascade-delete-atomic.test.ts` on the one code file this PR touches: exit 0, no findings. - `test:repo`: CI runs it, and the diff touches no file of that project. - Full dogfood: not owed. The title carries no `!`, and the `Clause-②:` line carries no `(narrowing)` arm. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CXc5hequf76uJfzrdK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fa5e782 commit 4c9d0d3

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

‎.changeset/27004-predicate-delete-walk-as-set.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44

55
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)
66

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.
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, whichever of the two rows it met first. Nothing is narrowed.
88
Metadata-protocol: none
99

1010
- **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.
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 the predicate did not match 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+
- **A by-id delete's probes, too.** Its own record is still probed with that record's filter, and each cascaded child's delete still probes its one record. The reads made before the first write changed: collecting the rows the cascade removes now probes the records found at one depth together (per object and relation, from the root's children down), and the check for an authored `restrict` that the delete's own cascade would override probes together the records that ask one relation. Two or more records are asked together with `$in` (on a multiple lookup, the OR of each id's `$contains`) where each used to get its own probe; a lone record keeps its own filter. Deleting a parent with two children on one cascading relation therefore probes each of the children's own cascading relations once with `$in`, where it used to send one probe per child. A driver answering these probes now receives `$in`; a batch with a single-valued row that names none of its ids by `String()` is asked again one id at a time.
13+
- **A matched row another matched row's cascade reaches** (an object's `cascade` reference to itself, with parent and child both matched). The cascade no longer deletes it through a nested delete of its own; the predicate delete removes it with the other matched rows. The count `engine.delete` returns, which a flow's `delete_record` hands on as its `result`, now includes that row, where it used to leave it out, and the row's `beforeDelete` and `afterDelete` hooks fire once, as a matched row, where they used to fire a second time from the cascade's delete.
14+
- **Which refusal is reported when more than one applies.** The row-by-row walk finished one matched row's relations before it started the next row, so it reported the first matched row that refused. The set walk asks each relation about every matched row before it moves to the next relation, so it reports the first relation that refuses, naming the first matched row that relation refuses for; and it checks every matched row for an authored `restrict` its own cascade would override before it walks the first relation.
15+
- **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, and the roll-up recompute after the delete.
1316
- **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-cascade-delete-atomic.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,38 @@ describe('[#26871] a predicate delete runs the referential walk for every matche
572572
expect(driver.writes).toEqual([]);
573573
expect(driver.rowsOf('detail')).toHaveLength(1);
574574
});
575+
576+
it('counts a matched row another matched row cascades into, and fires its delete hooks once (#27004)', async () => {
577+
// One walk over the matched set: the child is a member like its parent,
578+
// so the cascade leaves it to the predicate's own id-set delete instead of
579+
// deleting it through a nested delete of its own.
580+
const crew = {
581+
name: 'crew',
582+
label: 'Crew',
583+
fields: {
584+
id: { name: 'id', type: 'text' as const, primaryKey: true },
585+
name: { name: 'name', type: 'text' as const },
586+
lead: { name: 'lead', type: 'lookup' as const, reference: 'crew', deleteBehavior: 'cascade' },
587+
},
588+
};
589+
const { engine, driver } = await makeEngine([crew]);
590+
const lead = await engine.insert('crew', { name: 'doomed', lead: null });
591+
const member = await engine.insert('crew', { name: 'doomed', lead: lead.id });
592+
const kept = await engine.insert('crew', { name: 'kept', lead: null });
593+
const events: string[] = [];
594+
for (const event of ['beforeDelete', 'afterDelete'] as const) {
595+
engine.registerHook(event, async (ctx: any) => void events.push(`${event}:${ctx.input?.id}`), { object: 'crew' });
596+
}
597+
598+
const removed = await engine.delete('crew', { where: { name: 'doomed' }, multi: true });
599+
600+
expect(removed).toBe(2);
601+
expect(driver.rowsOf('crew').map((r: any) => r.id)).toEqual([kept.id]);
602+
expect([...events].sort()).toEqual([
603+
`afterDelete:${lead.id}`, `afterDelete:${member.id}`,
604+
`beforeDelete:${lead.id}`, `beforeDelete:${member.id}`,
605+
].sort());
606+
});
575607
});
576608

577609
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)