Skip to content

Commit ee3bde1

Browse files
baozhoutaoclaude
andauthored
feat(objectql,cli): os migrate 新增 summary count/sum 存量 NULL 回填迁移 (#6063) (#6158)
* feat(objectql,cli): os migrate summary-nulls backfills pre-seed NULL count/sum roll-ups (#6063) PR #6013 (#5749) seeds a roll-up's empty-set value at parent INSERT, which reaches new rows only: a database upgraded in place keeps pre-upgrade parents at NULL, because the recompute that would fix them runs only when one of their children is written. Those rows keep vanishing from `= 0` filters, sorts, GROUP BY and formulas. This adds the one-off, explicit data migration for them. - `backfillSummaryNulls` (packages/objectql/src/summary-backfill.ts): walk each object owning a count/sum roll-up, and recompute every row whose column is stored NULL. A pre-upgrade parent WITH children is NULL too and its correct value is the real aggregate, so `SET col = 0 WHERE col IS NULL` is wrong, not merely coarse. Dry run by default; idempotent; driver-agnostic (values are read and tested in JS, no null predicate pushed down); one row's failure is recorded and the run continues. - min/max/avg are never touched: undefined on an empty set, so a stored null there is the correct reading of "no child rows". The report names them as deliberately skipped. - `summary-aggregate.ts`: SummaryDescriptor, summaryEmptySetValue and the single-descriptor aggregate lifted out of engine.ts unchanged, so the seed, the recompute and the backfill share ONE computation instead of three that agree until one is edited. The descriptor gains `childObject` so the parent-side index is usable on its own. - `os migrate summary-nulls`: thin oclif shell over the migration, following the files-to-references precedent (occupancy gate, --apply/--yes, --object, --max-records, --json). No deployment flag — nothing is gated on this run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We * fix(objectql): type the roll-up aggregate call instead of grandfathering it (#6063) `check:query-options-erasure` rejects a NEW file in its baseline — the grandfather list only shrinks. The `as any` came along with the code lifted out of engine.ts and is not needed there: `SummaryAggregateEngine.aggregate` takes the query directly, so the cast is dropped and the baseline records only engine.ts's own count falling 13 -> 12. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We * fix(cli): type the objectql lookup in summary-nulls instead of erasing it to any (#6063) The slot-lookup rule (#4168/#4251) rejects `const engine: any = getService('objectql')` — the sibling migrate commands are silent only because they are grandfathered by file, and that baseline only shrinks. The command's real requirement is `SummaryBackfillEngine` (the slot contract plus the one member the backfill reads), so name that type: the call site keeps its checking and the lookup is not erased. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5e1ea65 commit ee3bde1

9 files changed

Lines changed: 1198 additions & 55 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/cli": patch
4+
---
5+
6+
feat(objectql,cli): `os migrate summary-nulls` backfills roll-up count/sum columns left NULL by pre-seed inserts (#6063)
7+
8+
#5749 / PR #6013 fixed the **producer**: a parent row created from that release
9+
on has its `count` / `sum` roll-up columns seeded to the empty-set value at
10+
insert, so `filter ["task_count", "=", 0]`, sorting, `GROUP BY` and formulas
11+
over the column stop silently dropping parents that never had a child.
12+
13+
Being a create-time fix, it reaches **new rows only**. A database upgraded **in
14+
place** still holds parents stored before the upgrade, and the recompute that
15+
would otherwise correct them runs only when one of their **children** is
16+
written — so those rows keep their `NULL` indefinitely and keep disappearing
17+
from the same queries. A freshly seeded deployment is correct; an upgraded one
18+
is not. This release ships the other half: a one-off, explicit data migration.
19+
20+
```bash
21+
os migrate summary-nulls # dry run: full report, writes nothing
22+
os migrate summary-nulls --apply # recompute and write (prompts)
23+
os migrate summary-nulls --apply --yes --json # CI / scripts
24+
os migrate summary-nulls --object project # restrict to one object (repeatable)
25+
```
26+
27+
**Every NULL row is recomputed, never blanket-set to 0.** A pre-upgrade parent
28+
that *does* have children is `NULL` too — nothing ever recomputed it — and its
29+
correct value is the real aggregate. `UPDATE ... SET col = 0 WHERE col IS NULL`
30+
would replace a visibly-missing value with a confidently-wrong one, which the
31+
next child write would then silently change back. The run computes each value
32+
through the same code path the engine's own child-write recompute uses
33+
(`aggregateSummaryValue`), over the descriptors the engine itself maintains, so
34+
a backfilled column and a recomputed one can never mean different things.
35+
36+
**`min` / `max` / `avg` are never touched.** They are undefined on an empty set
37+
— which is why the insert-time seed leaves them `null` — so a stored `null`
38+
there is the correct reading of "no child rows", not a defect. The report names
39+
them as deliberately skipped rather than omitting them silently.
40+
41+
Other properties: dry run by default and a dry run writes nothing at all;
42+
idempotent, so re-running is safe and a clean report is the operator's own
43+
verification; driver-agnostic (it reads values and tests them in JS rather than
44+
pushing a null predicate down, since null-predicate compilation is precisely
45+
where drivers diverge); one row's failure is recorded and the run carries on.
46+
It records no deployment flag — unlike its `os migrate` siblings, nothing is
47+
gated on it having run.
48+
49+
Never running it is safe in the sense that nothing breaks *further*: the
50+
affected rows simply stay missing from `= 0` filters until a child of theirs is
51+
written.

content/docs/deployment/cli.mdx

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ where the data lives.
659659
|---------|-------------|
660660
| `os migrate files-to-references` | Convert legacy file-field values to `sys_file` references, verify the ownership ledger, and record the deployment's migration flag |
661661
| `os migrate value-shapes` | Scan stored reference and structured-JSON field values against the platform's value contract, and record the deployment's migration flag when clean |
662+
| `os migrate summary-nulls` | Backfill roll-up `count` / `sum` columns still stored as `NULL` on parent rows created before the insert-time seed. Repairs values; no flag, nothing depends on it having run |
662663
| `os migrate meta --stored` | Replay the metadata conversion chain over this deployment's `sys_metadata` rows and rewrite the ones still carrying a pre-protocol shape. Hygiene, not a gate — nothing depends on it having run |
663664

664665
```bash
@@ -755,6 +756,43 @@ Same writing rules as its sibling: a dry run writes **nothing**, `--apply` is
755756
the only writing mode, a later failing run clears the verified state, and a
756757
running server reads the flag once — **restart it** after a successful apply.
757758

759+
#### `os migrate summary-nulls`
760+
761+
A roll-up `summary` field of function `count` or `sum` is **0** over an empty
762+
child collection — zero children is zero, not "unknown" — and since #5749 a
763+
parent row is created holding that value. Rows created *before* that are the
764+
exception: nothing seeded them, and the recompute that maintains a roll-up runs
765+
only when one of the parent's **children** is written, so a parent that has
766+
never had a child keeps its `NULL` indefinitely. `filter ["task_count", "=", 0]`
767+
then silently omits it, and so do sorting, `GROUP BY` and any formula reading
768+
the column (null propagation).
769+
770+
```bash
771+
os migrate summary-nulls # Dry run: full report, writes nothing
772+
os migrate summary-nulls --apply # Recompute and write (prompts)
773+
os migrate summary-nulls --apply --yes --json # CI / scripts
774+
os migrate summary-nulls --object project # Restrict to one object (repeatable)
775+
```
776+
777+
**Each affected row is recomputed, not set to 0.** A pre-upgrade parent that
778+
*does* have children is `NULL` too, and its correct value is the aggregate over
779+
them — writing 0 there would replace a missing value with a wrong one, and the
780+
next child write would change it back. The report separates the two: `N NULL
781+
row(s), M with real child data`.
782+
783+
`min` / `max` / `avg` are **never touched**. They are undefined on an empty set,
784+
so a `null` there is the correct reading of "no child rows"; the report lists
785+
them as deliberately skipped.
786+
787+
Idempotent — every write turns a `NULL` into a number, so a second run finds
788+
nothing and writes nothing. Re-running until the report says zero *is* the
789+
verification, which is why this command records no flag: it repairs values and
790+
changes no behaviour, so there is no posture for a flag to attest.
791+
792+
A deployment whose database was seeded fresh on this version has nothing to do
793+
here: its parents were created with the value already in place, and the run
794+
reports zero.
795+
758796
#### A database created by this version needs no migration
759797

760798
A deployment whose database the platform **creates from empty** records these
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { Command, Flags } from '@oclif/core';
4+
import chalk from 'chalk';
5+
import { createInterface } from 'node:readline';
6+
import {
7+
printHeader,
8+
printSuccess,
9+
printWarning,
10+
printError,
11+
printInfo,
12+
printStep,
13+
createTimer,
14+
emitJson,
15+
isExitSignal,
16+
} from '../../utils/format.js';
17+
import { bootSchemaStack } from '../../utils/schema-migrate.js';
18+
import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
19+
import { describeOccupancy } from '../../utils/sqlite-occupancy.js';
20+
import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
21+
// Type-only, so the heavy engine package is still loaded lazily below: this is
22+
// the surface the migration actually needs, which is wider than the `objectql`
23+
// slot contract by exactly one member (`getOwnedSummaryDescriptors`). Naming it
24+
// here keeps the call site checked instead of erasing the lookup to `any`
25+
// (#4168/#4251).
26+
import type { SummaryBackfillEngine } from '@objectstack/objectql';
27+
28+
async function confirm(question: string): Promise<boolean> {
29+
if (!process.stdin.isTTY) return false; // non-interactive → require --yes
30+
const rl = createInterface({ input: process.stdin, output: process.stdout });
31+
try {
32+
const answer: string = await new Promise((resolve) => rl.question(question, resolve));
33+
return /^y(es)?$/i.test(answer.trim());
34+
} finally {
35+
rl.close();
36+
}
37+
}
38+
39+
/**
40+
* `os migrate summary-nulls` — the one-off backfill of roll-up `count`/`sum`
41+
* columns left `NULL` by inserts predating PR #6013 (#6063, the second half of
42+
* #5749).
43+
*
44+
* PR #6013 fixed the producer: a parent created from that release on starts its
45+
* `count`/`sum` roll-ups at 0. It cannot reach rows that already exist, and the
46+
* recompute only ever visits a parent when one of its CHILDREN is written — so
47+
* a database upgraded IN PLACE keeps pre-upgrade parents at `NULL`, and every
48+
* `= 0` filter, sort, GROUP BY and formula over the column silently drops them.
49+
* A freshly seeded database has no such rows; this command is for the other
50+
* kind, and is needed exactly once per deployment.
51+
*
52+
* Dry run by default (writes nothing at all), `--apply` to backfill. Each
53+
* `NULL` row is RECOMPUTED — a pre-upgrade parent that has children is `NULL`
54+
* too and its correct value is the real aggregate, so writing 0 everywhere
55+
* would swap a missing value for a wrong one. Idempotent: a second run finds
56+
* nothing left to do.
57+
*
58+
* `min`/`max`/`avg` are never touched — undefined on an empty set, so a `null`
59+
* there is the correct reading of "no child rows", not a defect.
60+
*
61+
* ## No deployment flag, deliberately
62+
*
63+
* Its siblings (`files-to-references`, `value-shapes`) record a `sys_migration`
64+
* flag because that flag is what later OPENS irreversible behaviour on the
65+
* deployment. Nothing is gated on this run: it repairs values and changes no
66+
* posture, and its own idempotence is the verification (re-run; a clean report
67+
* is the evidence). A flag here would be a fact nothing reads.
68+
*/
69+
export default class MigrateSummaryNulls extends Command {
70+
static override description =
71+
'Backfill roll-up count/sum summary columns still stored as NULL on parent rows created before the ' +
72+
'insert-time seed (#5749). Dry-run by default; --apply recomputes and writes each affected row.';
73+
74+
static override examples = [
75+
'$ os migrate summary-nulls',
76+
'$ os migrate summary-nulls --apply',
77+
'$ os migrate summary-nulls --apply --yes --json',
78+
'$ os migrate summary-nulls --object project',
79+
];
80+
81+
static override flags = {
82+
'database-url': Flags.string({
83+
description: 'Database URL to migrate (defaults to $OS_DATABASE_URL / the project DB)',
84+
env: 'OS_DATABASE_URL',
85+
}),
86+
apply: Flags.boolean({
87+
description: 'Write the recomputed values (default is a read-only dry run)',
88+
default: false,
89+
}),
90+
yes: Flags.boolean({ char: 'y', description: 'Skip the --apply confirmation prompt', default: false }),
91+
force: Flags.boolean({
92+
description: 'Apply even when another process is using the database (SQLite occupancy check)',
93+
default: false,
94+
}),
95+
object: Flags.string({
96+
description: 'Restrict to this object (repeatable; default: every object owning a count/sum roll-up)',
97+
multiple: true,
98+
}),
99+
'max-records': Flags.integer({
100+
description: 'Safety bound on parent rows read per object — exceeding it truncates the walk',
101+
}),
102+
json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to apply)' }),
103+
};
104+
105+
async run(): Promise<void> {
106+
const { flags } = await this.parse(MigrateSummaryNulls);
107+
const timer = createTimer();
108+
const apply = flags.apply;
109+
110+
if (!flags.json) printHeader('Migrate · summary-nulls');
111+
112+
// Occupancy gate, like `files-to-references`: an apply run rewrites ROWS,
113+
// so a second writer on the same SQLite file is a real hazard. Probed
114+
// before boot (afterwards our own pool is what the probe finds) and before
115+
// the prompt, so an operator is never asked to confirm a run we refuse.
116+
const occupancy = await probeMigrationTarget(flags['database-url']);
117+
if (occupancy.status === 'busy' && apply && !flags.force) {
118+
if (flags.json) {
119+
await emitJson({
120+
error: 'database_busy',
121+
database: occupancy.filename,
122+
signal: occupancy.signal,
123+
detail: occupancy.detail,
124+
hint: OCCUPANCY_HINT,
125+
}, 0, { compact: true });
126+
this.exit(1);
127+
return;
128+
}
129+
printError(describeOccupancy(occupancy));
130+
printWarning(OCCUPANCY_HINT);
131+
this.exit(1);
132+
return;
133+
}
134+
if (occupancy.status === 'busy' && !flags.json) {
135+
printWarning(apply
136+
? `--force: ${describeOccupancy(occupancy)} Backfilling anyway — the live process may write rows mid-walk.`
137+
: `${describeOccupancy(occupancy)} The dry run below writes nothing, but its counts may shift while that process is running.`);
138+
}
139+
if (occupancy.status === 'unknown' && !flags.json) {
140+
printWarning(`Could not check whether the database is in use — ${occupancy.detail}`);
141+
}
142+
143+
if (apply && !flags.yes) {
144+
if (flags.json || !process.stdin.isTTY) {
145+
if (flags.json) {
146+
await emitJson({ error: 'confirmation_required', hint: 'pass --yes' }, 0, { compact: true });
147+
this.exit(1);
148+
return;
149+
}
150+
printWarning('Apply mode rewrites record data. Re-run with --yes to confirm, or run without --apply to preview.');
151+
this.exit(1);
152+
return;
153+
}
154+
const ok = await confirm(
155+
chalk.bold('\nRecompute and write every NULL count/sum roll-up value on this database? [y/N] '),
156+
);
157+
if (!ok) {
158+
printInfo('Aborted — no changes made.');
159+
return;
160+
}
161+
}
162+
163+
if (!flags.json) {
164+
printStep(apply ? 'Booting data stack (APPLY mode)…' : 'Booting data stack (dry run)…');
165+
}
166+
167+
let stack;
168+
try {
169+
stack = await bootSchemaStack({
170+
databaseUrl: flags['database-url'],
171+
extraPlugins: await buildDataMigrationPlugins(),
172+
});
173+
} catch (error: any) {
174+
if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); }
175+
printError(error.message || String(error));
176+
this.exit(1);
177+
return;
178+
}
179+
180+
try {
181+
const engine: SummaryBackfillEngine = stack.kernel.getService('objectql');
182+
if (typeof engine?.getOwnedSummaryDescriptors !== 'function') {
183+
throw new Error('No ObjectQL engine on this stack — cannot read the roll-up index.');
184+
}
185+
// An empty walk is indistinguishable from a clean one, and "clean" is the
186+
// answer an operator will act on — so refuse to run when no app metadata
187+
// is loaded (missing artifact / wrong directory) rather than report a
188+
// database the walk never looked at.
189+
const loadedObjects: string[] =
190+
typeof engine.getConfigs === 'function' ? Object.keys(engine.getConfigs()) : [];
191+
if (!loadedObjects.some((name) => !name.startsWith('sys_'))) {
192+
throw new Error(
193+
'No app objects are loaded, so the walk would examine nothing. ' +
194+
'Run "os build" in your project root first (the migration reads dist/objectstack.json), then re-run.',
195+
);
196+
}
197+
198+
const { backfillSummaryNulls, formatSummaryBackfillReport } = await import('@objectstack/objectql');
199+
200+
// In JSON mode keep stdout parseable — route warnings to stderr.
201+
const logger = flags.json
202+
? { info: (m: string) => console.error(m), warn: (m: string) => console.error(m) }
203+
: { info: (m: string) => printInfo(m), warn: (m: string) => printWarning(m) };
204+
205+
const report = await backfillSummaryNulls(engine, logger, {
206+
apply,
207+
objects: flags.object,
208+
maxRecordsPerObject: flags['max-records'],
209+
});
210+
211+
if (flags.json) {
212+
await emitJson({ database: stack.dbLabel, apply, report, duration: timer.elapsed() });
213+
if (report.failures.length > 0) this.exit(1);
214+
return;
215+
}
216+
217+
printInfo(`Database: ${chalk.white(stack.dbLabel)}`);
218+
console.log('');
219+
console.log(formatSummaryBackfillReport(report).join('\n'));
220+
console.log('');
221+
222+
if (report.failures.length > 0) {
223+
printError(`${report.failures.length} row(s) could not be recomputed — re-run to finish them.`);
224+
} else if (apply && report.filled > 0) {
225+
printSuccess(
226+
`Backfilled ${report.filled} roll-up value(s) across ${report.fields.length} column(s). ` +
227+
'Re-run any time — it only ever revisits rows still holding NULL.',
228+
);
229+
} else if (apply) {
230+
printSuccess('Nothing to backfill — every count/sum roll-up already holds a value.');
231+
} else if (report.nullRows > 0) {
232+
printInfo(`Dry run only — ${report.nullRows} value(s) would be recomputed. Re-run with --apply.`);
233+
} else {
234+
printSuccess('Nothing to backfill — every count/sum roll-up already holds a value.');
235+
}
236+
console.log(chalk.dim(` ${timer.display()}`));
237+
console.log('');
238+
if (report.failures.length > 0) this.exit(1);
239+
} catch (error: any) {
240+
if (isExitSignal(error)) throw error;
241+
if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); }
242+
printError(error.message || String(error));
243+
this.exit(1);
244+
} finally {
245+
await stack.shutdown();
246+
}
247+
}
248+
}

0 commit comments

Comments
 (0)