Skip to content

Commit d56bcdb

Browse files
baozhoutaoclaude
andauthored
fix(objectql): seed count/sum roll-up summaries at parent insert (#5749) (#6013)
`recomputeSummaries()` only ever visits parents named by a CHILD write (`recs`/`prevs` -> `desc.fkField`), so a parent that has never had a child is never visited and its summary column keeps insert's `null`. Delete the last child and the parent IS visited (via `previous`) and lands on 0 — one logical state, two values. The consequence is not cosmetic: `= 0` / `< 1` filters compare in the database and silently DROP every parent that never had a child; sorting, GROUP BY and formula fields reading it inherit the same null. Fixed at the producer. `buildSummaryIndex` now publishes the identical descriptors under a second, parent-side view, and `insert` seeds the count/sum summaries a new row OWNS with the empty-collection value right after `applyFieldDefaults`. The empty-set function list is extracted to `summaryEmptySetValue` so the insert seed and the recompute fallback read ONE list — min/max/avg have no empty-set value and stay `null`, unchanged. Boundaries: author-supplied values are never overwritten (same `!= null` rule as `applyFieldDefaults`, #2706) and `beforeInsert` still has the final say; a roll-up whose relationship cannot be resolved is not seeded, so "seeded" and "maintained by recompute" stay the same set; existing rows are untouched — this is create-time only and backfill is a separate decision. Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1eb13a0 commit d56bcdb

3 files changed

Lines changed: 321 additions & 14 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): 新建父行时把 count/sum 型 `Field.summary` 汇总字段初始化为 0
6+
7+
`recomputeSummaries()` 只会重算「本次子记录写入所指向的父行」,所以一个**从未有过子记录**的父行永远不会被访问到,汇总列停在 insert 时的 `null`;而把最后一条子记录**删掉**的父行反而会被访问到(经由 `previous`)并落到 `0`。同一个「零个子记录」的逻辑状态因此读出两个不同的值。
8+
9+
后果不是显示难看,而是**筛选静默漏行**:`["task_count","=",0]` / `["task_count","<",1]`(库内比对)会把「从来没建过任务的项目」整行丢掉,没有任何报错;排序、GROUP BY、以及以该字段为输入的公式字段(null 传播)同样受影响。
10+
11+
本次在**生产端**修:父行 insert 时,按父对象取一份 summary descriptor,把该行自己拥有的 `count` / `sum` 汇总字段落成空集合的值 `0` —— 与 `recomputeSummaries` 的空集兜底共用同一份函数清单(现已提取为单一来源),所以「从未有过子记录」与「删光子记录」必然读到同一个值。`min` / `max` / `avg` 在空集上没有定义,仍然保持 `null`,口径不变。
12+
13+
边界:
14+
15+
- 作者显式提供的值不会被覆盖(与 `applyFieldDefaults` 同口径:insert`undefined` 与显式 `null` 都算「未提供」);`beforeInsert` 钩子仍有最终决定权。
16+
- 关系无法解析的汇总字段不落初值 ——「落了初值」与「会被重算维护」是同一个集合,不会出现一个没人维护的 `0`
17+
- **存量数据不受本 PR 影响**:这是 create-time 初始化,已经存成 `null` 的老父行仍然是 `null`,直到某次子记录写入把它重算。存量回填是独立取舍,另行处理。

packages/objectql/src/engine.ts

Lines changed: 138 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,23 @@ interface SummaryDescriptor {
804804
filter?: Record<string, unknown>;
805805
}
806806

807+
/**
808+
* The value a roll-up summary takes over an **empty** child collection (#5749).
809+
*
810+
* `count` and `sum` are defined on the empty set — zero children is zero, not
811+
* "unknown" — while `min`/`max`/`avg` are not, so those stay `null`. This is the
812+
* ONE place that list is written down: {@link ObjectQL.recomputeSummaries} uses
813+
* it for the post-aggregate fallback (an aggregate over no rows returns
814+
* `null`/`undefined` on every driver), and the insert-time initialiser
815+
* {@link ObjectQL.initializeSummaryFields} uses it to seed a brand-new parent
816+
* row with the same value the first recompute would have produced. Two sites,
817+
* one list — a parent that has never had a child and a parent whose last child
818+
* was deleted are the SAME logical state and must read the same value.
819+
*/
820+
function summaryEmptySetValue(fn: SummaryDescriptor['fn']): number | null {
821+
return fn === 'count' || fn === 'sum' ? 0 : null;
822+
}
823+
807824
// `implements IObjectQLEngine` is the verification step of #4251 B3: every
808825
// member the `objectql` slot's contract declares is checked against this class
809826
// on every build, so the seven consumer-local surface declarations the contract
@@ -4156,6 +4173,15 @@ export class ObjectQL implements IObjectQLEngine {
41564173
* parent objects that aggregate it. Invalidated when packages register. */
41574174
private summaryIndex: Map<string, SummaryDescriptor[]> | null = null;
41584175

4176+
/** The SAME descriptors, indexed the other way: parent object name → the
4177+
* roll-up summary fields that object OWNS. Built in the same pass as
4178+
* {@link summaryIndex} and invalidated with it. The child index answers
4179+
* "whose summaries must I recompute after writing this row"; this one answers
4180+
* "which of my own summary fields must be seeded when I create this row"
4181+
* (#5749) — the question the child index structurally cannot answer, because
4182+
* a parent that has never had a child appears in no child write. */
4183+
private summaryIndexByParent: Map<string, SummaryDescriptor[]> | null = null;
4184+
41594185
/**
41604186
* Retry options for roll-up summary recompute (framework#3147). Public so a
41614187
* test can inject a no-op sleep for deterministic backoff; production uses
@@ -4166,12 +4192,20 @@ export class ObjectQL implements IObjectQLEngine {
41664192
/** Invalidate the cached roll-up summary index (call when metadata changes). */
41674193
private invalidateSummaryIndex(): void {
41684194
this.summaryIndex = null;
4169-
}
4170-
4171-
/** Scan all registered objects for `summary` fields and index them by the
4172-
* child object they aggregate, resolving the child→parent FK field. */
4173-
private buildSummaryIndex(): Map<string, SummaryDescriptor[]> {
4195+
this.summaryIndexByParent = null;
4196+
}
4197+
4198+
/** Scan all registered objects for `summary` fields and index them BOTH ways
4199+
* — by the child object they aggregate and by the parent object that owns
4200+
* them — resolving the child→parent FK field. One scan, two views of the
4201+
* identical descriptor objects, so the two indexes can never disagree about
4202+
* which roll-ups exist. */
4203+
private buildSummaryIndex(): {
4204+
byChild: Map<string, SummaryDescriptor[]>;
4205+
byParent: Map<string, SummaryDescriptor[]>;
4206+
} {
41744207
const index = new Map<string, SummaryDescriptor[]>();
4208+
const byParent = new Map<string, SummaryDescriptor[]>();
41754209
let objects: any[] = [];
41764210
try { objects = (this._registry as any).getAllObjects?.() ?? []; } catch { objects = []; }
41774211
for (const parent of objects) {
@@ -4204,18 +4238,34 @@ export class ObjectQL implements IObjectQLEngine {
42044238
const filter = so.filter && typeof so.filter === 'object' && !Array.isArray(so.filter)
42054239
? so.filter as Record<string, unknown>
42064240
: undefined;
4241+
const descriptor: SummaryDescriptor = {
4242+
parentObject: parent.name, summaryField, fkField, fn, sourceField: so.field, filter,
4243+
};
42074244
const list = index.get(childObject) ?? [];
4208-
list.push({ parentObject: parent.name, summaryField, fkField, fn, sourceField: so.field, filter });
4245+
list.push(descriptor);
42094246
index.set(childObject, list);
4247+
// Same descriptor, parent-side view. Only descriptors that made it this
4248+
// far are indexed either way, so "seeded at insert" and "maintained by
4249+
// recompute" are the same set by construction — a roll-up whose
4250+
// relationship could not be resolved (the `continue` above) is left
4251+
// untouched on both paths rather than seeded with a 0 nothing updates.
4252+
const owned = byParent.get(parent.name) ?? [];
4253+
owned.push(descriptor);
4254+
byParent.set(parent.name, owned);
42104255
}
42114256
}
4212-
return index;
4257+
return { byChild: index, byParent };
42134258
}
42144259

42154260
/** `registry.objectRevision` the cached {@link summaryIndex} was built at. */
42164261
private summaryIndexRevision = -1;
42174262

4218-
private getSummaryDescriptors(childObject: string): SummaryDescriptor[] {
4263+
/**
4264+
* Ensure both roll-up indexes are present and current. Split out of
4265+
* {@link getSummaryDescriptors} so the parent-side view (#5749) shares the
4266+
* exact same staleness rule instead of re-deriving one.
4267+
*/
4268+
private ensureSummaryIndexes(): void {
42194269
// Rebuild whenever the REGISTRY's object set has moved since the index was
42204270
// built — not only when someone remembered to call
42214271
// `invalidateSummaryIndex`. That single site (`registerApp`) is bypassed by
@@ -4228,11 +4278,67 @@ export class ObjectQL implements IObjectQLEngine {
42284278
// "已完成任务数" shipped empty over correct metadata (cloud#970).
42294279
const revision = (this._registry as unknown as { objectRevision?: number })?.objectRevision;
42304280
const stale = typeof revision === 'number' && revision !== this.summaryIndexRevision;
4231-
if (!this.summaryIndex || stale) {
4232-
this.summaryIndex = this.buildSummaryIndex();
4281+
if (!this.summaryIndex || !this.summaryIndexByParent || stale) {
4282+
const built = this.buildSummaryIndex();
4283+
this.summaryIndex = built.byChild;
4284+
this.summaryIndexByParent = built.byParent;
42334285
if (typeof revision === 'number') this.summaryIndexRevision = revision;
42344286
}
4235-
return this.summaryIndex.get(childObject) ?? [];
4287+
}
4288+
4289+
/** Roll-up descriptors for summaries that aggregate `childObject` — i.e. the
4290+
* ones a write to `childObject` must recompute. Semantics unchanged. */
4291+
private getSummaryDescriptors(childObject: string): SummaryDescriptor[] {
4292+
this.ensureSummaryIndexes();
4293+
return this.summaryIndex!.get(childObject) ?? [];
4294+
}
4295+
4296+
/** Roll-up descriptors for the summary fields `parentObject` OWNS (#5749) —
4297+
* i.e. the ones a NEW row of `parentObject` must have seeded. */
4298+
private getOwnedSummaryDescriptors(parentObject: string): SummaryDescriptor[] {
4299+
this.ensureSummaryIndexes();
4300+
return this.summaryIndexByParent!.get(parentObject) ?? [];
4301+
}
4302+
4303+
/**
4304+
* Seed the roll-up `summary` fields a freshly-created row owns (#5749).
4305+
*
4306+
* `recomputeSummaries` only ever visits parents named by a child write, so a
4307+
* parent that has NEVER had a child is never visited and its summary column
4308+
* keeps whatever insert put there — `null`. Delete the last child and the
4309+
* parent DOES get visited (via `previous`) and lands on 0. Same logical state,
4310+
* two different values: `filter ["task_count","=",0]` silently skipped every
4311+
* parent that never had a child, and so did sorting, GROUP BY and any formula
4312+
* reading the field (null propagation).
4313+
*
4314+
* The fix is at the producer: write the empty-collection value at create time,
4315+
* so `count`/`sum` start at 0 and only ever move to another number.
4316+
* `min`/`max`/`avg` have no empty-set value and deliberately stay `null` —
4317+
* {@link summaryEmptySetValue} is the single list both this and the recompute
4318+
* fallback read.
4319+
*
4320+
* Author-supplied values are never overwritten. The `!= null` test matches
4321+
* {@link applyFieldDefaults} exactly (#2706): on INSERT an explicit `null` is
4322+
* "no value supplied", any real value — including a deliberate 0 or a seeded
4323+
* count — is respected. Runs before the `beforeInsert` hooks for the same
4324+
* reason defaults do, so a hook still has the final say.
4325+
*
4326+
* Existing rows are untouched: this is create-time only, so parents already
4327+
* stored with `null` stay `null` until a child write recomputes them.
4328+
*/
4329+
private initializeSummaryFields(object: string, record: any): any {
4330+
const descriptors = this.getOwnedSummaryDescriptors(object);
4331+
if (descriptors.length === 0) return record;
4332+
if (!record || typeof record !== 'object' || Array.isArray(record)) return record;
4333+
let out: Record<string, unknown> = record;
4334+
for (const desc of descriptors) {
4335+
const seed = summaryEmptySetValue(desc.fn);
4336+
if (seed == null) continue; // min/max/avg — undefined on an empty set
4337+
if (out[desc.summaryField] != null) continue; // author supplied a value
4338+
if (out === record) out = { ...record };
4339+
out[desc.summaryField] = seed;
4340+
}
4341+
return out;
42364342
}
42374343

42384344
/**
@@ -4277,7 +4383,10 @@ export class ObjectQL implements IObjectQLEngine {
42774383
context: execCtx,
42784384
} as any);
42794385
let value = rows?.[0]?.value;
4280-
if (value == null) value = (desc.fn === 'count' || desc.fn === 'sum') ? 0 : null;
4386+
// An aggregate over no rows returns null/undefined on every driver.
4387+
// Behaviour unchanged — the empty-set list simply moved to the one
4388+
// place the insert-time seed reads it from too (#5749).
4389+
if (value == null) value = summaryEmptySetValue(desc.fn);
42814390
await this.update(desc.parentObject, { id: parentId, [desc.summaryField]: value }, { context: execCtx } as any);
42824391
}, this.summaryRetryOptions);
42834392
} catch (err) {
@@ -5037,13 +5146,28 @@ export class ObjectQL implements IObjectQLEngine {
50375146
// (#2703). The hook still has final say — it runs after and may override
50385147
// any defaulted field. `applyFieldDefaults` returns a fresh copy and only
50395148
// fills fields left `undefined`, so client-supplied values are untouched.
5149+
//
5150+
// [#5749] Roll-up `summary` fields this object OWNS are seeded in the same
5151+
// pass, right after the declared defaults: `count`/`sum` over the empty
5152+
// child collection is 0, and a brand-new parent HAS an empty child
5153+
// collection. Without it the row stored `null` and stayed there until some
5154+
// child write happened to name it — so "never had a child" (null) and
5155+
// "had one, deleted it" (0) read differently and `= 0` filters dropped
5156+
// rows. Same placement rules as the defaults above: caller-supplied values
5157+
// untouched, hooks run after and may override.
50405158
const nowSnap = new Date();
50415159
const isBatch = Array.isArray(opCtx.data);
50425160
const defaultedData = isBatch
50435161
? (opCtx.data as any[]).map((row) =>
5044-
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
5162+
this.initializeSummaryFields(
5163+
object,
5164+
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
5165+
),
50455166
)
5046-
: this.applyFieldDefaults(object, opCtx.data as Record<string, unknown>, opCtx.context, nowSnap);
5167+
: this.initializeSummaryFields(
5168+
object,
5169+
this.applyFieldDefaults(object, opCtx.data as Record<string, unknown>, opCtx.context, nowSnap),
5170+
);
50475171

50485172
// Batch inserts trigger beforeInsert/afterInsert PER ROW, each with the
50495173
// exact single-record context shape (`input.data` = one row, `result` =

0 commit comments

Comments
 (0)