diff --git a/.changeset/migrate-plan-lists-datetime-convergence.md b/.changeset/migrate-plan-lists-datetime-convergence.md new file mode 100644 index 0000000000..fef0e8d7d9 --- /dev/null +++ b/.changeset/migrate-plan-lists-datetime-convergence.md @@ -0,0 +1,31 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/cli": patch +--- + +fix(cli,driver-sql): `os migrate plan` lists the datetime storage convergence (#3954) + +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, +so `plan` performed neither and `apply` performed both — the behaviour was never +wrong. The reporting was. + +`PendingSchemaWork` could only express `create_table` / `add_columns`, so an +operator saw a plan listing two added columns, confirmed it, 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. + +- `PendingSchemaWork.kind` gains `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 — + the number that decides "now" versus "in a maintenance window". +- `previewDeferredSchemaWork()` measures both without performing either, reusing + the exact predicate each migration uses (the backfill's whole `WHERE`, the + widening's own `information_schema` filter) so the plan and the apply cannot + name different sets. A probe that cannot run is swallowed to "unlisted", 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` — the + line read just before typing `y` — never omits in-place work. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 2b54421c19..b0e5156a76 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -511,12 +511,27 @@ performed. So `plan` really is a dry run, and everything `apply` is about to do + crm_quote [create_table, 9 column(s)] + crm_contact [add_columns: nickname, region] + In place (existing rows converged when you apply) + ~ crm_contact [normalize_datetime_storage: signed_at — 1,240 row update(s)] + Safe (loosening — applied without --allow-destructive) ✓ crm_contact.email [relax_not_null] ``` Answering `n` leaves the database exactly as it was. +The two upper sections differ in a way worth reading carefully. **New** is +purely additive — it creates tables and columns and never touches a row. **In +place** rewrites existing data: the storage-form convergence a `Field.datetime` +column needs when the database predates the canonical UTC storage (ADR-0053 +addendum D-B1..D-B4). It carries a row count because that is the number +deciding whether to run it now; on MySQL it reads `widen_datetime_columns` and +is an `ALTER … MODIFY` table rebuild that holds a metadata lock for its +duration. + +Both are safe to apply — the convergence preserves every stored instant and is +idempotent — but only the second takes time proportional to your data. + #### Occupancy check (SQLite) A running `os dev` / `os serve` holding the same SQLite file open is the usual diff --git a/packages/cli/src/utils/schema-migrate.pending-render.test.ts b/packages/cli/src/utils/schema-migrate.pending-render.test.ts new file mode 100644 index 0000000000..bf992e4b5f --- /dev/null +++ b/packages/cli/src/utils/schema-migrate.pending-render.test.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3954 — how `os migrate plan` renders the pending work. + * + * The additive section carries an implicit promise: what it lists is created, + * never data-losing, and needs no `--allow-destructive` thought. The datetime + * convergence (#3912/#3942) rewrites rows and rebuilds columns, so folding it + * into that section would quietly extend the promise to cover a table rewrite. + * These tests pin the split, and pin that the summary line — the one an operator + * reads before typing "yes" — never omits in-place work. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + renderPendingSchemaWork, + summarizePendingSchemaWork, + type PendingSchemaWork, +} from './schema-migrate.js'; + +let lines: string[]; + +beforeEach(() => { + lines = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + lines.push(args.map(String).join(' ')); + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const out = () => lines.join('\n'); + +const ADDITIVE: PendingSchemaWork[] = [ + { table: 'widgets', kind: 'create_table', columns: ['sku', 'qty'] }, + { table: 'orders', kind: 'add_columns', columns: ['note'] }, +]; + +const IN_PLACE: PendingSchemaWork[] = [ + { table: 'evt', kind: 'normalize_datetime_storage', columns: ['at'], rows: 1234567 }, + { table: 'legacy', kind: 'widen_datetime_columns', columns: ['at', 'created_at'], rows: 42 }, +]; + +describe('renderPendingSchemaWork (#3954)', () => { + it('renders nothing at all when there is nothing pending', () => { + renderPendingSchemaWork([]); + expect(out()).toBe(''); + }); + + it('keeps the additive section exactly as it was when only additive work is pending', () => { + renderPendingSchemaWork(ADDITIVE); + expect(out()).toContain('New (additive — created when you apply)'); + expect(out()).toContain('widgets'); + expect(out()).toContain('[create_table, 2 column(s)]'); + expect(out()).toContain('[add_columns: note]'); + // No second heading appears when there is no in-place work. + expect(out()).not.toContain('In place'); + }); + + it('puts the datetime convergence under its OWN heading, not the additive one', () => { + renderPendingSchemaWork(IN_PLACE); + expect(out()).toContain('In place (existing rows converged when you apply)'); + // The additive heading claims the work is never data-losing; a row rewrite + // must never be listed beneath it. + expect(out()).not.toContain('New (additive'); + }); + + it('names the columns and the size of each in-place step', () => { + renderPendingSchemaWork(IN_PLACE); + expect(out()).toContain('normalize_datetime_storage: at'); + expect(out()).toContain('1,234,567 row update(s)'); + expect(out()).toContain('widen_datetime_columns: at, created_at'); + // A MySQL widen is ALTER … MODIFY — a rebuild, said outright. + expect(out()).toContain('42 row table rebuild'); + }); + + it('shows both sections when both kinds are pending', () => { + renderPendingSchemaWork([...ADDITIVE, ...IN_PLACE]); + expect(out()).toContain('New (additive — created when you apply)'); + expect(out()).toContain('In place (existing rows converged when you apply)'); + }); + + it('reads an unmeasured count as unknown rather than zero', () => { + renderPendingSchemaWork([{ table: 'evt', kind: 'normalize_datetime_storage', columns: ['at'] }]); + expect(out()).toContain('? row update(s)'); + expect(out()).not.toContain('0 row update(s)'); + }); +}); + +describe('summarizePendingSchemaWork (#3954)', () => { + it('is unchanged for purely additive work', () => { + expect(summarizePendingSchemaWork(ADDITIVE)).toBe('1 table(s) to create, 1 column(s) to add'); + }); + + it('is unchanged when nothing is pending', () => { + expect(summarizePendingSchemaWork([])).toBe('0 table(s) to create, 0 column(s) to add'); + }); + + it('never omits in-place work — this is the line read before confirming', () => { + const summary = summarizePendingSchemaWork([...ADDITIVE, ...IN_PLACE]); + expect(summary).toContain('1 table(s) to create'); + expect(summary).toContain('1 column(s) to add'); + expect(summary).toContain('3 datetime column(s) to converge in place'); + expect(summary).toContain('~1,234,609 rows'); + }); +}); diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 4f058e87cf..b967c51152 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -14,6 +14,7 @@ */ import chalk from 'chalk'; import type { ManagedDriftEntry, DriftCategory, PendingSchemaWork } from '@objectstack/driver-sql'; +import { isInPlaceSchemaWork } from '@objectstack/driver-sql'; import { describeDriverConnection } from './connection-display.js'; export type { PendingSchemaWork }; @@ -251,23 +252,59 @@ export function summarize(drift: ManagedDriftEntry[]): string { } /** - * Render the additive work the boot sync was held back from doing (#3917). + * Render the work the boot sync was held back from doing (#3917), in two + * sections split by whether it touches existing data (#3954). * - * Deliberately its own section rather than a `DriftCategory`: this is not - * divergence between metadata and an existing column — it is the create/add - * that used to happen silently at boot, now shown before it runs. Purely - * additive and never data-losing, so it carries no `--allow-destructive` gate. + * Deliberately its own block rather than a `DriftCategory`: this is not + * divergence between metadata and an existing column — it is what used to + * happen silently at boot, now shown before it runs. + * + * The split matters. The additive section tells the operator the work is never + * data-losing, and that promise must not quietly come to cover the datetime + * convergence, which rewrites rows (SQLite) or rebuilds a column (MySQL). Those + * get their own heading, and their row counts, because "how long will this hold + * the table" is the question they raise and the additive kinds do not. */ export function renderPendingSchemaWork(pending: PendingSchemaWork[]): void { if (pending.length === 0) return; - console.log(` ${chalk.bold('New (additive — created when you apply)')}`); - for (const p of pending) { - const detail = p.kind === 'create_table' - ? `[create_table, ${p.columns.length} column(s)]` - : `[add_columns: ${p.columns.join(', ')}]`; - console.log(` ${chalk.cyan('+')} ${chalk.cyan(p.table)} ${chalk.dim(detail)}`); + + const additive = pending.filter((p) => !isInPlaceSchemaWork(p.kind)); + const inPlace = pending.filter((p) => isInPlaceSchemaWork(p.kind)); + + if (additive.length > 0) { + console.log(` ${chalk.bold('New (additive — created when you apply)')}`); + for (const p of additive) { + const detail = p.kind === 'create_table' + ? `[create_table, ${p.columns.length} column(s)]` + : `[add_columns: ${p.columns.join(', ')}]`; + console.log(` ${chalk.cyan('+')} ${chalk.cyan(p.table)} ${chalk.dim(detail)}`); + } + console.log(''); + } + + if (inPlace.length > 0) { + console.log(` ${chalk.bold('In place (existing rows converged when you apply)')}`); + for (const p of inPlace) { + const label = p.kind === 'normalize_datetime_storage' + ? 'normalize_datetime_storage' + : 'widen_datetime_columns'; + // A MySQL widen is `ALTER … MODIFY`, i.e. a full table rebuild holding a + // metadata lock — worth saying outright, not just implying via the count. + const cost = p.kind === 'widen_datetime_columns' + ? `${formatRows(p.rows)} row table rebuild` + : `${formatRows(p.rows)} row update(s)`; + console.log( + ` ${chalk.yellow('~')} ${chalk.yellow(p.table)} ` + + `${chalk.dim(`[${label}: ${p.columns.join(', ')} — ${cost}]`)}`, + ); + } + console.log(''); } - console.log(''); +} + +/** `rows` is optional on the type; an unmeasured count reads as unknown, not zero. */ +function formatRows(rows: number | undefined): string { + return rows === undefined ? '?' : rows.toLocaleString('en-US'); } export function summarizePendingSchemaWork(pending: PendingSchemaWork[]): string { @@ -275,5 +312,15 @@ export function summarizePendingSchemaWork(pending: PendingSchemaWork[]): string const columns = pending .filter((p) => p.kind === 'add_columns') .reduce((n, p) => n + p.columns.length, 0); - return `${creates} table(s) to create, ${columns} column(s) to add`; + const parts = [`${creates} table(s) to create`, `${columns} column(s) to add`]; + + // Only mentioned when there is some, so the common in-sync summary is + // unchanged — but never omitted when there is, which is the #3954 point. + const inPlace = pending.filter((p) => isInPlaceSchemaWork(p.kind)); + if (inPlace.length > 0) { + const cols = inPlace.reduce((n, p) => n + p.columns.length, 0); + const rows = inPlace.reduce((n, p) => n + (p.rows ?? 0), 0); + parts.push(`${cols} datetime column(s) to converge in place (~${formatRows(rows)} rows)`); + } + return parts.join(', '); } diff --git a/packages/plugins/driver-sql/src/index.ts b/packages/plugins/driver-sql/src/index.ts index edb983fff9..964accf2ba 100644 --- a/packages/plugins/driver-sql/src/index.ts +++ b/packages/plugins/driver-sql/src/index.ts @@ -21,6 +21,7 @@ export { diffManagedIndexes, expectedIndexes, isIndexDriftOp, + isInPlaceSchemaWork, isManagedIndexName, legacyUniqueIndexNames, legacyUniqueReplacements, @@ -38,6 +39,7 @@ export type { ExpectedIndex, LegacyUniqueReplacement, PendingSchemaWork, + PendingSchemaWorkKind, FieldDef as DriftFieldDef, } from './schema-drift.js'; diff --git a/packages/plugins/driver-sql/src/schema-drift.ts b/packages/plugins/driver-sql/src/schema-drift.ts index 05cdca7399..f328def2b9 100644 --- a/packages/plugins/driver-sql/src/schema-drift.ts +++ b/packages/plugins/driver-sql/src/schema-drift.ts @@ -91,20 +91,60 @@ export type DriftOp = }; /** - * Physical schema work the *additive* boot sync is holding back (#3917). + * Physical work the boot sync is holding back (#3917). * * Distinct from {@link DriftOp}: drift is divergence between metadata and an * EXISTING column/index that only a deliberate reconcile may resolve, whereas - * this is the create-table / add-column work `initObjects` performs on its own - * — captured rather than executed while the driver runs with DDL deferred, so - * `os migrate plan` can show it and `os migrate apply` can gate it behind the - * confirmation prompt. + * this is the work `initObjects` performs on its own — captured rather than + * executed while the driver runs with DDL deferred, so `os migrate plan` can + * show it and `os migrate apply` can gate it behind the confirmation prompt. + * + * The plan's promise is that it shows what `apply` will do, so **anything added + * to `initObjects`' physical path has to be representable here** — otherwise an + * operator confirms a two-column plan and `apply` additionally rewrites a table. + * That is the gap #3954 closed for the datetime convergence; keep it closed. */ export interface PendingSchemaWork { table: string; - kind: 'create_table' | 'add_columns'; - /** Declared columns for a create; the missing ones for an add. */ + kind: PendingSchemaWorkKind; + /** + * Declared columns for a create; the missing ones for an add; the columns + * being converged for the two datetime steps. + */ columns: string[]; + /** + * How much data the step touches, when that is knowable up front and worth + * knowing — absent for the additive kinds, which touch none. + * + * For `normalize_datetime_storage` it is the number of row-writes (summed + * across `columns`, since each is its own `UPDATE`). For + * `widen_datetime_columns` it is the table's row count, because MySQL's + * `ALTER … MODIFY` is a full rebuild holding a metadata lock — which is the + * number that decides "now" versus "in a maintenance window". + */ + rows?: number; +} + +/** + * What kind of physical work a {@link PendingSchemaWork} entry represents. + * + * The first two are purely additive and never touch existing rows. The datetime + * pair is NOT: `normalize_datetime_storage` rewrites rows in place (the SQLite + * canonical-UTC backfill) and `widen_datetime_columns` rebuilds a column (the + * MySQL `TIMESTAMP` → `DATETIME(3)` widening) — both from #3912/#3942. They are + * rendered under their own heading for that reason: the additive section tells + * the operator the work is never data-losing, and that claim must not silently + * come to cover a row rewrite. + */ +export type PendingSchemaWorkKind = + | 'create_table' + | 'add_columns' + | 'normalize_datetime_storage' + | 'widen_datetime_columns'; + +/** True for the kinds that rewrite or rebuild existing data rather than adding to it. */ +export function isInPlaceSchemaWork(kind: PendingSchemaWorkKind): boolean { + return kind === 'normalize_datetime_storage' || kind === 'widen_datetime_columns'; } /** Ops that act on an index rather than a column — reconciled without a table rebuild. */ diff --git a/packages/plugins/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts b/packages/plugins/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts index 08b4f0a3fd..6c3c6e87d4 100644 --- a/packages/plugins/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts @@ -224,6 +224,71 @@ describe.skipIf(!URL)('MySQL TIMESTAMP → DATETIME(3) migration (#3942)', () => }); }); +describe.skipIf(!URL)('os migrate plan lists the MySQL widening (#3954)', () => { + const LEGACY = 'os3954_legacy'; + const SHAPE = { name: LEGACY, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }; + let driver: SqlDriver; + + beforeEach(async () => { + const legacy = rawDriver(); + await legacy.execute(`drop table if exists ${LEGACY}`); + await legacy.execute( + `create table ${LEGACY} ( + id varchar(255) not null primary key, + created_at timestamp null default current_timestamp, + updated_at timestamp null default current_timestamp, + label varchar(255) null, + at timestamp null + )`, + ); + await legacy.execute(`set time_zone = '+00:00'`); + for (let i = 0; i < 5; i++) { + await legacy.execute(`insert into ${LEGACY} (id, label, at) values (?, ?, ?)`, [ + `e${i}`, `e${i}`, '2026-03-20 12:00:00', + ]); + } + await legacy.disconnect(); + driver = new SqlDriver({ client: 'mysql2', connection: URL }); + }); + + afterEach(async () => { + await driver.execute(`drop table if exists ${LEGACY}`).catch(() => {}); + await driver.disconnect(); + }); + + it('reports the widening, its columns and the table size — without performing it', async () => { + driver.setDeferredDdl(true); + await driver.initObjects([SHAPE]); + + const pending = await driver.previewDeferredSchemaWork(); + const widen = pending.filter((p) => p.kind === 'widen_datetime_columns'); + expect(widen).toHaveLength(1); + expect(widen[0].table).toBe(LEGACY); + // The audit columns are declared datetime too, so all three are rebuilt. + expect([...widen[0].columns].sort()).toEqual(['at', 'created_at', 'updated_at']); + // `ALTER … MODIFY` is a full rebuild holding a metadata lock, so the table's + // size is the number that decides "now" versus "in a maintenance window". + expect(widen[0].rows).toBe(5); + + // Planning must not have touched the schema. + expect((await columnTypes(driver, LEGACY)).at).toBe('timestamp'); + }); + + it('applies exactly what it planned, and then finds nothing left', async () => { + driver.setDeferredDdl(true); + await driver.initObjects([SHAPE]); + const planned = await driver.previewDeferredSchemaWork(); + expect(planned.some((p) => p.kind === 'widen_datetime_columns')).toBe(true); + + await driver.flushDeferredSchemaDdl(); + expect((await columnTypes(driver, LEGACY)).at).toBe('datetime(3)'); + + driver.setDeferredDdl(true); + await driver.initObjects([SHAPE]); + expect(await driver.previewDeferredSchemaWork()).toEqual([]); + }); +}); + // ── helpers ───────────────────────────────────────────────────────────────── /** mysql2 hands back `[rows, fields]`; normalise to just the rows. */ diff --git a/packages/plugins/driver-sql/src/sql-driver-deferred-datetime-convergence.test.ts b/packages/plugins/driver-sql/src/sql-driver-deferred-datetime-convergence.test.ts new file mode 100644 index 0000000000..32fc21da3b --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-deferred-datetime-convergence.test.ts @@ -0,0 +1,161 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3954 — `os migrate plan` must list the datetime storage convergence. + * + * The datetime canonicalisation (#3912/#3942) added two steps to `initObjects`' + * physical path: a row-rewriting backfill on SQLite and a column rebuild on + * MySQL. Both correctly respect the DDL deferral, so `plan` performs neither and + * `apply` performs both — the behaviour was never wrong. + * + * What was wrong is the reporting. `PendingSchemaWork` could only express + * `create_table` / `add_columns`, so an operator saw a plan listing two added + * columns, confirmed it, and `apply` additionally rewrote every row of a + * datetime column. The plan promises to show what apply will do. + * + * These tests pin three things: the step is listed, it is measured, and it is + * still not *performed* by the preview. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; +import { LegacyStorageDriver } from './legacy-datetime-storage.testkit.js'; + +const EVT = { + name: 'evt', + fields: { + label: { type: 'text' }, + at: { type: 'datetime' }, + seen_at: { type: 'datetime' }, + }, +} as any; + +const make = (Ctor: new (cfg: any) => T): T => + new Ctor({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + +/** A database an older build wrote: legacy datetime forms, backfill not yet run. */ +async function seedLegacyDatabase(driver: LegacyStorageDriver) { + await driver.initObjects([EVT]); + await driver.seedLegacyRows('evt', 'at', [ + { id: 'e1', label: 'a', at: Date.parse('2026-03-20T12:00:00.000Z'), seen_at: '2026-03-20T12:00:00.000Z' }, + { id: 'e2', label: 'b', at: '2026-03-20 13:00:00', seen_at: '2026-03-20T13:00:00.000Z' }, + { id: 'e3', label: 'c', at: '2026-03-20T14:00:00+08:00', seen_at: '2026-03-20T06:00:00.000Z' }, + ]); + // `seen_at` was written canonical, so only `at` should be reported. + driver.forgetCanonical('evt', 'seen_at'); +} + +describe('os migrate plan lists the datetime convergence (#3954)', () => { + let driver: LegacyStorageDriver; + + afterEach(async () => { + await driver.disconnect(); + }); + + it('reports the pending backfill, naming the columns and the row count', async () => { + driver = make(LegacyStorageDriver); + await seedLegacyDatabase(driver); + + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + const pending = await driver.previewDeferredSchemaWork(); + + const converge = pending.filter((p) => p.kind === 'normalize_datetime_storage'); + expect(converge).toHaveLength(1); + expect(converge[0].table).toBe('evt'); + // Only the column that actually holds non-canonical rows. + expect(converge[0].columns).toEqual(['at']); + expect(converge[0].rows).toBe(3); + }); + + it('does not PERFORM the backfill while previewing it', async () => { + driver = make(LegacyStorageDriver); + await seedLegacyDatabase(driver); + const before = await driver.storedForms('evt', 'at'); + + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + await driver.previewDeferredSchemaWork(); + + // The whole point of the deferral: plan measures, apply changes. + expect(await driver.storedForms('evt', 'at')).toEqual(before); + }); + + it('flushing the deferral converges the rows the plan named', async () => { + driver = make(LegacyStorageDriver); + await seedLegacyDatabase(driver); + + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + const planned = await driver.previewDeferredSchemaWork(); + expect(planned.some((p) => p.kind === 'normalize_datetime_storage')).toBe(true); + + await driver.flushDeferredSchemaDdl(); + + for (const row of await driver.storedForms('evt', 'at')) { + expect(String(row.value)).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); + } + // And re-planning now finds nothing left to converge. + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + const after = await driver.previewDeferredSchemaWork(); + expect(after.filter((p) => p.kind === 'normalize_datetime_storage')).toEqual([]); + }); + + it('reports nothing for a database that is already canonical', async () => { + driver = make(LegacyStorageDriver); + await driver.initObjects([EVT]); + await driver.create('evt', { id: 'ok', label: 'ok', at: '2026-03-20T12:00:00.000Z' }, { bypassTenantAudit: true }); + + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + expect(await driver.previewDeferredSchemaWork()).toEqual([]); + }); + + it('reports nothing for a table that does not exist yet — it is created empty', async () => { + driver = make(LegacyStorageDriver); + driver.setDeferredDdl(true); + await driver.initObjects([EVT]); + + const pending = await driver.previewDeferredSchemaWork(); + expect(pending.map((p) => p.kind)).toEqual(['create_table']); + }); + + it('reports the convergence ALONGSIDE an add_columns on the same table', async () => { + // The combination the gap was worst in: a plan that lists one added column + // while apply also rewrites the table. + driver = make(LegacyStorageDriver); + await seedLegacyDatabase(driver); + + const widened = { ...EVT, fields: { ...EVT.fields, note: { type: 'text' } } }; + driver.setDeferredDdl(true); + await driver.initObjects([widened]); + const pending = await driver.previewDeferredSchemaWork(); + + expect(pending.map((p) => p.kind).sort()).toEqual(['add_columns', 'normalize_datetime_storage']); + expect(pending.find((p) => p.kind === 'add_columns')!.columns).toEqual(['note']); + }); + + it('survives a probe that cannot run, rather than failing the plan', async () => { + // Same posture as the migrations themselves, which log and swallow: a probe + // that cannot run costs an UNLISTED step, while throwing would cost the whole + // plan — including the create/add entries that have nothing to do with it. + class BrokenProbe extends LegacyStorageDriver { + protected override sqliteCanonicalDatetimeSql(): string { + throw new Error('probe exploded'); + } + } + const broken = make(BrokenProbe); + try { + await seedLegacyDatabase(broken); + broken.setDeferredDdl(true); + await broken.initObjects([{ ...EVT, fields: { ...EVT.fields, note: { type: 'text' } } }]); + + const pending = await broken.previewDeferredSchemaWork(); + expect(pending.map((p) => p.kind)).toEqual(['add_columns']); + } finally { + await broken.disconnect(); + } + driver = make(LegacyStorageDriver); // satisfy afterEach + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 2c8f93d246..2b4e31c4ab 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -2731,6 +2731,43 @@ export class SqlDriver implements IDataDriver { } } + /** + * The `Field.datetime` (and audit) columns of `table` that MySQL still stores + * as a legacy `TIMESTAMP`, with the nullability each must keep. + * + * Shared by {@link migrateMysqlDatetimeColumns}, which widens them, and + * {@link previewDatetimeConvergence}, which reports them into `os migrate + * plan` (#3954) — so what the plan lists and what apply does are the same set + * by construction, not by two filters that agree today. + * + * `[]` on any dialect but MySQL, and on a table declaring no datetime column. + */ + protected async legacyMysqlTimestampColumns( + table: string, + fields: Record, + ): Promise> { + if (!this.isMysql) return []; + const candidates = new Set(AUDIT_TIMESTAMP_COLUMNS); + for (const [name, field] of Object.entries(fields)) { + if ((field?.type ?? 'string') === 'datetime' && !field?.multiple) candidates.add(name); + } + if (candidates.size === 0) return []; + + const res: any = await this.knex.raw( + `select column_name, is_nullable from information_schema.columns + where table_schema = database() and table_name = ? and data_type = 'timestamp'`, + [table], + ); + // mysql2 returns [rows, fields]; column names vary in case by server. + const rows: any[] = Array.isArray(res?.[0]) ? res[0] : (res?.rows ?? res ?? []); + return rows + .map((r) => ({ + name: String(r.COLUMN_NAME ?? r.column_name ?? ''), + nullable: String(r.IS_NULLABLE ?? r.is_nullable ?? 'YES').toUpperCase() !== 'NO', + })) + .filter((c) => c.name && candidates.has(c.name)); + } + /** * Widen a table's legacy MySQL `TIMESTAMP` datetime columns to `DATETIME(3)` * (#3942) — the MySQL counterpart of {@link backfillCanonicalDatetimes}. @@ -2758,26 +2795,8 @@ export class SqlDriver implements IDataDriver { fields: Record, ): Promise { if (!this.isMysql) return; - const candidates = new Set(AUDIT_TIMESTAMP_COLUMNS); - for (const [name, field] of Object.entries(fields)) { - if ((field?.type ?? 'string') === 'datetime' && !field?.multiple) candidates.add(name); - } - if (candidates.size === 0) return; - try { - const res: any = await this.knex.raw( - `select column_name, is_nullable from information_schema.columns - where table_schema = database() and table_name = ? and data_type = 'timestamp'`, - [table], - ); - // mysql2 returns [rows, fields]; column names vary in case by server. - const rows: any[] = Array.isArray(res?.[0]) ? res[0] : (res?.rows ?? res ?? []); - const legacy = rows - .map((r) => ({ - name: String(r.COLUMN_NAME ?? r.column_name ?? ''), - nullable: String(r.IS_NULLABLE ?? r.is_nullable ?? 'YES').toUpperCase() !== 'NO', - })) - .filter((c) => c.name && candidates.has(c.name)); + const legacy = await this.legacyMysqlTimestampColumns(table, fields); if (legacy.length === 0) return; for (const col of legacy) { @@ -2835,15 +2854,23 @@ export class SqlDriver implements IDataDriver { /** * What the deferred sync *would* do, without doing it. * - * Read-only: `hasTable` + `columnInfo`, the same two probes the additive sync - * uses to decide between create and alter. Tables and columns that already - * match metadata produce no entry, so an in-sync database returns `[]`. + * Read-only: `hasTable` + `columnInfo` decide between create and alter (the + * same two probes the additive sync uses), then + * {@link previewDatetimeConvergence} asks whether the datetime storage steps + * have anything left to do. Tables that already match metadata and hold + * canonical data produce no entry, so an in-sync database returns `[]`. + * + * The convergence probe COUNTS rows, so this is more than metadata lookups on + * a database that has not been migrated yet. That is the right trade for a + * command the operator ran to be told the size of the job — and it is paid + * once, by `plan`/`apply`, never on a normal boot. */ async previewDeferredSchemaWork(): Promise { const out: PendingSchemaWork[] = []; for (const [tableName, obj] of this.deferredSchemaObjects) { const declared = Object.keys(obj.fields ?? {}); if (!(await this.knex.schema.hasTable(tableName))) { + // A table that does not exist yet is created empty, so nothing to converge. out.push({ table: tableName, kind: 'create_table', columns: declared }); continue; } @@ -2852,11 +2879,73 @@ export class SqlDriver implements IDataDriver { if (missing.length > 0) { out.push({ table: tableName, kind: 'add_columns', columns: missing }); } + out.push(...(await this.previewDatetimeConvergence(tableName, obj.fields ?? {}, existing))); } - out.sort((a, b) => a.table.localeCompare(b.table)); + out.sort((a, b) => a.table.localeCompare(b.table) || a.kind.localeCompare(b.kind)); return out; } + /** + * The datetime storage-convergence work {@link backfillCanonicalDatetimes} and + * {@link migrateMysqlDatetimeColumns} would do for `table` — measured, not + * performed (#3954). + * + * Both steps run inside `initObjects` alongside the create/alter path, so + * `apply` performs them; without an entry here `plan` would show a two-column + * change and `apply` would additionally rewrite every row of a datetime column + * or rebuild one on a large table. The plan promises to show what apply does. + * + * Each probe reuses the very predicate its migration uses, so the preview + * cannot claim work the migration will not do (or miss work it will): + * - SQLite counts rows matching `col IS NOT ` — the backfill's + * entire `WHERE`. + * - MySQL lists the candidate columns still typed `timestamp` — the + * migration's own `information_schema` filter. + * + * Failures are swallowed to `[]`, matching the migrations themselves: a probe + * that cannot run must not fail the plan, and under-reporting here costs an + * unlisted step rather than a wrong one. + */ + protected async previewDatetimeConvergence( + table: string, + fields: Record, + existingColumns: Set, + ): Promise { + try { + if (this.isSqlite) { + const declared = [...(this.datetimeFields[table] ?? [])].filter((c) => existingColumns.has(c)); + if (declared.length === 0) return []; + const canonical = this.sqliteCanonicalDatetimeSql('??'); + const columns: string[] = []; + let rows = 0; + for (const field of declared) { + const res: any = await this.knex.raw( + `select count(*) as n from ?? where ?? is not null and ?? is not ${canonical}`, + [table, field, field, field, field, field, field], + ); + const n = Number((Array.isArray(res) ? res[0] : res)?.n ?? 0); + if (n > 0) { columns.push(field); rows += n; } + } + return columns.length === 0 + ? [] + : [{ table, kind: 'normalize_datetime_storage', columns, rows }]; + } + + if (this.isMysql) { + const legacy = await this.legacyMysqlTimestampColumns(table, fields); + if (legacy.length === 0) return []; + const res: any = await this.knex.raw(`select count(*) as n from ??`, [table]); + const counted = Array.isArray(res?.[0]) ? res[0] : (res?.rows ?? res ?? []); + const rows = Number((counted[0] as any)?.n ?? (counted as any)?.n ?? 0); + return [{ table, kind: 'widen_datetime_columns', columns: legacy.map((c) => c.name), rows }]; + } + + return []; + } catch { + return []; + } + } + /** * Run the deferred sync and disarm the deferral. Returns the work that was * outstanding (captured before the DDL ran, so the caller can report what it