Skip to content

Commit 9e01213

Browse files
authored
fix(cli,driver-sql): os migrate plan lists the datetime storage convergence (#3954) (#3964)
The datetime canonicalisation (#3912/#3942) added two steps to initObjects' physical path: a row-rewriting backfill on SQLite and a TIMESTAMP → DATETIME(3) column rebuild on MySQL. Both already respected the DDL deferral (#3917), so plan performed neither and apply performed both — the behaviour was never wrong. The reporting was. PendingSchemaWork.kind could only express create_table / add_columns, so neither step could appear in the plan. An operator saw two added columns, confirmed, and apply additionally rewrote every row of a datetime column — or took a metadata lock to rebuild one on a large table. The plan promises to show what apply will do, so anything added to initObjects' physical path has to be representable there; the type now says so. - Two new kinds, normalize_datetime_storage and widen_datetime_columns, plus an optional `rows` carrying how much data the step touches: row-writes for the backfill, the table's size for the rebuild, because ALTER … MODIFY is a full rebuild holding a metadata lock and that is the number deciding "now" versus "in a maintenance window". - previewDeferredSchemaWork measures both without performing either. Each probe reuses the exact predicate its migration uses — the backfill's whole WHERE, the widening's own information_schema filter, extracted to legacyMysqlTimestampColumns and shared — so the plan and the apply cannot name different sets. A probe that cannot run is swallowed to an unlisted step, never to a failed plan. - The CLI renders them under their own heading rather than folding them into the additive section, whose "created when you apply" framing carries an implicit promise that the work is never data-losing. summarizePendingSchemaWork never omits in-place work, and is unchanged when there is none. The documented sample output in deployment/cli.mdx gains the section too. Verified on SQLite and against a real MariaDB 10.11: plan lists the columns and row count without touching the schema, apply performs exactly that, re-plan is empty. Closes #3954
1 parent c1dcacd commit 9e01213

9 files changed

Lines changed: 601 additions & 43 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(cli,driver-sql): `os migrate plan` lists the datetime storage convergence (#3954)
7+
8+
The datetime canonicalisation (#3912/#3942) added two steps to `initObjects`'
9+
physical path: a row-rewriting backfill on SQLite and a `TIMESTAMP`
10+
`DATETIME(3)` column rebuild on MySQL. Both already respected the DDL deferral,
11+
so `plan` performed neither and `apply` performed both — the behaviour was never
12+
wrong. The reporting was.
13+
14+
`PendingSchemaWork` could only express `create_table` / `add_columns`, so an
15+
operator saw a plan listing two added columns, confirmed it, and `apply`
16+
additionally rewrote every row of a datetime column — or took a metadata lock to
17+
rebuild one on a large table. The plan promises to show what apply will do.
18+
19+
- `PendingSchemaWork.kind` gains `normalize_datetime_storage` and
20+
`widen_datetime_columns`, plus an optional `rows` carrying how much data the
21+
step touches: row-writes for the backfill, the table's size for the rebuild —
22+
the number that decides "now" versus "in a maintenance window".
23+
- `previewDeferredSchemaWork()` measures both without performing either, reusing
24+
the exact predicate each migration uses (the backfill's whole `WHERE`, the
25+
widening's own `information_schema` filter) so the plan and the apply cannot
26+
name different sets. A probe that cannot run is swallowed to "unlisted", never
27+
to a failed plan.
28+
- The CLI renders them under their own heading rather than folding them into the
29+
additive section, whose "created when you apply" framing carries an implicit
30+
promise that the work is never data-losing. `summarizePendingSchemaWork` — the
31+
line read just before typing `y` — never omits in-place work.

content/docs/deployment/cli.mdx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,27 @@ performed. So `plan` really is a dry run, and everything `apply` is about to do
511511
+ crm_quote [create_table, 9 column(s)]
512512
+ crm_contact [add_columns: nickname, region]
513513
514+
In place (existing rows converged when you apply)
515+
~ crm_contact [normalize_datetime_storage: signed_at — 1,240 row update(s)]
516+
514517
Safe (loosening — applied without --allow-destructive)
515518
✓ crm_contact.email [relax_not_null]
516519
```
517520

518521
Answering `n` leaves the database exactly as it was.
519522

523+
The two upper sections differ in a way worth reading carefully. **New** is
524+
purely additive — it creates tables and columns and never touches a row. **In
525+
place** rewrites existing data: the storage-form convergence a `Field.datetime`
526+
column needs when the database predates the canonical UTC storage (ADR-0053
527+
addendum D-B1..D-B4). It carries a row count because that is the number
528+
deciding whether to run it now; on MySQL it reads `widen_datetime_columns` and
529+
is an `ALTER … MODIFY` table rebuild that holds a metadata lock for its
530+
duration.
531+
532+
Both are safe to apply — the convergence preserves every stored instant and is
533+
idempotent — but only the second takes time proportional to your data.
534+
520535
#### Occupancy check (SQLite)
521536

522537
A running `os dev` / `os serve` holding the same SQLite file open is the usual
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #3954 — how `os migrate plan` renders the pending work.
5+
*
6+
* The additive section carries an implicit promise: what it lists is created,
7+
* never data-losing, and needs no `--allow-destructive` thought. The datetime
8+
* convergence (#3912/#3942) rewrites rows and rebuilds columns, so folding it
9+
* into that section would quietly extend the promise to cover a table rewrite.
10+
* These tests pin the split, and pin that the summary line — the one an operator
11+
* reads before typing "yes" — never omits in-place work.
12+
*/
13+
14+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
15+
import {
16+
renderPendingSchemaWork,
17+
summarizePendingSchemaWork,
18+
type PendingSchemaWork,
19+
} from './schema-migrate.js';
20+
21+
let lines: string[];
22+
23+
beforeEach(() => {
24+
lines = [];
25+
vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => {
26+
lines.push(args.map(String).join(' '));
27+
});
28+
});
29+
30+
afterEach(() => {
31+
vi.restoreAllMocks();
32+
});
33+
34+
const out = () => lines.join('\n');
35+
36+
const ADDITIVE: PendingSchemaWork[] = [
37+
{ table: 'widgets', kind: 'create_table', columns: ['sku', 'qty'] },
38+
{ table: 'orders', kind: 'add_columns', columns: ['note'] },
39+
];
40+
41+
const IN_PLACE: PendingSchemaWork[] = [
42+
{ table: 'evt', kind: 'normalize_datetime_storage', columns: ['at'], rows: 1234567 },
43+
{ table: 'legacy', kind: 'widen_datetime_columns', columns: ['at', 'created_at'], rows: 42 },
44+
];
45+
46+
describe('renderPendingSchemaWork (#3954)', () => {
47+
it('renders nothing at all when there is nothing pending', () => {
48+
renderPendingSchemaWork([]);
49+
expect(out()).toBe('');
50+
});
51+
52+
it('keeps the additive section exactly as it was when only additive work is pending', () => {
53+
renderPendingSchemaWork(ADDITIVE);
54+
expect(out()).toContain('New (additive — created when you apply)');
55+
expect(out()).toContain('widgets');
56+
expect(out()).toContain('[create_table, 2 column(s)]');
57+
expect(out()).toContain('[add_columns: note]');
58+
// No second heading appears when there is no in-place work.
59+
expect(out()).not.toContain('In place');
60+
});
61+
62+
it('puts the datetime convergence under its OWN heading, not the additive one', () => {
63+
renderPendingSchemaWork(IN_PLACE);
64+
expect(out()).toContain('In place (existing rows converged when you apply)');
65+
// The additive heading claims the work is never data-losing; a row rewrite
66+
// must never be listed beneath it.
67+
expect(out()).not.toContain('New (additive');
68+
});
69+
70+
it('names the columns and the size of each in-place step', () => {
71+
renderPendingSchemaWork(IN_PLACE);
72+
expect(out()).toContain('normalize_datetime_storage: at');
73+
expect(out()).toContain('1,234,567 row update(s)');
74+
expect(out()).toContain('widen_datetime_columns: at, created_at');
75+
// A MySQL widen is ALTER … MODIFY — a rebuild, said outright.
76+
expect(out()).toContain('42 row table rebuild');
77+
});
78+
79+
it('shows both sections when both kinds are pending', () => {
80+
renderPendingSchemaWork([...ADDITIVE, ...IN_PLACE]);
81+
expect(out()).toContain('New (additive — created when you apply)');
82+
expect(out()).toContain('In place (existing rows converged when you apply)');
83+
});
84+
85+
it('reads an unmeasured count as unknown rather than zero', () => {
86+
renderPendingSchemaWork([{ table: 'evt', kind: 'normalize_datetime_storage', columns: ['at'] }]);
87+
expect(out()).toContain('? row update(s)');
88+
expect(out()).not.toContain('0 row update(s)');
89+
});
90+
});
91+
92+
describe('summarizePendingSchemaWork (#3954)', () => {
93+
it('is unchanged for purely additive work', () => {
94+
expect(summarizePendingSchemaWork(ADDITIVE)).toBe('1 table(s) to create, 1 column(s) to add');
95+
});
96+
97+
it('is unchanged when nothing is pending', () => {
98+
expect(summarizePendingSchemaWork([])).toBe('0 table(s) to create, 0 column(s) to add');
99+
});
100+
101+
it('never omits in-place work — this is the line read before confirming', () => {
102+
const summary = summarizePendingSchemaWork([...ADDITIVE, ...IN_PLACE]);
103+
expect(summary).toContain('1 table(s) to create');
104+
expect(summary).toContain('1 column(s) to add');
105+
expect(summary).toContain('3 datetime column(s) to converge in place');
106+
expect(summary).toContain('~1,234,609 rows');
107+
});
108+
});

packages/cli/src/utils/schema-migrate.ts

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515
import chalk from 'chalk';
1616
import type { ManagedDriftEntry, DriftCategory, PendingSchemaWork } from '@objectstack/driver-sql';
17+
import { isInPlaceSchemaWork } from '@objectstack/driver-sql';
1718
import { describeDriverConnection } from './connection-display.js';
1819

1920
export type { PendingSchemaWork };
@@ -251,29 +252,75 @@ export function summarize(drift: ManagedDriftEntry[]): string {
251252
}
252253

253254
/**
254-
* Render the additive work the boot sync was held back from doing (#3917).
255+
* Render the work the boot sync was held back from doing (#3917), in two
256+
* sections split by whether it touches existing data (#3954).
255257
*
256-
* Deliberately its own section rather than a `DriftCategory`: this is not
257-
* divergence between metadata and an existing column — it is the create/add
258-
* that used to happen silently at boot, now shown before it runs. Purely
259-
* additive and never data-losing, so it carries no `--allow-destructive` gate.
258+
* Deliberately its own block rather than a `DriftCategory`: this is not
259+
* divergence between metadata and an existing column — it is what used to
260+
* happen silently at boot, now shown before it runs.
261+
*
262+
* The split matters. The additive section tells the operator the work is never
263+
* data-losing, and that promise must not quietly come to cover the datetime
264+
* convergence, which rewrites rows (SQLite) or rebuilds a column (MySQL). Those
265+
* get their own heading, and their row counts, because "how long will this hold
266+
* the table" is the question they raise and the additive kinds do not.
260267
*/
261268
export function renderPendingSchemaWork(pending: PendingSchemaWork[]): void {
262269
if (pending.length === 0) return;
263-
console.log(` ${chalk.bold('New (additive — created when you apply)')}`);
264-
for (const p of pending) {
265-
const detail = p.kind === 'create_table'
266-
? `[create_table, ${p.columns.length} column(s)]`
267-
: `[add_columns: ${p.columns.join(', ')}]`;
268-
console.log(` ${chalk.cyan('+')} ${chalk.cyan(p.table)} ${chalk.dim(detail)}`);
270+
271+
const additive = pending.filter((p) => !isInPlaceSchemaWork(p.kind));
272+
const inPlace = pending.filter((p) => isInPlaceSchemaWork(p.kind));
273+
274+
if (additive.length > 0) {
275+
console.log(` ${chalk.bold('New (additive — created when you apply)')}`);
276+
for (const p of additive) {
277+
const detail = p.kind === 'create_table'
278+
? `[create_table, ${p.columns.length} column(s)]`
279+
: `[add_columns: ${p.columns.join(', ')}]`;
280+
console.log(` ${chalk.cyan('+')} ${chalk.cyan(p.table)} ${chalk.dim(detail)}`);
281+
}
282+
console.log('');
283+
}
284+
285+
if (inPlace.length > 0) {
286+
console.log(` ${chalk.bold('In place (existing rows converged when you apply)')}`);
287+
for (const p of inPlace) {
288+
const label = p.kind === 'normalize_datetime_storage'
289+
? 'normalize_datetime_storage'
290+
: 'widen_datetime_columns';
291+
// A MySQL widen is `ALTER … MODIFY`, i.e. a full table rebuild holding a
292+
// metadata lock — worth saying outright, not just implying via the count.
293+
const cost = p.kind === 'widen_datetime_columns'
294+
? `${formatRows(p.rows)} row table rebuild`
295+
: `${formatRows(p.rows)} row update(s)`;
296+
console.log(
297+
` ${chalk.yellow('~')} ${chalk.yellow(p.table)} ` +
298+
`${chalk.dim(`[${label}: ${p.columns.join(', ')}${cost}]`)}`,
299+
);
300+
}
301+
console.log('');
269302
}
270-
console.log('');
303+
}
304+
305+
/** `rows` is optional on the type; an unmeasured count reads as unknown, not zero. */
306+
function formatRows(rows: number | undefined): string {
307+
return rows === undefined ? '?' : rows.toLocaleString('en-US');
271308
}
272309

273310
export function summarizePendingSchemaWork(pending: PendingSchemaWork[]): string {
274311
const creates = pending.filter((p) => p.kind === 'create_table').length;
275312
const columns = pending
276313
.filter((p) => p.kind === 'add_columns')
277314
.reduce((n, p) => n + p.columns.length, 0);
278-
return `${creates} table(s) to create, ${columns} column(s) to add`;
315+
const parts = [`${creates} table(s) to create`, `${columns} column(s) to add`];
316+
317+
// Only mentioned when there is some, so the common in-sync summary is
318+
// unchanged — but never omitted when there is, which is the #3954 point.
319+
const inPlace = pending.filter((p) => isInPlaceSchemaWork(p.kind));
320+
if (inPlace.length > 0) {
321+
const cols = inPlace.reduce((n, p) => n + p.columns.length, 0);
322+
const rows = inPlace.reduce((n, p) => n + (p.rows ?? 0), 0);
323+
parts.push(`${cols} datetime column(s) to converge in place (~${formatRows(rows)} rows)`);
324+
}
325+
return parts.join(', ');
279326
}

packages/plugins/driver-sql/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export {
2121
diffManagedIndexes,
2222
expectedIndexes,
2323
isIndexDriftOp,
24+
isInPlaceSchemaWork,
2425
isManagedIndexName,
2526
legacyUniqueIndexNames,
2627
legacyUniqueReplacements,
@@ -38,6 +39,7 @@ export type {
3839
ExpectedIndex,
3940
LegacyUniqueReplacement,
4041
PendingSchemaWork,
42+
PendingSchemaWorkKind,
4143
FieldDef as DriftFieldDef,
4244
} from './schema-drift.js';
4345

packages/plugins/driver-sql/src/schema-drift.ts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,60 @@ export type DriftOp =
9191
};
9292

9393
/**
94-
* Physical schema work the *additive* boot sync is holding back (#3917).
94+
* Physical work the boot sync is holding back (#3917).
9595
*
9696
* Distinct from {@link DriftOp}: drift is divergence between metadata and an
9797
* EXISTING column/index that only a deliberate reconcile may resolve, whereas
98-
* this is the create-table / add-column work `initObjects` performs on its own
99-
* — captured rather than executed while the driver runs with DDL deferred, so
100-
* `os migrate plan` can show it and `os migrate apply` can gate it behind the
101-
* confirmation prompt.
98+
* this is the work `initObjects` performs on its own — captured rather than
99+
* executed while the driver runs with DDL deferred, so `os migrate plan` can
100+
* show it and `os migrate apply` can gate it behind the confirmation prompt.
101+
*
102+
* The plan's promise is that it shows what `apply` will do, so **anything added
103+
* to `initObjects`' physical path has to be representable here** — otherwise an
104+
* operator confirms a two-column plan and `apply` additionally rewrites a table.
105+
* That is the gap #3954 closed for the datetime convergence; keep it closed.
102106
*/
103107
export interface PendingSchemaWork {
104108
table: string;
105-
kind: 'create_table' | 'add_columns';
106-
/** Declared columns for a create; the missing ones for an add. */
109+
kind: PendingSchemaWorkKind;
110+
/**
111+
* Declared columns for a create; the missing ones for an add; the columns
112+
* being converged for the two datetime steps.
113+
*/
107114
columns: string[];
115+
/**
116+
* How much data the step touches, when that is knowable up front and worth
117+
* knowing — absent for the additive kinds, which touch none.
118+
*
119+
* For `normalize_datetime_storage` it is the number of row-writes (summed
120+
* across `columns`, since each is its own `UPDATE`). For
121+
* `widen_datetime_columns` it is the table's row count, because MySQL's
122+
* `ALTER … MODIFY` is a full rebuild holding a metadata lock — which is the
123+
* number that decides "now" versus "in a maintenance window".
124+
*/
125+
rows?: number;
126+
}
127+
128+
/**
129+
* What kind of physical work a {@link PendingSchemaWork} entry represents.
130+
*
131+
* The first two are purely additive and never touch existing rows. The datetime
132+
* pair is NOT: `normalize_datetime_storage` rewrites rows in place (the SQLite
133+
* canonical-UTC backfill) and `widen_datetime_columns` rebuilds a column (the
134+
* MySQL `TIMESTAMP` → `DATETIME(3)` widening) — both from #3912/#3942. They are
135+
* rendered under their own heading for that reason: the additive section tells
136+
* the operator the work is never data-losing, and that claim must not silently
137+
* come to cover a row rewrite.
138+
*/
139+
export type PendingSchemaWorkKind =
140+
| 'create_table'
141+
| 'add_columns'
142+
| 'normalize_datetime_storage'
143+
| 'widen_datetime_columns';
144+
145+
/** True for the kinds that rewrite or rebuild existing data rather than adding to it. */
146+
export function isInPlaceSchemaWork(kind: PendingSchemaWorkKind): boolean {
147+
return kind === 'normalize_datetime_storage' || kind === 'widen_datetime_columns';
108148
}
109149

110150
/** Ops that act on an index rather than a column — reconciled without a table rebuild. */

0 commit comments

Comments
 (0)