fix(objectql): a predicate update or delete recomputes the parent roll-up summaries its matched rows name (#26950)

.changeset/26950-predicate-write-summary-recompute.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql): a predicate update or delete (`multi: true` with a filter) recomputes the parent roll-up summaries its matched rows name (#26950)
+
+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.
+Metadata-protocol: none
+
+- **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.
+- **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).
+- **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

@@ -199,6 +199,7 @@ import { TransactionUnsupportedError } from './transaction-errors.js';
 import {
   aggregateSummaryValue,
   summaryEmptySetValue,
+  withoutDeletedSelfParents,
   type SummaryDescriptor,
 } from './summary-aggregate.js';
 import { ReadonlyFieldRejectedError } from './readonly-strict-errors.js';
@@ -12402,6 +12403,14 @@ export class ObjectQL implements IObjectQLEngine {
     return this.summaryIndex!.get(childObject) ?? [];
   }
 
+  /** [#26950] The child-side foreign-key columns the roll-ups over
+   *  `childObject` read: what a predicate write must read off its matched rows,
+   *  before it writes, to hand {@link recomputeSummaries} the parents to
+   *  recompute. Empty when no roll-up aggregates `childObject`. */
+  private summaryForeignKeyColumns(childObject: string): string[] {
+    return [...new Set(this.getSummaryDescriptors(childObject).map((desc) => desc.fkField))];
+  }
+
   /** Roll-up descriptors for the summary fields `parentObject` OWNS (#5749) —
    *  i.e. the ones a NEW row of `parentObject` must have seeded.
    *
@@ -16735,6 +16744,11 @@ export class ObjectQL implements IObjectQLEngine {
            // driver returning a row from `updateMany` would silently reroute a
            // bulk write onto the per-record contract if we inferred it.
            let isPredicateWrite = false;
+           // [#26950] The roll-up input a predicate write hands
+           // `recomputeSummaries`: each matched row as it was (`previous`) and
+           // as the write leaves it (`next`). `null` when no roll-up aggregates
+           // this object, and on the by-id branch, which passes its own row.
+           let summaryRows: { previous: Record<string, unknown>[]; next: Record<string, unknown>[] } | null = null;
            const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(updateSchema);
            const valueShapeStrict = await this.valueShapeStrictFor(updateSchema);
            const updateMsgCtx = this.validationMessageContext(object, opCtx.context);
@@ -17504,6 +17518,23 @@ export class ObjectQL implements IObjectQLEngine {
                // the N matched rows conflicted — a fabricated row attribution
                // is worse than an absent one (triage ruling, 2026-09-02),
                // exactly as the composite-index case already behaves on insert.
+               //
+               // [#26950] The parents this write moves a roll-up of, read BEFORE
+               // the write: the matched rows' foreign keys as they are, and as
+               // the payload leaves them, so a reparent recomputes the old parent
+               // and the new one, as the by-id branch does with `priorRecord`.
+               // Only the descriptors' FK columns are read, through this write's
+               // own row-scoped AST and driver options; the memoized matched-row
+               // read serves instead when a consumer above already made it. An
+               // object no roll-up aggregates reads nothing here.
+               const summaryKeyColumns = this.summaryForeignKeyColumns(object);
+               if (summaryKeyColumns.length > 0) {
+                   const previous = priorRowsRead
+                     ? ((await readPriorRows()) ?? [])
+                     : (((await driver.find(object, { ...ast, fields: summaryKeyColumns }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]);
+                   const payload = hookContext.input.data as Record<string, unknown>;
+                   summaryRows = { previous, next: previous.map((row) => ({ ...row, ...payload })) };
+               }
                try {
                    result = await driver.updateMany!(object, ast, hookContext.input.data as Record<string, unknown>, hookContext.input.options as any);
                } catch (driverError) {
@@ -17591,7 +17622,10 @@ export class ObjectQL implements IObjectQLEngine {
 
            // Roll-up: recompute parent summaries; pass priorRecord too so a child
            // that moved to a different parent updates BOTH old and new parent.
-           const summaryFailures = await this.recomputeSummaries(object, result, priorRecord, opCtx.context);
+           // [#26950] A predicate write passes its matched rows, old and new.
+           const summaryFailures = summaryRows
+             ? await this.recomputeSummaries(object, summaryRows.next, summaryRows.previous, opCtx.context)
+             : await this.recomputeSummaries(object, result, priorRecord, opCtx.context);
 
            // Publish the update event under whichever contract this write can
            // honour: per-record `data.record.updated` (#4626), or the aggregate
@@ -19607,6 +19641,12 @@ export class ObjectQL implements IObjectQLEngine {
       // `record-after-delete` flow must see each deleted row rather than one
       // context that names none of them.
       let bulkPerRowRows: Record<string, unknown>[] | null = null;
+      // [#26950] The predicate path's matched rows as the per-row hook read
+      // below took them (`null` when no delete-side hook asked for them), and
+      // the rows whose foreign keys `recomputeSummaries` is handed after the
+      // delete (`null` when no roll-up aggregates this object).
+      let predicateMatchedRows: Record<string, unknown>[] | null = null;
+      let summaryPriorRows: Record<string, unknown>[] | null = null;
 
       // [#5574] The dispatch ladder, resolved BEFORE the before phase — see
       // update()'s twin for the full reasoning. Keyed on the SAME falsy-`id`
@@ -19746,6 +19786,7 @@ export class ObjectQL implements IObjectQLEngine {
             (await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [],
             declaredDeleteColumns,
           );
+          predicateMatchedRows = doomed;
           // [D6] One ceiling, both phases, BEFORE the first per-row dispatch
           // and before the driver call.
           this.assertBulkPerRowHookBudget(
@@ -19841,8 +19882,23 @@ export class ObjectQL implements IObjectQLEngine {
                // `'none'` means no relation of the delete family points at
                // this object, so the walk could write nothing and the arm keeps
                // its single `deleteMany`, no rows read and no transaction.
+               //
+               // [#26950] A roll-up over this object needs the matched rows'
+               // foreign keys, read BEFORE the delete, to find the parents to
+               // recompute after it: the by-id branch hands `recomputeSummaries`
+               // its pre-image, and this arm hands it the matched rows. Only the
+               // descriptors' FK columns are read, through the composed AST and
+               // the write's driver options, and only once: the walk's own id
+               // read carries them below, and with no walk the per-row hook
+               // read serves when it was made. An object no roll-up aggregates
+               // reads nothing new.
+               const summaryKeyColumns = this.summaryForeignKeyColumns(object);
                const plan = this.planCascadeAtomicity(object);
                if (plan === 'none') {
+                 if (summaryKeyColumns.length > 0) {
+                   summaryPriorRows = predicateMatchedRows
+                     ?? (((await driver.find(object, { ...opCtx.ast!, fields: summaryKeyColumns }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]);
+                 }
                  result = await driver.deleteMany!(object, opCtx.ast!, hookContext.input.options as any);
                } else {
                  // Otherwise: resolve the matched rows through the composed,
@@ -19857,8 +19913,9 @@ export class ObjectQL implements IObjectQLEngine {
                  // branch's, for the same reasons (#7413).
                  const runPredicateDelete = async (writeContext?: ExecutionContext): Promise<unknown> => {
                    const writeOptions = this.buildDriverOptions(object, writeContext, hookContext.input.options as any);
-                   const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id'] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
+                   const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id', ...summaryKeyColumns] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
                    const matchedIds = matched.map((row) => row?.id).filter((v): v is string | number => v != null);
+                   if (summaryKeyColumns.length > 0) summaryPriorRows = withoutDeletedSelfParents(object, matched, matchedIds, this.getSummaryDescriptors(object));
                    for (const matchedId of matchedIds) {
                      await this.cascadeDeleteRelations(object, matchedId, writeContext);
                    }
@@ -19926,9 +19983,11 @@ export class ObjectQL implements IObjectQLEngine {
           // from the row's FK values captured BEFORE deletion. [#5272] That
           // capture is now the same single pre-image read `previous` rides on
           // (it used to be its own later `findOne`), which is also what
-          // `update()` passes here.
-          const summaryFailures = priorRecord
-            ? await this.recomputeSummaries(object, null, priorRecord, opCtx.context)
+          // `update()` passes here. [#26950] A predicate delete passes the
+          // matched rows it read before the delete.
+          const summaryPrevious = priorRecord ?? summaryPriorRows;
+          const summaryFailures = summaryPrevious
+            ? await this.recomputeSummaries(object, null, summaryPrevious, opCtx.context)
             : [];
 
           // Same split as update(): per-record `data.record.deleted` (#4626),

packages/objectql/src/summary-aggregate.ts

@@ -120,3 +120,38 @@ export async function aggregateSummaryValue(
   const value = rows?.[0]?.value;
   return value == null ? summaryEmptySetValue(desc.fn) : value;
 }
+
+/**
+ * [#26950] The matched rows a predicate delete hands the recompute, less every
+ * foreign key that names a row the same delete removed.
+ *
+ * A roll-up over its own object (a `summary` aggregating `object` through a
+ * self-reference) can name, from one matched row, a parent that is itself a
+ * matched row. That parent is gone after the delete, so it has no roll-up left
+ * to recompute, and asking would answer not-found for a row the delete itself
+ * took: a `SummaryRecomputeError` on a delete that succeeded. The by-id delete
+ * never meets this, because it deletes one row and its children's own deletes
+ * recompute it while it still exists. Only self-referencing descriptors are
+ * touched; every other foreign key is handed over as read.
+ */
+export function withoutDeletedSelfParents(
+  object: string,
+  rows: Array<Record<string, unknown>>,
+  deletedIds: ReadonlyArray<string | number>,
+  descriptors: readonly SummaryDescriptor[],
+): Array<Record<string, unknown>> {
+  const selfKeys = descriptors.filter((desc) => desc.parentObject === object).map((desc) => desc.fkField);
+  if (selfKeys.length === 0 || deletedIds.length === 0) return rows;
+  const gone = new Set(deletedIds.map(String));
+  return rows.map((row) => {
+    let out = row;
+    for (const key of selfKeys) {
+      const v = row?.[key];
+      if (v != null && gone.has(String(v))) {
+        if (out === row) out = { ...row };
+        out[key] = null;
+      }
+    }
+    return out;
+  });
+}