diff --git a/.changeset/generated-migration-audit-stamp-timestamptz.md b/.changeset/generated-migration-audit-stamp-timestamptz.md new file mode 100644 index 0000000000..8cb6de3857 --- /dev/null +++ b/.changeset/generated-migration-audit-stamp-timestamptz.md @@ -0,0 +1,22 @@ +--- +"@objectstack/cli": patch +--- + +`os generate migration --format sql` gives every timestamp column the time zone the platform actually stores. + +The SQL format spelled its two audit-stamp columns — and every declared `datetime` field — as bare `TIMESTAMP`. In PostgreSQL that is `timestamp WITHOUT time zone`, while both of the other producers of the same columns yield `timestamptz`. This implements **[ADR-0053](../docs/adr/0053-date-and-datetime-semantics.md) D-B4** (accepted), which governs both sites in one sentence: `Field.datetime` maps to `DATETIME(3)` on MySQL while "Postgres deliberately keeps `timestamptz`", and "the builtin `created_at`/`updated_at` take the same type — the registry declares them `Field.datetime`". So the declared-field row and the audit-stamp rows are one decision rather than two judgement calls, and `driver-sql`'s `createAuditTimestampColumn`, its `createColumn` `datetime` arm and this CLI's TypeScript migration format were already implementing it — the SQL format was the one producer that was not. + +Driven, not compiled: all three producers were run against a live PostgreSQL 16.13 and their columns read back out of `information_schema.columns`. Only the SQL format came back zone-naive, and the consequence is a data defect rather than a cosmetic type difference. A zone-naive column stores the wall clock of whatever session wrote the row and keeps nothing to recover the offset from, and `DEFAULT now()` is folded into that session's `TimeZone` on the way in. Two defaulted rows inserted **six milliseconds apart**, one under `TimeZone='UTC'` and one under `Asia/Tokyo`, were recorded **nine hours apart** in the generated table and 3 ms apart in the driver's own: + +``` +sqlgen (timestamp) a_utc 2026-09-05 22:31:28.309421 +sqlgen (timestamp) b_tokyo 2026-09-06 07:31:28.315458 <- +9h, same instant +tsgen (timestamptz) a_utc 2026-09-05 22:31:28.31332+00 +tsgen (timestamptz) b_tokyo 2026-09-05 22:31:28.316401+00 +``` + +The whole temporal class was enumerated in that same run and `datetime` is its only divergent member: `date` is `DATE` and `time` is `TIME` on all three producers, so neither moves. + +Two things this deliberately does not change. The audit columns' **nullability** stays as it is: the driver leaves both nullable and both generators say `NOT NULL`, nothing fails either way, and the driver's own audit DDL is dialect-branched in a way a Postgres-flavoured generated migration does not reproduce — so which side moves is a ruling, recorded in `generate-builtin-id-column.pin.test.ts` and still open. The `DEFAULT now()` spelling stays too: it is the same instant as the driver's `CURRENT_TIMESTAMP` (both are `transaction_timestamp()`) and only reads differently in the catalog. + +Scope for an existing project: already-generated migration files are checked-in artifacts and are not rewritten, and no deployed column is altered — a table created from an older generated migration keeps `timestamp without time zone` until its owner migrates it. What changes is what the next generated migration says. diff --git a/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts b/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts index 744e6b1f9c..1b68fe2a2f 100644 --- a/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts +++ b/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts @@ -44,11 +44,24 @@ * That pin covers the FIELDS; this one covers the builtin column itself, which * is not a vocabulary entry and so had no rule anywhere. * - * ## ⚠️ Recorded divergence, NOT a ruling: the audit-stamp columns (#15040) + * ## The audit-stamp columns: one half ruled (#15521), one half still recorded * - * The last `it` below records — and deliberately does not correct — a THIRD - * disagreement measured in the same pass. It is recorded so it cannot change - * shape unnoticed, and so no reader mistakes this file for having decided it. + * #15040 measured a THIRD disagreement in the same pass and recorded it here + * without correcting it. #15521 split that record in two and ruled only the + * half that was decidable: + * + * TYPE — RULED. The sql format spelled both columns bare `TIMESTAMP` + * (`timestamp WITHOUT time zone`) while the driver and the typescript format + * both build them with knex's `table.timestamp` = `timestamptz`. Driven on a + * live PostgreSQL 16.13: two defaulted rows inserted 6 ms apart under + * different session timezones landed NINE HOURS apart in the zone-naive + * column and 3 ms apart in the aware one. One producer of three was wrong and + * nothing had to be decided, so the sql format moved. + * + * NULLABILITY (and the `now()` / `CURRENT_TIMESTAMP` default spelling) — + * STILL RECORDED, still not ruled: the driver leaves both columns nullable + * and both generators say NOT NULL. Nothing fails either way, so which side + * moves is a ruling #15521 holds open. */ import fs from 'node:fs'; @@ -173,40 +186,79 @@ describe('the builtin id column both migration generators emit (#15040)', () => } }); - // ── Recorded divergence, NOT coverage, and NOT a ruling ────────────────── + // ── #15521, the TYPE half: ruled, and now asserted as agreement ──────── + // + // Bare `TIMESTAMP` is `timestamp WITHOUT time zone`. Both knex producers of + // these same two columns — `driver-sql`'s `createAuditTimestampColumn` and + // `generateMigrationTs`'s `table.timestamps(true, true)` — yield + // `timestamptz`. Read back out of `information_schema.columns` on a live + // PostgreSQL 16.13, all three producers driven: // - // Measured in the same pass as the id column, on the same two generators. - // The audit-stamp columns disagree with the driver too, in a way the id - // column did not, and the disagreement is NULLABILITY rather than type: + // driver created_at timestamp with time zone null=YES default=CURRENT_TIMESTAMP + // ts gen created_at timestamp with time zone null=NO default=CURRENT_TIMESTAMP + // sql gen created_at timestamp without time zone null=NO default=now() // - // driver-sql `table.timestamp(name).defaultTo(knex.fn.now())` (nullable) - // sql gen `"created_at" TIMESTAMP NOT NULL DEFAULT now()` - // ts gen `table.timestamps(true, true)` — knex 3.3.0 compiles this - // to `.notNullable().defaultTo(CURRENT_TIMESTAMP)` on both - // columns (`knex/lib/schema/tablebuilder.js`). + // Asserted as AGREEMENT with the driver rather than as the literal + // `TIMESTAMPTZ` alone: the driver's builder is read where it lives, so the day + // it stops emitting a knex `table.timestamp` this fails here instead of + // leaving the generators quietly wrong again — the same discipline the id + // column above already uses for its width. + it('#15521 — the audit columns take the driver\'s zone-AWARE type in both generators', () => { + const sql = generateMigrationSql(CONFIG as Record); + for (const col of ['created_at', 'updated_at']) { + expect(sql).toContain(`"${col}" TIMESTAMPTZ NOT NULL DEFAULT now()`); + // `\b` discriminates: `TIMESTAMPTZ` is not a match for `TIMESTAMP\b`. + expect( + sql, + `the sql format spells ${col} zone-NAIVE again — a defaulted row then records the ` + + 'wall clock of whatever session wrote it, with nothing left to recover the offset from', + ).not.toMatch(new RegExp(`"${col}" TIMESTAMP\\b`)); + } + // Anti-vacuity: the predicate really does fire on the shape this replaced. + expect(' "created_at" TIMESTAMP NOT NULL DEFAULT now()').toMatch(/"created_at" TIMESTAMP\b/); + // The authority, read where it lives — both knex paths, neither transcribed. + expect( + SQL_DRIVER_SOURCE, + 'driver-sql no longer builds the audit columns with knex\'s `table.timestamp`, which is ' + + 'what makes them `timestamptz` on Postgres. Re-read #15521 before trusting the ' + + 'generators\' TIMESTAMPTZ.', + ).toContain('table.timestamp(name).defaultTo(this.knex.fn.now());'); + expect(generateMigrationTs(CONFIG as Record)).toContain('table.timestamps(true, true);'); + }); + + // ── #15521, the NULLABILITY half: recorded, NOT coverage, and NOT a ruling ── // - // Compiled offline against knex's pg dialect, the two shapes are: + // The driver leaves both columns nullable; both generators say NOT NULL: // - // driver "created_at" timestamptz default CURRENT_TIMESTAMP - // ts gen "created_at" timestamptz not null default CURRENT_TIMESTAMP + // driver-sql `table.timestamp(name).defaultTo(knex.fn.now())` (nullable) + // sql gen `"created_at" TIMESTAMPTZ NOT NULL DEFAULT now()` + // ts gen `table.timestamps(true, true)` — knex 3.3.0 compiles this to + // `.notNullable().defaultTo(CURRENT_TIMESTAMP)` on both columns + // (`knex/lib/schema/tablebuilder.js`), which the live-Postgres + // catalog read above confirms as `null=NO`. // - // Unlike the id column this is not obviously a wrong value: the driver stamps - // both columns on every write, so NOT NULL is arguably the truer constraint — - // and the driver's own DDL is dialect-branched (`datetime(3)` on MySQL, a + // Unlike the type half this is not a wrong value: the driver stamps both + // columns on every write, so NOT NULL is arguably the truer constraint, and + // the driver's own audit DDL is dialect-branched (`datetime(3)` on MySQL, a // canonical ISO default on SQLite) in a way a Postgres-flavoured generated - // migration does not try to reproduce. Which side moves is not this card's to - // decide, so nothing here changes those lines. Asserted only so the - // divergence cannot change shape unnoticed. - it('#15040 record — the audit-stamp columns diverge from the driver, deliberately unresolved', () => { + // migration does not try to reproduce — so "match the driver byte for byte" + // is not even well-defined across dialects. Which side moves is #15521's to + // decide; nothing here changes those lines. + // + // The DEFAULT spelling rides with it: this generator's `now()` and the + // driver's `CURRENT_TIMESTAMP` are the same instant (both are + // `transaction_timestamp()`), but Postgres keeps them textually apart in the + // catalog, so they are two rows of the same schema-diff noise. + it('#15521 record — the audit columns\' NULLABILITY diverges, deliberately unresolved', () => { const sql = generateMigrationSql(CONFIG as Record); - expect(sql).toContain('"created_at" TIMESTAMP NOT NULL DEFAULT now()'); - expect(sql).toContain('"updated_at" TIMESTAMP NOT NULL DEFAULT now()'); + expect(sql).toContain('"created_at" TIMESTAMPTZ NOT NULL DEFAULT now()'); + expect(sql).toContain('"updated_at" TIMESTAMPTZ NOT NULL DEFAULT now()'); expect(generateMigrationTs(CONFIG as Record)).toContain('table.timestamps(true, true);'); // The driver side, read where it lives: one audit-column builder, and its // default arm carries no `.notNullable()`. expect( SQL_DRIVER_SOURCE, - 'driver-sql\'s audit-column DDL moved — re-read the #15040 record above before trusting it.', + 'driver-sql\'s audit-column DDL moved — re-read the #15521 record above before trusting it.', ).toContain('table.timestamp(name).defaultTo(this.knex.fn.now());'); const auditArm = SQL_DRIVER_SOURCE.slice( SQL_DRIVER_SOURCE.indexOf('protected createAuditTimestampColumn('), diff --git a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts index f113cc8a95..cb94081b65 100644 --- a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts +++ b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts @@ -543,7 +543,7 @@ describe('#14828 — the SQL answers are the platform’s, not this file’s inv it('a member typed as a STRING on the record never takes a NUMERIC column', () => { // ⚠️ Deliberately "not numeric" rather than "is a character column". // `date` / `datetime` / `time` are strings on the record and take DATE / - // TIMESTAMP / TIME columns, which is correct — a rule demanding a varchar + // TIMESTAMPTZ / TIME columns, which is correct — a rule demanding a varchar // would report those three as defects. What `autonumber` did is the // narrower thing: a rendered string in a column that only accepts numbers. const NUMERIC_COLUMN = /^(SERIAL|BIGSERIAL|SMALLSERIAL|INTEGER|INT|BIGINT|SMALLINT|DECIMAL|NUMERIC|REAL|FLOAT|DOUBLE)\b/i; @@ -571,6 +571,52 @@ describe('#14828 — the SQL answers are the platform’s, not this file’s inv expect(tsColumn('autonumber')).toBe("table.string('f_autonumber')"); }); + // ── Rule 5: the TEMPORAL class takes the driver's own column types (#15521) ── + // + // The whole class, driven against a live PostgreSQL 16.13 — every producer + // run for real and its columns read back out of `information_schema.columns` + // — and exactly one member diverged: + // + // date DATE driver DATE ts DATE agree + // datetime timestamp WITHOUT time zone driver timestamptz ts timestamptz DIVERGED + // time time without time zone driver time ts time agree + // + // `datetime` was the only bare `TIMESTAMP` in this map, and bare `TIMESTAMP` + // is `timestamp WITHOUT time zone`: the column stores the wall clock of + // whatever session wrote the row and keeps nothing to recover the offset + // from. `createColumn`'s own arm calls the zone-aware column a decision + // rather than a default ("Postgres deliberately keeps `table.timestamp` → + // `timestamptz`"), and this file's typescript generator already emitted + // `table.timestamp` for it — so the SQL map was one producer of three + // disagreeing with the other two, which is what #15521 corrected. + // + // Asserted against the driver's arm rather than against the literal, for the + // reason Rule 3 already gives: a transcribed `TIMESTAMPTZ` would re-create the + // defect one layer up the day the driver's arm moves. + + it('the temporal members take the column type the driver builds for them', () => { + // The driver's non-MySQL arms, read where they live. `table.timestamp` is + // `timestamptz` on Postgres; `table.date` and `table.time` are their plain + // selves, which is why only `datetime` had a time zone to lose. + expect(createColumnArm('datetime')).toContain('table.timestamp(name)'); + expect(createColumnArm('date')).toContain('table.date(name)'); + expect(createColumnArm('time')).toContain('table.time(name)'); + + expect(sqlColumn('datetime')).toBe('TIMESTAMPTZ'); + expect(sqlColumn('date')).toBe('DATE'); + expect(sqlColumn('time')).toBe('TIME'); + + // The third producer agrees by construction — the same knex builders. + expect(tsColumn('datetime')).toBe("table.timestamp('f_datetime')"); + expect(tsColumn('date')).toBe("table.date('f_date')"); + expect(tsColumn('time')).toBe("table.time('f_time')"); + + // Anti-vacuity: the reader really discriminates, and the pre-#15521 + // spelling really is a different answer rather than a formatting variant. + expect(sqlColumn('datetime')).not.toBe('TIMESTAMP'); + expect(sqlColumn('this_is_not_a_field_type')).toBeNull(); + }); + // ── Recorded divergence, NOT coverage: FILE_REFERENCE_TYPES (#15041) ───── // // These five are in driver-sql's `JSON_COLUMN_TYPES` (it spreads the class by diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 06db8cc9cc..b02a8510e1 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -1014,7 +1014,23 @@ const FIELD_TYPE_SQL_MAP: Record = { percent: 'DECIMAL(5,2)', boolean: 'BOOLEAN', date: 'DATE', - datetime: 'TIMESTAMP', + // #15521 — TIMESTAMPTZ, not TIMESTAMP, for the same reason and with the same + // measured consequence as the audit-stamp columns in `generateMigrationSql` + // below: bare `TIMESTAMP` is `timestamp WITHOUT time zone`, while the driver + // creates a declared `Field.datetime` as `table.timestamp(name)` = knex's + // `timestamptz`. `createColumn`'s `datetime` arm states that as a decision, + // not an accident — "Postgres deliberately keeps `table.timestamp` → + // `timestamptz`: asking for precision 3 there would REDUCE it from + // microseconds" — and this file's OWN typescript generator already emitted + // `table.timestamp` for it, so the SQL format was one producer of three + // disagreeing with the other two. Measured on live PostgreSQL 16.13, a + // `datetime` field: driver `timestamp with time zone`, ts format `timestamp + // with time zone`, this map `timestamp without time zone`. + // + // The whole temporal class was enumerated in that same run, and it is the only + // member that diverged: `date` is DATE and `time` is TIME on all three + // producers, so neither moves. + datetime: 'TIMESTAMPTZ', time: 'TIME', email: 'VARCHAR(255)', phone: 'VARCHAR(50)', @@ -1200,8 +1216,40 @@ export function generateMigrationSql(config: Record): string { fieldLines.push(` "${fieldName}" ${sqlType}${notNull}`); } - fieldLines.push(' "created_at" TIMESTAMP NOT NULL DEFAULT now()'); - fieldLines.push(' "updated_at" TIMESTAMP NOT NULL DEFAULT now()'); + // #15521 — TIMESTAMPTZ, not TIMESTAMP. Bare `TIMESTAMP` is `timestamp + // WITHOUT time zone`; both knex producers of these same two columns — + // `driver-sql`'s `createAuditTimestampColumn` and `generateMigrationTs` + // below — yield `timestamptz`. Driven rather than compiled: all three were + // run against a live PostgreSQL 16.13 and the columns read back out of + // `information_schema.columns`, where this literal was the only one that + // came back zone-naive. + // + // driver created_at timestamp with time zone null=YES default=CURRENT_TIMESTAMP + // ts gen created_at timestamp with time zone null=NO default=CURRENT_TIMESTAMP + // sql gen created_at timestamp without time zone null=NO default=now() <- this line + // + // Not a cosmetic type nit. A zone-naive column stores the wall clock of + // whatever session wrote the row and keeps nothing to recover the offset + // from, and `DEFAULT now()` is folded into that session's `TimeZone` on the + // way in. Two defaulted rows inserted SIX MILLISECONDS apart, one under + // `TimeZone='UTC'` and one under `Asia/Tokyo`, were recorded NINE HOURS + // apart in the generated table and 3 ms apart in the driver's own: + // + // sqlgen (timestamp) a_utc 2026-09-05 22:31:28.309421 + // sqlgen (timestamp) b_tokyo 2026-09-06 07:31:28.315458 <- +9h, same instant + // tsgen (timestamptz) a_utc 2026-09-05 22:31:28.31332+00 + // tsgen (timestamptz) b_tokyo 2026-09-05 22:31:28.316401+00 + // + // ⚠️ The NOT NULL half of #15521 is deliberately NOT touched here. The + // driver leaves both columns nullable and both generators say NOT NULL; + // which side moves is a ruling that card holds open, and it is a different + // shape of question — nothing fails either way. `DEFAULT now()` is likewise + // left alone: it is the same instant as the driver's `CURRENT_TIMESTAMP` + // (both are `transaction_timestamp()`), it only reads differently in the + // catalog. `generate-builtin-id-column.pin.test.ts` records both of those, + // still unresolved, beside this half now resolved. + fieldLines.push(' "created_at" TIMESTAMPTZ NOT NULL DEFAULT now()'); + fieldLines.push(' "updated_at" TIMESTAMPTZ NOT NULL DEFAULT now()'); lines.push(fieldLines.join(',\n')); lines.push(');'); lines.push('');