Skip to content

Commit 0eb2d21

Browse files
fix(objectql): a predicate update or delete recomputes the parent roll-up summaries its matched rows name (#26950) (#27000)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XU1LK3XtFTgExsmvSjfSEw7)_ Fixes #26950 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:按条件批量更新/删除(`multi: true` + filter,流程的 `update_record` / `delete_record` 批量节点走的就是它)现在会重算父记录上的汇总字段(count/sum/min/max/avg):删除重算被删子记录原来挂的父记录,更新同时重算旧父和新父。 - **为什么改**:此前批量写只返回影响行数,`recomputeSummaries` 拿不到任何父 id,父记录汇总一直停在旧值,直到某次按 id 写子记录才被纠正。流程批量删除或改挂订单行,订单合计静默错误。 - **风险与回滚**:没有汇总字段指向的对象完全不变,仍是一次批量驱动调用、不多读一行。有汇总时在写之前多读一次匹配行的外键列(已有读取时复用,不重复读)。重算失败与按 id 写一样抛 `SummaryRecomputeError`(记录已写入)。回滚即 revert 本 PR。 - **先看**:`packages/objectql/src/engine.ts` 的 `update()` 谓词分支(`[#26950]` 段,`updateMany` 之前)与 `delete()` 谓词分支(`plan === 'none'` 与走查分支);`packages/objectql/src/summary-aggregate.ts` 的 `withoutDeletedSelfParents`。 - **两分钟验证**:`pnpm --filter @objectstack/service-automation exec vitest run src/builtin/predicate-write-summary-recompute.integration.test.ts`,预期 6 条全绿(真 SQLite 驱动,含两条流程节点用例)。 - **需要你做的**:无,席位落地。 ## What changes The predicate arms of `ObjectQL.update()` and `ObjectQL.delete()` (`multi: true`, no scalar `where.id`) now hand the existing `recomputeSummaries` the matched rows' foreign keys. Seat ruling: no second recompute routine, no new option on `engine.update` / `engine.delete`, no refusal of `multi: true`. - **Which parents.** A delete hands the matched rows as `previous` (the old parents). An update hands them as `previous` and, merged with the payload the driver receives, as `records` (the new parents), so a reparent recomputes both, as the by-id update does with `priorRecord`. - **Cheap when there is nothing to recompute.** A new private `summaryForeignKeyColumns(object)` names the FK columns of the roll-up descriptors over the object (`getSummaryDescriptors`). Empty means nothing is read and the arm keeps its single `driver.updateMany` / `driver.deleteMany`. - **One read, before the write, through the write's own composed AST and driver options.** Only the FK columns are read (`fields: [fk…]`). Where the write already reads its matched rows, that read serves and no second one is made: on an update, the matched-row memo (`readPriorRows`) when a consumer above made it (per-row hooks, per-row validation, the post-hook image check); on a delete, the referential walk's own id read (#26930), which now reads `['id', …fk]`, or, with no walk, the per-row hook read. - **Failures and atomicity are the by-id path's.** The recompute runs where the by-id arm runs it (after the `after*` dispatch, with the write's context and its transaction handle when there is one), and a failure surfaces as the same `SummaryRecomputeError`. No new error code. - **A self roll-up's deleted parent is not asked about.** A roll-up over its own object can name, from one matched row, a parent that the same predicate delete removes. Measured during this work: handing that row unfiltered made the delete throw `SummaryRecomputeError` (the parent update answers not-found) after the rows were deleted. `withoutDeletedSelfParents` (in `summary-aggregate.ts`) clears, on the walk arm's rows, only the self-referencing descriptors' FK values that name a row this delete removed. The by-id delete never meets this case (its children's own deletes recompute the parent while it still exists). ## Tests - `packages/services/service-automation/src/builtin/predicate-write-summary-recompute.integration.test.ts` (new; real `ObjectKernel` + `ObjectQLPlugin` + `SqlDriver` better-sqlite3 + `AutomationServicePlugin`, the stack the #26871 flow test boots). An order with a `count` and a `sum` roll-up over `master_detail` lines, two orders per case, roll-ups read out of the driver. Six cases: a predicate delete of both of one order's lines brings it to 0/0 and leaves the other order alone; a predicate update moving both lines to the other order recomputes both (old 0/0, new 3/47); a predicate update of the summed field recomputes the parent it stays under; a predicate delete of a second child that a `set_null` lookup points at (so it takes the referential walk's arm) recomputes the parent and still clears the lookup; a flow `delete_record` with `multi: true`; a flow `update_record` with `multi: true` that reparents. - `packages/objectql/src/summary-rollup.test.ts`, new section `[#26950]` (6 cases, this file's own fake driver, which learned `updateMany` / `deleteMany`, an ordered read/write log and copying `find` answers). The control: an object no roll-up aggregates makes exactly one `updateMany` and one `deleteMany` and no row read. With a roll-up: a predicate update and a predicate delete each read the matched rows ONCE before the write, `fields: ['inv']`, under the caller's `where`, and land the right totals; with per-row `afterUpdate` / `afterDelete` hooks registered the hooks' read is the only one; with a lookup pointing at the child the walk's id read is the only one and carries `['id', 'inv']`; a self roll-up predicate delete that removes a parent and its child answers its count and recomputes only the surviving root. ## Acceptance notes - **A predicate write can now end in `SummaryRecomputeError`.** It is the by-id write's existing answer when a parent recompute fails after retries (the rows were written, the roll-up may be stale). Before, a predicate write never asked, so it never failed this way; it left the roll-up stale instead. No write is refused, and no accepted value or shape changes: `Clause-②: no`. - **Cost.** An object with a roll-up over it pays one FK-only read per predicate write (none when the write already reads its matched rows) and one aggregate plus one parent update per distinct parent per roll-up, as N by-id writes would, deduplicated per parent. No ceiling is added (none was asked). An object no roll-up aggregates pays nothing new. - **The self roll-up filter runs on the walk arm only.** A roll-up over its own object reaches its parent through a reference to that object, which makes `planCascadeAtomicity` answer other than `'none'`, so its predicate delete always takes the walk arm where the filter sits. - **No body the #25522 split series moved is edited.** `update()`, `delete()`, `recomputeSummaries` and `getSummaryDescriptors` all still live in `engine.ts`. At start and before opening this PR, no open PR touched `packages/objectql/src/engine.ts` (#26831 touches `engine-execution-plumbing.ts` only). ## Verification All local readings below are on head `e3a38f12` (the merge of `main` at `49b50591` into the branch) unless a line names another, worktree `objectstack-issue-26950`, shared container. <details><summary>Red first, on base 40264be (before the fix)</summary> - The flow and engine file, objectql dist built from the base: `Tests 6 failed (6)`. Each parent kept its pre-write roll-up, e.g. `expected { count: 2, total: 42 } to deeply equal { count: +0, total: +0 }` for the predicate delete, and `the old parent: expected { count: 2, total: 42 } …` for the reparent. - `src/summary-rollup.test.ts` with `engine.ts` restored to the base blob `fb30f0f7` (tree only): `Tests 4 failed | 18 passed (22)`. The four read-discipline cases red; the no-roll-up control stays green, as it must. Restored: blob `31029717` == HEAD, `git diff HEAD` empty, objectql rebuilt. </details> <details><summary>Green, and the ablations</summary> - objectql full suite `pnpm --filter @objectstack/objectql test` on `f6f89395`: Test Files 475 passed (475), Tests 8883 passed (8883). service-automation full suite on `f6f89395`: Test Files 215 passed (215), Tests 2752 passed (2752). - `pnpm --filter @objectstack/objectql typecheck` and `pnpm --filter @objectstack/service-automation typecheck`: exit 0 (on `f6f89395`; objectql re-run exit 0 after the last source commit, a type-only change of two casts). - On `e3a38f12`, after a full build: `summary-rollup`, `engine-cascade-delete-atomic`, `engine-nested-relation-lowering` 55 passed (3 files); the new flow file plus #26871's `delete-record-predicate-referential-walk` 8 passed (2 files). - `eslint --no-inline-config packages/objectql packages/services/service-automation`: exit 0, no output. - Ablations, each through `scripts/ablation-replace.mjs` (anchor 1 to 0, blob moved, restored to HEAD blob, `git diff HEAD` empty), objectql rebuilt and `ablation-dist-preflight` finding the marker in dist where the flow file was read: 1. The update hands no new FKs (`next: previous`): flow file 2 failed (both reparent cases), objectql 2 failed. 2. The walk arm keeps no FK rows: flow file 1 failed (the walk case), objectql 1 failed. 3. No reuse of the per-row hook read: objectql 1 failed (the reuse case); the flow file stays green, as it should, since an extra read changes no total. 4. The self roll-up filter removed: objectql 1 failed, the delete throwing `SummaryRecomputeError`. The restore leg rebuilt objectql and `ablation-dist-preflight --absent` was clean for every marker. </details> <details><summary>Gates (derived by dispatch-gates --commands on the real diff)</summary> - On `e3a38f12`, after a full `turbo run build`: 65 families derived, each run with its exit code captured before any pipe: 65 exit 0. `dispatch-gates --ran`: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN. - The first sweep (on `f6f89395`) caught two things: `check:query-options-erasure` counted the two new driver reads' `as any` option casts (8 to 10 in `engine.ts`), now typed as the third parameter of `IDataDriver['find']`, as the by-id precondition read is; `check-plugin-teardown-shape --self-test` needed its pinned fixture commit fetched. - Everything else (the `check:*` farm, `Test Core`, `Test UI`, the rest of the required nine) is CI's: see this PR's check-runs. </details> <details><summary>Dogfood battery</summary> - Not owed: no `!` in the title and no `(narrowing)` arm on `Clause-②`. </details> Session: `session_01FEkQ5mKGG79zhwLb1obtFs` --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5mKGG79zhwLb1obtFs)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5ad6f22 commit 0eb2d21

5 files changed

Lines changed: 475 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): a predicate update or delete (`multi: true` with a filter) recomputes the parent roll-up summaries its matched rows name (#26950)
6+
7+
Clause-②: no — no write is refused and no key, value or shape changes: a stored roll-up that a predicate write used to leave stale is now recomputed, as the same write done record by record recomputes it.
8+
Metadata-protocol: none
9+
10+
- **Before.** `engine.update(object, data, { where, multi: true })` and `engine.delete(object, { where, multi: true })`, which a flow's `update_record` / `delete_record` node with `multi: true` and a filter reaches, recomputed no `summary` roll-up. The parent's count, sum, min, max or average kept its old value until some by-id write on one of its children recomputed it. A bulk step that deleted or reassigned line items left the order totals wrong, with no error.
11+
- **Now.** When a roll-up aggregates the object, the predicate write reads the foreign-key columns those roll-ups name off the rows it matches, before it writes, through the same row-scoped query, and recomputes every parent they name after the write: for a delete the old parents, for an update the old and the new ones, so a reparent recomputes both. A recompute failure surfaces as the by-id write's does (`SummaryRecomputeError`: the rows were written, the roll-up may be stale).
12+
- **Unchanged.** An object no roll-up aggregates reads no extra row: its predicate write is still one bulk driver call. Where the write already reads its matched rows (per-row hooks, per-row validation, the referential walk of a predicate delete), that read carries the foreign keys and no second read is made.

‎packages/objectql/src/engine.ts‎

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ import { TransactionUnsupportedError } from './transaction-errors.js';
199199
import {
200200
aggregateSummaryValue,
201201
summaryEmptySetValue,
202+
withoutDeletedSelfParents,
202203
type SummaryDescriptor,
203204
} from './summary-aggregate.js';
204205
import { ReadonlyFieldRejectedError } from './readonly-strict-errors.js';
@@ -12402,6 +12403,14 @@ export class ObjectQL implements IObjectQLEngine {
1240212403
return this.summaryIndex!.get(childObject) ?? [];
1240312404
}
1240412405

12406+
/** [#26950] The child-side foreign-key columns the roll-ups over
12407+
* `childObject` read: what a predicate write must read off its matched rows,
12408+
* before it writes, to hand {@link recomputeSummaries} the parents to
12409+
* recompute. Empty when no roll-up aggregates `childObject`. */
12410+
private summaryForeignKeyColumns(childObject: string): string[] {
12411+
return [...new Set(this.getSummaryDescriptors(childObject).map((desc) => desc.fkField))];
12412+
}
12413+
1240512414
/** Roll-up descriptors for the summary fields `parentObject` OWNS (#5749) —
1240612415
* i.e. the ones a NEW row of `parentObject` must have seeded.
1240712416
*
@@ -16735,6 +16744,11 @@ export class ObjectQL implements IObjectQLEngine {
1673516744
// driver returning a row from `updateMany` would silently reroute a
1673616745
// bulk write onto the per-record contract if we inferred it.
1673716746
let isPredicateWrite = false;
16747+
// [#26950] The roll-up input a predicate write hands
16748+
// `recomputeSummaries`: each matched row as it was (`previous`) and
16749+
// as the write leaves it (`next`). `null` when no roll-up aggregates
16750+
// this object, and on the by-id branch, which passes its own row.
16751+
let summaryRows: { previous: Record<string, unknown>[]; next: Record<string, unknown>[] } | null = null;
1673816752
const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(updateSchema);
1673916753
const valueShapeStrict = await this.valueShapeStrictFor(updateSchema);
1674016754
const updateMsgCtx = this.validationMessageContext(object, opCtx.context);
@@ -17504,6 +17518,23 @@ export class ObjectQL implements IObjectQLEngine {
1750417518
// the N matched rows conflicted — a fabricated row attribution
1750517519
// is worse than an absent one (triage ruling, 2026-09-02),
1750617520
// exactly as the composite-index case already behaves on insert.
17521+
//
17522+
// [#26950] The parents this write moves a roll-up of, read BEFORE
17523+
// the write: the matched rows' foreign keys as they are, and as
17524+
// the payload leaves them, so a reparent recomputes the old parent
17525+
// and the new one, as the by-id branch does with `priorRecord`.
17526+
// Only the descriptors' FK columns are read, through this write's
17527+
// own row-scoped AST and driver options; the memoized matched-row
17528+
// read serves instead when a consumer above already made it. An
17529+
// object no roll-up aggregates reads nothing here.
17530+
const summaryKeyColumns = this.summaryForeignKeyColumns(object);
17531+
if (summaryKeyColumns.length > 0) {
17532+
const previous = priorRowsRead
17533+
? ((await readPriorRows()) ?? [])
17534+
: (((await driver.find(object, { ...ast, fields: summaryKeyColumns }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]);
17535+
const payload = hookContext.input.data as Record<string, unknown>;
17536+
summaryRows = { previous, next: previous.map((row) => ({ ...row, ...payload })) };
17537+
}
1750717538
try {
1750817539
result = await driver.updateMany!(object, ast, hookContext.input.data as Record<string, unknown>, hookContext.input.options as any);
1750917540
} catch (driverError) {
@@ -17591,7 +17622,10 @@ export class ObjectQL implements IObjectQLEngine {
1759117622

1759217623
// Roll-up: recompute parent summaries; pass priorRecord too so a child
1759317624
// that moved to a different parent updates BOTH old and new parent.
17594-
const summaryFailures = await this.recomputeSummaries(object, result, priorRecord, opCtx.context);
17625+
// [#26950] A predicate write passes its matched rows, old and new.
17626+
const summaryFailures = summaryRows
17627+
? await this.recomputeSummaries(object, summaryRows.next, summaryRows.previous, opCtx.context)
17628+
: await this.recomputeSummaries(object, result, priorRecord, opCtx.context);
1759517629

1759617630
// Publish the update event under whichever contract this write can
1759717631
// honour: per-record `data.record.updated` (#4626), or the aggregate
@@ -19607,6 +19641,12 @@ export class ObjectQL implements IObjectQLEngine {
1960719641
// `record-after-delete` flow must see each deleted row rather than one
1960819642
// context that names none of them.
1960919643
let bulkPerRowRows: Record<string, unknown>[] | null = null;
19644+
// [#26950] The predicate path's matched rows as the per-row hook read
19645+
// below took them (`null` when no delete-side hook asked for them), and
19646+
// the rows whose foreign keys `recomputeSummaries` is handed after the
19647+
// delete (`null` when no roll-up aggregates this object).
19648+
let predicateMatchedRows: Record<string, unknown>[] | null = null;
19649+
let summaryPriorRows: Record<string, unknown>[] | null = null;
1961019650

1961119651
// [#5574] The dispatch ladder, resolved BEFORE the before phase — see
1961219652
// update()'s twin for the full reasoning. Keyed on the SAME falsy-`id`
@@ -19746,6 +19786,7 @@ export class ObjectQL implements IObjectQLEngine {
1974619786
(await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [],
1974719787
declaredDeleteColumns,
1974819788
);
19789+
predicateMatchedRows = doomed;
1974919790
// [D6] One ceiling, both phases, BEFORE the first per-row dispatch
1975019791
// and before the driver call.
1975119792
this.assertBulkPerRowHookBudget(
@@ -19841,8 +19882,23 @@ export class ObjectQL implements IObjectQLEngine {
1984119882
// `'none'` means no relation of the delete family points at
1984219883
// this object, so the walk could write nothing and the arm keeps
1984319884
// its single `deleteMany`, no rows read and no transaction.
19885+
//
19886+
// [#26950] A roll-up over this object needs the matched rows'
19887+
// foreign keys, read BEFORE the delete, to find the parents to
19888+
// recompute after it: the by-id branch hands `recomputeSummaries`
19889+
// its pre-image, and this arm hands it the matched rows. Only the
19890+
// descriptors' FK columns are read, through the composed AST and
19891+
// the write's driver options, and only once: the walk's own id
19892+
// read carries them below, and with no walk the per-row hook
19893+
// read serves when it was made. An object no roll-up aggregates
19894+
// reads nothing new.
19895+
const summaryKeyColumns = this.summaryForeignKeyColumns(object);
1984419896
const plan = this.planCascadeAtomicity(object);
1984519897
if (plan === 'none') {
19898+
if (summaryKeyColumns.length > 0) {
19899+
summaryPriorRows = predicateMatchedRows
19900+
?? (((await driver.find(object, { ...opCtx.ast!, fields: summaryKeyColumns }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]);
19901+
}
1984619902
result = await driver.deleteMany!(object, opCtx.ast!, hookContext.input.options as any);
1984719903
} else {
1984819904
// Otherwise: resolve the matched rows through the composed,
@@ -19857,8 +19913,9 @@ export class ObjectQL implements IObjectQLEngine {
1985719913
// branch's, for the same reasons (#7413).
1985819914
const runPredicateDelete = async (writeContext?: ExecutionContext): Promise<unknown> => {
1985919915
const writeOptions = this.buildDriverOptions(object, writeContext, hookContext.input.options as any);
19860-
const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id'] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
19916+
const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id', ...summaryKeyColumns] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
1986119917
const matchedIds = matched.map((row) => row?.id).filter((v): v is string | number => v != null);
19918+
if (summaryKeyColumns.length > 0) summaryPriorRows = withoutDeletedSelfParents(object, matched, matchedIds, this.getSummaryDescriptors(object));
1986219919
for (const matchedId of matchedIds) {
1986319920
await this.cascadeDeleteRelations(object, matchedId, writeContext);
1986419921
}
@@ -19926,9 +19983,11 @@ export class ObjectQL implements IObjectQLEngine {
1992619983
// from the row's FK values captured BEFORE deletion. [#5272] That
1992719984
// capture is now the same single pre-image read `previous` rides on
1992819985
// (it used to be its own later `findOne`), which is also what
19929-
// `update()` passes here.
19930-
const summaryFailures = priorRecord
19931-
? await this.recomputeSummaries(object, null, priorRecord, opCtx.context)
19986+
// `update()` passes here. [#26950] A predicate delete passes the
19987+
// matched rows it read before the delete.
19988+
const summaryPrevious = priorRecord ?? summaryPriorRows;
19989+
const summaryFailures = summaryPrevious
19990+
? await this.recomputeSummaries(object, null, summaryPrevious, opCtx.context)
1993219991
: [];
1993319992

1993419993
// Same split as update(): per-record `data.record.deleted` (#4626),

‎packages/objectql/src/summary-aggregate.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,38 @@ export async function aggregateSummaryValue(
120120
const value = rows?.[0]?.value;
121121
return value == null ? summaryEmptySetValue(desc.fn) : value;
122122
}
123+
124+
/**
125+
* [#26950] The matched rows a predicate delete hands the recompute, less every
126+
* foreign key that names a row the same delete removed.
127+
*
128+
* A roll-up over its own object (a `summary` aggregating `object` through a
129+
* self-reference) can name, from one matched row, a parent that is itself a
130+
* matched row. That parent is gone after the delete, so it has no roll-up left
131+
* to recompute, and asking would answer not-found for a row the delete itself
132+
* took: a `SummaryRecomputeError` on a delete that succeeded. The by-id delete
133+
* never meets this, because it deletes one row and its children's own deletes
134+
* recompute it while it still exists. Only self-referencing descriptors are
135+
* touched; every other foreign key is handed over as read.
136+
*/
137+
export function withoutDeletedSelfParents(
138+
object: string,
139+
rows: Array<Record<string, unknown>>,
140+
deletedIds: ReadonlyArray<string | number>,
141+
descriptors: readonly SummaryDescriptor[],
142+
): Array<Record<string, unknown>> {
143+
const selfKeys = descriptors.filter((desc) => desc.parentObject === object).map((desc) => desc.fkField);
144+
if (selfKeys.length === 0 || deletedIds.length === 0) return rows;
145+
const gone = new Set(deletedIds.map(String));
146+
return rows.map((row) => {
147+
let out = row;
148+
for (const key of selfKeys) {
149+
const v = row?.[key];
150+
if (v != null && gone.has(String(v))) {
151+
if (out === row) out = { ...row };
152+
out[key] = null;
153+
}
154+
}
155+
return out;
156+
});
157+
}

0 commit comments

Comments
 (0)