Skip to content

Commit cd5f554

Browse files
fix(objectql,cli): os migrate stored-expressions fills a stored computed field on rows stored before it was declared, and the boot names it (#30227)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg1cdCRrn63ZpnD6M1busUdm)_ domain:server#2 · os-bill · session_01Utp6Cb2FL7H75wB1wA6PpZ ## 维护者速读 - **改了什么:** 新增 `os migrate stored-expressions`:给"在已有数据的对象上新加的存储型计算字段"补算存量行(默认只补 NULL,`--changed` 再重算表达式改过后不一致的值),先报告、`--apply` 才写;另在启动时抽查每个声明了存储型计算字段的对象的前 100 行,发现这种 NULL 就告警一次并点名命令。 - **为什么改:** 存储型计算字段只在写入时计算,升级后的库里新字段在所有旧行上一直是 NULL(hotclm 121/121 行),分析看板把 NULL 当 0 天,静默多算超期。 - **风险与回滚:** 命令默认只读;写入走 `engine.update`(保留 `updated_at`),钩子、汇总、审计照常触发;表达式在某行算不出而该行已有值时整行跳过并列出,不会用 NULL 覆盖。启动抽查只读、有上限、出错不影响启动。回滚即撤 PR。 - **先看:** `packages/objectql/src/stored-expression-backfill.ts`(分类与写入规则)、`packages/objectql/src/engine.ts` 新方法 `computeStoredExpressionsOver`、`packages/objectql/src/plugin.ts` 的 `kernel:bootstrapped` 一行。 - **两分钟验证:** `pnpm --filter @objectstack/objectql exec vitest run src/stored-expression-backfill.test.ts`,预期 29 个用例全过(含"加字段后旧行 NULL → --apply 补齐 → 再跑为 0")。 - **需要你做的:** 无,席位落地。 Fixes #29961 Clause-②: yes (widening) Metadata-protocol: none ## Why See the first two bullets of 维护者速读 and #29961: a stored computed field added to an object that already has rows stays NULL on every existing row until each row is next written, and an analytics view reading it treats the NULL as 0. ## What changes **The producer.** A stored computed field (a non-`formula` field declaring `expression`) is computed by `computeStoredExpressions` on insert and update only. Schema sync adds its column NULL on every existing row, and nothing computes it until each row is next written. The fix leaves the write path alone and adds the missing producer for existing rows, in the shape `os migrate summary-nulls` and `os migrate recompute-home-amounts` already have: a report-first command, plus a bounded boot advisory. - **`ObjectQL.computeStoredExpressionsOver(object, rows, context?, onFault?)`** (`engine.ts`, one method): the write's own plan (`planStoredExpressions`) and serving (declared option labels, the one hop, the `can()` map) over rows already stored. Evaluates over copies, writes nothing. - **`stored-expression-backfill.ts`** (new): `backfillStoredExpressions` walks every row by keyset (`id`), computes each page through the method above, and classifies each (row, field): - stored NULL, expression gives a value → **hole**: written by default; - stored NULL, expression gives NULL (an empty reference, a null input) → already the write's value, left alone; - stored value differs from the expression → **stale**: counted on every run, written only with `changed`; - the expression does not evaluate on the row → **faulted**: listed; when the row holds a value there, the whole row is skipped, because the write would store NULL over it. A written row goes through `engine.update` with the computed values and the row's `updated_at` under `preserveAudit`, as `recompute-home-amounts` writes. The write computes every stored field of the row again, so a stale value on a filled row is rewritten with it (counted under `written`). Refused before any row is read: an unknown object (`OBJECT_NOT_FOUND`), a named object or `field` declaring no stored computed field, and `field` without exactly one object (`INVALID_FIELD`, 400). Idempotent; no `sys_migration` flag (nothing gates on it). - **Boot advisory** (`announceStoredExpressionHoles`, called from the objectql plugin's `kernel:bootstrapped` hook beside `announceOpenMigrationGates`): one `find` of at most 100 rows per object that declares a stored computed field, nothing for objects declaring none. When the sample holds a hole, it warns once, naming each `object.field (n of the first m rows)` and the command. It writes nothing, and a read or compute failure is logged at debug and never fails the boot. A column added to an object with rows is NULL on every row, so the first page finds it. - **`os migrate stored-expressions`** (`packages/cli`, new, discovered by oclif's pattern strategy, so no registration file): `--object` (repeatable), `--field` (needs `--object`), `--changed`, `--apply`, `--yes`, `--json`, `--force`, `--max-records`, `--database-url`; the occupancy gate, confirmation, read-only dry-run boot and SQLite journal-mode note of its siblings. Exits 1 while a row is skipped, a write failed, or `--max-records` truncated the walk. - **Docs:** one row in `content/docs/deployment/cli.mdx`'s data-migration table. ## Tests Measured at `c82c1c7f` (branch merged with `origin/main` once before this PR). Head `72809e52` adds only the changeset's `Clause-②` line on top of it. - `packages/objectql/src/stored-expression-backfill.test.ts` (new, 29 tests): a real `ObjectQL` over an in-file driver. Dry run reports and writes nothing; `apply` fills holes, keeps `updated_at`, and a re-run finds 0; stale values are counted by default and written with `changed`; a faulted row that holds a value is skipped and listed; a refused write is isolated to its row; `field` narrows the walk; the three refusals; an unreadable object; the boot advisory (holes found, none found, a read failure that never throws); a `sameStoredValue` table. - `packages/cli/src/commands/migrate/stored-expressions.test.ts` (new, 10 tests, integration project): flags reach the walk, `--field` needs `--object`, `--apply` without `--yes` writes nothing and exits 1, an undeclared object is refused, exit 1 on skipped / failed / truncated, and the dry-run face closes with the report's line. - Pin and ledger rows: `migrate-confirmation-exit.pin.test.ts` (the new command in the confirmation roster) and `federated-injected-column-readers.test.ts` (the new `updated_at` reader skips an unprovisioned federated column). - Ablation: replacing the `if (!apply) continue;` guard turns 5 backfill tests red; restore verified blob == HEAD. ## Acceptance notes - **Boot backfill vs command, measured choice.** An automatic boot backfill was not taken: each fill is a write (hooks, record-triggered flows, audit rows, roll-up recomputes) nobody ordered, a large table would hold the boot for a full walk, a remote driver pays a round trip per row, and an expression reading a lookup reads another object per page. The boot advisory is bounded to one 100-row read per object declaring a stored computed field. - **Roll-ups have no boot notice.** The triage said "as roll-ups have"; on `main` the roll-up backfill (`summary-backfill.ts`) has no boot notice (the lint's `isAlwaysValuedSummary` docblock says "Nothing backfills a stored NULL automatically"). The in-tree boot-advisory shape is `announceOpenMigrationGates` at `kernel:bootstrapped`, which this follows. - **A changed expression.** Covered by `--changed`. No boot can tell that an expression changed (the stored value is not a function of the declaration alone, and a `now()`-reading expression always differs), so the advisory does not report stale values; every run of the command counts them and its report says to re-run with `--changed`. - **Clause-② corrected to `yes (widening)`.** The dispatch order proposed `no`. Measured: new published `@objectstack/objectql` exports (the stored-expression backfill functions and types), one new public method `ObjectQL.computeStoredExpressionsOver`, and a new CLI command `os migrate stored-expressions`; nothing narrows. Declared as #30183 declared the same shape; the changeset carries the same line from `72809e52`. `Metadata-protocol` stays `none`. - **Advisory false negatives.** Holes only past the first 100 rows by `id` are not seen by the advisory; the command walks every row. ## Verification <details><summary>Reproduction on SQLite and memory, before and after</summary> An object with 4 rows, booted; stored computed fields then declared (a doubled number, a date offset, a boolean, a text, a scale-2 decimal) and booted again over the same file. Before: 4/4 rows NULL in each new column on both drivers. `os migrate stored-expressions` dry run reported the holes and wrote nothing; `--apply` wrote the expected values (`qty_double` 2/4/6/8, `offset` -5/-5/-5/null for a row with an empty input); a re-run found 0 holes and 0 stale. Changing an expression to `qty*3` then showed 4 stale in default mode, `--changed --apply` rewrote them, and a re-run found 0. </details> <details><summary>Suites, typechecks, lint and gates</summary> ``` pnpm --filter @objectstack/objectql test :: exit 0 (549 files, 10892 tests) pnpm --filter @objectstack/objectql typecheck :: exit 0 pnpm --filter @objectstack/cli typecheck :: exit 0 pnpm --filter @objectstack/cli exec vitest run --project unit :: exit 0 (382 files, 5384 tests) cli integration: stored-expressions.test.ts :: exit 0 (10 tests) pnpm lint :: exit 0 node scripts/pm/dispatch-gates.mjs --ran :: 96 derived, 96 run, 0 NOT-MEASURED ``` </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (`packages/spec` untouched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the maintainer's Chinese summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. <!-- signed: domain:server#2 · os-bill · session_01Utp6Cb2FL7H75wB1wA6PpZ --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 171b6dd commit cd5f554

14 files changed

Lines changed: 1411 additions & 3 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/objectql': patch
3+
'@objectstack/cli': patch
4+
---
5+
6+
fix(objectql,cli): `os migrate stored-expressions` fills a stored computed field on rows stored before it was declared, and the boot names the command when a sample of rows needs it
7+
8+
Clause-②: yes (widening)
9+
Metadata-protocol: none
10+
11+
A stored computed field (a non-`formula` field declaring `expression`) is computed by the engine on every insert and update and on nothing else, so declaring one on an object that already holds rows left the new column NULL on every existing row until each was next written; a filter, sort, roll-up or analytics anchor over it read NULL where the expression gives a value, with no error. A fresh database was right and an upgraded one was not.
12+
13+
- **`@objectstack/cli`: `os migrate stored-expressions [--object <o>]… [--field <f>] [--changed] [--apply] [--yes] [--json] [--force] [--max-records] [--database-url]`.** A dry run (the default) reports, per field, the rows holding a NULL the expression gives a value; `--apply` writes them. Each value is the write's own computation over the stored row (option labels, the one reference hop, the `can()` map), and the apply writes through `engine.update` with `updated_at` kept, so hooks, roll-ups over the column and audit capture see an ordinary write. A NULL the expression itself answers (an empty reference, a null input) is left alone. A value that differs from the expression — the expression changed after the row was written — is counted on every run and rewritten only with `--changed`; an expression reading the clock always differs there. A row on which an expression does not evaluate while it holds a value is listed and not written, because the write would store NULL over it. The command exits 1 while a row is skipped or could not be written, or when `--max-records` cut the walk short. Idempotent: a re-run finds nothing to fill.
14+
- **Boot advisory.** At `kernel:bootstrapped` the engine plugin reads the first 100 rows of each object declaring a stored computed field (nothing for an object declaring none) and, when that sample holds such a NULL, warns once naming each `object.field` and `os migrate stored-expressions`. It writes nothing and never fails a boot.
15+
- **`@objectstack/objectql`:** `backfillStoredExpressions`, `formatStoredExpressionBackfillReport`, `storedExpressionBackfillComplete`, `announceStoredExpressionHoles`, `sampleStoredExpressionHoles`, and `ObjectQL.computeStoredExpressionsOver(object, rows, context?, onFault?)` — a stored computed field's values over rows already stored, computed as a write computes them.

‎content/docs/deployment/cli.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,6 +1303,7 @@ where the data lives.
13031303
| `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 |
13041304
| `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 |
13051305
| `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 |
1306+
| `os migrate stored-expressions` | Fill a stored computed field (a non-formula field declaring `expression`) on the rows stored before the field was declared, which hold `NULL` until each is next written; the boot warns, naming the field, when a sample of rows holds such a `NULL`. `--changed` also recomputes values an edited expression no longer gives. Rows an expression cannot compute while they hold a value are listed, never written. No flag; idempotent |
13061307
| `os migrate blank-single-values` | Rewrite the empty strings (`''`) still stored on optional single-value option, reference and file columns to `null`, the form a cleared value is stored in today, so `$empty` finds those rows. Repairs values; no flag, nothing depends on it having run. See [below](#os-migrate-blank-single-values) |
13071308
| `os migrate option-codes` | Rewrite option values still stored as a number, a boolean or SQLite `"N.0"` text (rows written before option fields stored the code string) to that code string, so filters, reads and unique keys see one value. Values it cannot decide are listed, never rewritten. No flag; idempotent. See [below](#os-migrate-option-codes) |
13081309
| `os migrate text-scalars` | Rewrite numbers and booleans still stored raw on text fields (rows written before text fields stored the string) to that string, so filters, reads and unique keys see one value. SQLite `"N.0"` text it cannot decide is listed, never rewritten. No flag; idempotent. See [below](#os-migrate-text-scalars) |
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `os migrate stored-expressions` command shape (#29961).
5+
*
6+
* The backfill itself is proven in `@objectstack/objectql`'s
7+
* `stored-expression-backfill.test.ts`. What is pinned here is what a unit
8+
* test of the walk cannot see: the command is a dry run by default, every
9+
* flag reaches `backfillStoredExpressions` as its option, `--field` needs
10+
* `--object`, a run that leaves a row skipped or unwritten exits 1, and the
11+
* human face closes a dry run with the report's own line. The seams that
12+
* would boot a database are replaced; the command's own parse and control
13+
* flow, and the report's formatter, run for real.
14+
*/
15+
16+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
17+
import { dirname, resolve } from 'node:path';
18+
import { fileURLToPath } from 'node:url';
19+
import { stripVTControlCharacters } from 'node:util';
20+
import MigrateStoredExpressions from './stored-expressions.js';
21+
import { bootSchemaStack } from '../../utils/schema-migrate.js';
22+
import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
23+
import { isExitSignal } from '../../utils/format.js';
24+
import { backfillStoredExpressions } from '@objectstack/objectql';
25+
26+
vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: vi.fn() }));
27+
vi.mock('../../utils/migrate-occupancy-gate.js', () => ({
28+
OCCUPANCY_HINT: 'occupancy hint',
29+
probeMigrationTarget: vi.fn(),
30+
}));
31+
vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) }));
32+
// The walk is replaced; the formatter is the real one.
33+
vi.mock('@objectstack/objectql', async (importOriginal) => {
34+
const real = await importOriginal<typeof import('@objectstack/objectql')>();
35+
return {
36+
backfillStoredExpressions: vi.fn(),
37+
formatStoredExpressionBackfillReport: real.formatStoredExpressionBackfillReport,
38+
};
39+
});
40+
41+
const HERE = dirname(fileURLToPath(import.meta.url));
42+
const CLI_ROOT = resolve(HERE, '..', '..', '..');
43+
/** oclif builds its whole command table on the first `run()` in a process. */
44+
const RUN_TIMEOUT = 60_000;
45+
46+
/** The engine surface the command checks before it runs, and one loaded app object. */
47+
const engine = {
48+
computeStoredExpressionsOver: async () => [],
49+
getConfigs: () => ({ contract: {}, sys_user: {} }),
50+
};
51+
52+
const EMPTY_REPORT = {
53+
scannedObjects: [], scannedRecords: 0, fields: [], nullRows: 0, staleRows: 0, faultedRows: 0,
54+
skippedRows: 0, writtenRows: 0, changed: false, applied: false, truncated: false,
55+
unreadableObjects: [], faults: [], faultsTruncated: false, failures: [],
56+
};
57+
58+
/** A dry run that found the hotclm shape: one field NULL on every row. */
59+
const DRY_REPORT_WITH_HOLES = {
60+
...EMPTY_REPORT,
61+
scannedObjects: ['contract'],
62+
scannedRecords: 2,
63+
fields: [{ object: 'contract', field: 'review_sla_days_offset', nullRows: 2, staleRows: 0, faultedRows: 0, written: 0, sampleRecordIds: ['c1', 'c2'] }],
64+
nullRows: 2,
65+
};
66+
67+
let stdout: ReturnType<typeof vi.spyOn>;
68+
let log: ReturnType<typeof vi.spyOn>;
69+
beforeEach(() => {
70+
vi.mocked(probeMigrationTarget).mockResolvedValue({ status: 'free' } as any);
71+
vi.mocked(bootSchemaStack).mockClear();
72+
vi.mocked(bootSchemaStack).mockResolvedValue({
73+
kernel: { getService: () => engine },
74+
dbLabel: 'postgres://db.example.com/app',
75+
driver: { config: { client: 'pg' } },
76+
shutdown: vi.fn(async () => {}),
77+
} as any);
78+
vi.mocked(backfillStoredExpressions).mockReset();
79+
vi.mocked(backfillStoredExpressions).mockResolvedValue(EMPTY_REPORT as any);
80+
// `emitJson` awaits the write's DRAIN callback, so the double has to invoke it.
81+
stdout = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, enc?: unknown, cb?: unknown) => {
82+
const done = typeof enc === 'function' ? enc : cb;
83+
if (typeof done === 'function') done();
84+
return true;
85+
}) as typeof process.stdout.write);
86+
log = vi.spyOn(console, 'log').mockImplementation(() => {});
87+
});
88+
afterEach(() => {
89+
stdout.mockRestore();
90+
log.mockRestore();
91+
});
92+
93+
const optionsHandedToBackfill = () => {
94+
expect(vi.mocked(backfillStoredExpressions)).toHaveBeenCalledTimes(1);
95+
return vi.mocked(backfillStoredExpressions).mock.calls[0][2]!;
96+
};
97+
const exitCodeOf = (err: unknown) => (isExitSignal(err) ? (err as { oclif?: { exit?: number } }).oclif?.exit : undefined);
98+
99+
describe('os migrate stored-expressions', () => {
100+
it('is a dry run by default, and writing needs --yes', () => {
101+
expect(MigrateStoredExpressions.flags.apply.default).toBe(false);
102+
expect(MigrateStoredExpressions.flags.yes.default).toBe(false);
103+
expect(MigrateStoredExpressions.flags.changed.default).toBe(false);
104+
});
105+
106+
it('a bare run walks every object, NULL holes only, without writing', async () => {
107+
await MigrateStoredExpressions.run(['--json'], { root: CLI_ROOT });
108+
expect(optionsHandedToBackfill()).toEqual({
109+
apply: false, objects: undefined, field: undefined, changed: false, maxRecordsPerObject: undefined,
110+
});
111+
}, RUN_TIMEOUT);
112+
113+
it('hands --object, --field, --changed, --max-records and --apply --yes through', async () => {
114+
await MigrateStoredExpressions.run(
115+
['--json', '--object', 'contract', '--field', 'review_sla_days_offset', '--changed', '--max-records', '50', '--apply', '--yes'],
116+
{ root: CLI_ROOT },
117+
);
118+
expect(optionsHandedToBackfill()).toEqual({
119+
apply: true, objects: ['contract'], field: 'review_sla_days_offset', changed: true, maxRecordsPerObject: 50,
120+
});
121+
}, RUN_TIMEOUT);
122+
123+
it('refuses --field without --object at parse time', async () => {
124+
const err = await MigrateStoredExpressions.run(['--json', '--field', 'x'], { root: CLI_ROOT }).catch((e: unknown) => e);
125+
expect(String((err as Error)?.message)).toMatch(/--object/);
126+
expect(vi.mocked(backfillStoredExpressions)).not.toHaveBeenCalled();
127+
}, RUN_TIMEOUT);
128+
129+
it('--apply without --yes in a non-interactive run writes nothing and exits 1', async () => {
130+
const err = await MigrateStoredExpressions.run(['--json', '--apply'], { root: CLI_ROOT }).catch((e: unknown) => e);
131+
expect(exitCodeOf(err)).toBe(1);
132+
expect(vi.mocked(bootSchemaStack)).not.toHaveBeenCalled();
133+
expect(vi.mocked(backfillStoredExpressions)).not.toHaveBeenCalled();
134+
}, RUN_TIMEOUT);
135+
136+
it('an object the deployment does not declare is refused before the walk', async () => {
137+
const err = await MigrateStoredExpressions.run(['--json', '--object', 'nope'], { root: CLI_ROOT }).catch((e: unknown) => e);
138+
expect(exitCodeOf(err)).toBe(1);
139+
expect(vi.mocked(backfillStoredExpressions)).not.toHaveBeenCalled();
140+
const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join(''));
141+
expect(payload).toMatchObject({ code: 'OBJECT_NOT_FOUND' });
142+
}, RUN_TIMEOUT);
143+
144+
it.each([
145+
['a row skipped because a faulting field holds a value', { skippedRows: 1 }],
146+
['a row the write refused', { failures: [{ object: 'contract', recordId: 'c1', error: 'refused' }] }],
147+
['a walk --max-records cut short', { truncated: true }],
148+
])('exits 1 on %s', async (_label, extra) => {
149+
vi.mocked(backfillStoredExpressions).mockResolvedValue({ ...EMPTY_REPORT, ...extra } as any);
150+
const err = await MigrateStoredExpressions.run(['--json'], { root: CLI_ROOT }).catch((e: unknown) => e);
151+
expect(exitCodeOf(err)).toBe(1);
152+
}, RUN_TIMEOUT);
153+
154+
it('a dry run that finds holes exits 0 and closes with the report\'s dry-run line', async () => {
155+
vi.mocked(backfillStoredExpressions).mockResolvedValue(DRY_REPORT_WITH_HOLES as any);
156+
await MigrateStoredExpressions.run([], { root: CLI_ROOT });
157+
const printed = log.mock.calls.map((c: unknown[]) => stripVTControlCharacters(String(c[0]))).join('\n');
158+
expect(printed).toContain('contract.review_sla_days_offset — 2 NULL row(s)');
159+
expect(printed).toContain('Dry run — no row or table was written. Re-run with --apply to write.');
160+
}, RUN_TIMEOUT);
161+
});

0 commit comments

Comments
 (0)