Skip to content

Commit 8a7e9f1

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): canonical ISO-8601-Z for user NOW()-default temporal fields on SQLite (#2346)
A user-declared Field.datetime (or date/time) with defaultValue:'NOW()' took the knex.fn.now() -> CURRENT_TIMESTAMP column default on SQLite, storing a timezone-naive 'YYYY-MM-DD HH:MM:SS' that Date.parse reads as LOCAL time (the same class of bug ADR-0074 fixed for the builtin audit columns, scoped out there for user fields). Worse, the same column mixed storage: an explicit Date is bound by better-sqlite3 as INTEGER epoch ms while an omitted value took the naive TEXT default. - createColumn's NOW() default now emits a per-type canonical via strftime (datetime ISO-8601-Z, date YYYY-MM-DD, time HH:MM:SS.fff) through the new nowColumnDefault helper. - formatOutput folds every Field.datetime storage form (INTEGER epoch ms, canonical ISO-Z, legacy naive TEXT) to one canonical ISO-8601-Z instant (normalizeSqliteDatetimeOutput), repairing legacy rows on read with no data migration. SQLite-only; Postgres/MySQL keep native now() and are unaffected. Follow-up to ADR-0074 (no new ADR), rebased onto it. normalizeSqliteDatetimeOutput reuses ADR-0074's repairNaiveUtcAuditTimestamp for the string shapes and adds the INTEGER epoch-ms / Date folding, so a datetime-typed audit column now also presents as canonical ISO-Z (consistent with every datetime field) — ADR-0074's "datetime-typed created_at reads as epoch-ms number" test is updated accordingly. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e360c9e commit 8a7e9f1

4 files changed

Lines changed: 362 additions & 12 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
---
4+
5+
Fix: canonical storage + presentation for user-declared `NOW()`-default temporal fields on SQLite (ADR-0074 follow-up)
6+
7+
A user-declared `Field.datetime` (or `date`/`time`) with `defaultValue: 'NOW()'`
8+
took the `knex.fn.now()``CURRENT_TIMESTAMP` column default on SQLite, storing a
9+
**timezone-naive**, space-separated `'YYYY-MM-DD HH:MM:SS'` (no millis, no zone).
10+
`Date.parse` reads such a zone-less string as *local* time, so the stored UTC
11+
wall-clock shifted by the host offset on a non-UTC runtime — the same class of bug
12+
ADR-0074 fixed for the builtin `created_at`/`updated_at` audit columns, but left
13+
scoped out for user fields. Worse, the **same** column mixed storage: an explicit
14+
JS `Date` is bound by better-sqlite3 as INTEGER epoch ms, while an omitted value
15+
took the naive TEXT default — so one column held both INTEGER ms and naive TEXT.
16+
17+
This fix, SQLite-only:
18+
19+
- **DDL default → canonical.** The `NOW()` default now emits a per-type canonical
20+
via `strftime`: datetime → ISO-8601 with explicit `Z`
21+
(`strftime('%Y-%m-%dT%H:%M:%fZ','now')`, e.g. `2026-06-26T10:34:13.891Z`,
22+
matching `new Date().toISOString()`); date → `YYYY-MM-DD`; time → `HH:MM:SS.fff`
23+
time-of-day (not a full timestamp).
24+
- **Read → uniform instant.** `formatOutput` folds every `Field.datetime` storage
25+
form — INTEGER epoch ms, canonical ISO-`Z`, and legacy naive `CURRENT_TIMESTAMP`
26+
TEXT — to one canonical ISO-8601-`Z` instant (`normalizeSqliteDatetimeOutput`),
27+
interpreting a naive wall-clock as UTC. Idempotent on already-zone-explicit
28+
values; total on null/unparseable. This transparently repairs existing rows on
29+
read (a DDL default only governs newly-created columns), so no data migration is
30+
needed — mirroring the `Field.date`/numeric read-repairs already in place.
31+
32+
Applied as DDL-default + read-normalization, NOT app-side write stamping (the
33+
inverse of ADR-0074's audit-column fix): the read path already repairs
34+
existing-table rows transparently, and an explicit `Date` is bound as INTEGER
35+
epoch ms regardless of any write stamp, so stamping wouldn't make on-disk storage
36+
uniform anyway — the INTEGER-vs-TEXT split is inherent to SQLite and resolved at
37+
the read boundary. This keeps the hot insert/upsert/bulk paths untouched.
38+
39+
The analytics SQL-bucketing path (`strftime`, bypasses `formatOutput`) is
40+
unchanged: ISO-`Z` TEXT buckets identically to the old naive TEXT. Postgres/MySQL
41+
keep native `now()` (a real zone-aware `TIMESTAMP`) and are entirely unaffected.
42+
43+
Generalizes ADR-0074's `repairNaiveUtcAuditTimestamp` by also folding the INTEGER
44+
epoch-ms storage form; the two read-repairs can be unified once both land.

packages/plugins/driver-sql/src/sql-driver-timestamp-format.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,21 @@ describe('SqlDriver canonical audit-timestamp format (SQLite)', () => {
135135
expect(row.updated_at).toBe(canonical);
136136
});
137137

138-
it('does not mangle a Field.datetime-typed audit column stored as epoch ms', async () => {
138+
it('presents a Field.datetime-typed audit column as canonical ISO-Z (Field.datetime owns it)', async () => {
139139
// When created_at is declared `datetime`, better-sqlite3 stores a JS Date as
140-
// INTEGER ms; the repair must leave that number alone (Field.datetime owns it).
140+
// INTEGER ms. The audit repair leaves that number alone, but the Field.datetime
141+
// read-normalization (normalizeSqliteDatetimeOutput) now owns its presentation
142+
// and folds it to one canonical ISO-8601-Z instant — consistent with every
143+
// other datetime field, and with the string-typed audit columns above.
141144
const dd = new SqlDriver({
142145
client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true,
143146
});
144147
try {
145148
await dd.initObjects([{ name: 'evt', fields: { created_at: { type: 'datetime' }, label: { type: 'string' } } }]);
146149
await dd.create('evt', { id: 'e1', label: 'x', created_at: new Date('2026-04-04T04:04:04.004Z') }, { bypassTenantAudit: true });
147150
const row: any = await dd.findOne('evt', 'e1', { bypassTenantAudit: true });
148-
expect(typeof row.created_at).toBe('number');
149-
expect(row.created_at).toBe(Date.parse('2026-04-04T04:04:04.004Z'));
151+
expect(typeof row.created_at).toBe('string');
152+
expect(row.created_at).toBe('2026-04-04T04:04:04.004Z');
150153
} finally {
151154
await dd.disconnect();
152155
}
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Canonical storage + presentation for USER-declared `defaultValue: 'NOW()'`
5+
* temporal fields on SQLite — the ADR-0053/ADR-0074 follow-up.
6+
*
7+
* A `Field.datetime` with `defaultValue: 'NOW()'` used to take the
8+
* `knex.fn.now()` → `CURRENT_TIMESTAMP` column default on SQLite, storing a
9+
* timezone-NAIVE, space-separated `'YYYY-MM-DD HH:MM:SS'` (no millis, no zone).
10+
* `Date.parse` reads such a zone-less string as LOCAL time, so the stored UTC
11+
* wall-clock shifts by the host offset on a non-UTC runtime — the same class of
12+
* bug ADR-0074 fixed for the builtin `created_at`/`updated_at` audit columns.
13+
* Worse, the SAME column mixes storage: an explicit JS `Date` is bound by
14+
* better-sqlite3 as INTEGER epoch ms, while an omitted value takes the naive
15+
* TEXT default — so one column holds both INTEGER ms and naive TEXT.
16+
*
17+
* These tests pin the fix:
18+
* 1. the DDL default now emits a canonical instant — ISO-8601 with `Z` for
19+
* datetime, `YYYY-MM-DD` for date, time-of-day for time;
20+
* 2. `formatOutput` folds every datetime storage form (INTEGER epoch ms,
21+
* canonical ISO-`Z`, legacy naive TEXT) to one canonical ISO-`Z` instant on
22+
* read, so reads are uniform regardless of how/when the row was written.
23+
* Postgres/MySQL keep native `now()` (a real zone-aware TIMESTAMP) and are
24+
* unaffected.
25+
*/
26+
27+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
28+
import { SqlDriver } from '../src/index.js';
29+
30+
const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/;
31+
const DATE_ONLY = /^\d{4}-\d{2}-\d{2}$/;
32+
const TIME_ONLY = /^\d{2}:\d{2}:\d{2}(?:\.\d{1,3})?$/;
33+
34+
/** Probe the per-dialect `nowColumnDefault` SQL without opening a connection. */
35+
class ProbeDriver extends SqlDriver {
36+
nowDefaultSql(type: string): string {
37+
return (this as any).nowColumnDefault(type).toString();
38+
}
39+
}
40+
function makeProbe(client: string): ProbeDriver {
41+
return new ProbeDriver({ client, connection: { filename: ':memory:' }, useNullAsDefault: true } as any);
42+
}
43+
44+
describe('User NOW()-default temporal fields — canonical format (SQLite)', () => {
45+
let driver: SqlDriver;
46+
let raw: any;
47+
48+
beforeEach(async () => {
49+
driver = new SqlDriver({
50+
client: 'better-sqlite3',
51+
connection: { filename: ':memory:' },
52+
useNullAsDefault: true,
53+
});
54+
raw = (driver as any).knex;
55+
await driver.initObjects([
56+
{
57+
name: 'event',
58+
fields: {
59+
label: { type: 'string' },
60+
starts_at: { type: 'datetime', defaultValue: 'NOW()' },
61+
on_day: { type: 'date', defaultValue: 'NOW()' },
62+
at_time: { type: 'time', defaultValue: 'NOW()' },
63+
},
64+
},
65+
]);
66+
});
67+
68+
afterEach(async () => {
69+
await driver.disconnect();
70+
});
71+
72+
// ── DDL default (raw on disk) ───────────────────────────────────────────────
73+
74+
it('datetime NOW()-default stores canonical ISO-8601-Z on an omitted insert (raw on disk)', async () => {
75+
await driver.create('event', { id: 'e1', label: 'A' }, { bypassTenantAudit: true });
76+
const row = await raw('event').where('id', 'e1').first();
77+
// The raw on-disk value — NOT the naive space-separated `CURRENT_TIMESTAMP`.
78+
expect(row.starts_at).toMatch(ISO_Z);
79+
expect(row.starts_at.endsWith('Z')).toBe(true);
80+
expect(row.starts_at).not.toContain(' ');
81+
});
82+
83+
it('date NOW()-default stores YYYY-MM-DD; time NOW()-default stores a time-of-day (not a full timestamp)', async () => {
84+
await driver.create('event', { id: 'e2', label: 'B' }, { bypassTenantAudit: true });
85+
const row = await raw('event').where('id', 'e2').first();
86+
expect(row.on_day).toMatch(DATE_ONLY);
87+
expect(row.at_time).toMatch(TIME_ONLY);
88+
expect(row.at_time).not.toContain('-'); // time-of-day, not a `YYYY-MM-DD …` timestamp
89+
});
90+
91+
// ── Read presentation: mixed storage → one canonical instant ────────────────
92+
93+
it('an explicit Date (stored as INTEGER epoch ms) reads back as canonical ISO-8601-Z', async () => {
94+
const when = new Date('2026-03-20T12:34:56.789Z');
95+
await driver.create('event', { id: 'e3', label: 'C', starts_at: when }, { bypassTenantAudit: true });
96+
97+
// Raw on disk is the INTEGER epoch (better-sqlite3 binds a Date as getTime()).
98+
const rawRow = await raw('event').where('id', 'e3').first();
99+
expect(typeof rawRow.starts_at).toBe('number');
100+
expect(rawRow.starts_at).toBe(when.getTime());
101+
102+
// …but formatOutput presents the canonical instant.
103+
const row: any = await driver.findOne('event', 'e3', { bypassTenantAudit: true });
104+
expect(typeof row.starts_at).toBe('string');
105+
expect(row.starts_at).toBe('2026-03-20T12:34:56.789Z');
106+
});
107+
108+
it('CONSISTENT PRESENTATION: an explicit-Date row and a defaulted row both read back as ISO-Z, despite genuinely mixed on-disk storage', async () => {
109+
await driver.create('event', { id: 'explicit', label: 'X', starts_at: new Date('2026-01-02T03:04:05.006Z') }, { bypassTenantAudit: true });
110+
await driver.create('event', { id: 'defaulted', label: 'Y' }, { bypassTenantAudit: true }); // omitted → DDL default
111+
112+
// On disk: one INTEGER, one TEXT — exactly the mixed storage the fix targets.
113+
const rawRows = await raw('event').whereIn('id', ['explicit', 'defaulted']).select('id', 'starts_at');
114+
const onDiskTypes = new Set(rawRows.map((r: any) => typeof r.starts_at));
115+
expect(onDiskTypes).toEqual(new Set(['number', 'string']));
116+
117+
// On read: uniform canonical ISO-Z, both parse to a real instant.
118+
for (const id of ['explicit', 'defaulted']) {
119+
const row: any = await driver.findOne('event', id, { bypassTenantAudit: true });
120+
expect(row.starts_at).toMatch(ISO_Z);
121+
expect(Number.isNaN(new Date(row.starts_at).getTime())).toBe(false);
122+
}
123+
});
124+
125+
it('an explicit ISO-8601-Z string is preserved (idempotent) on read', async () => {
126+
const iso = '2026-05-25T08:00:00.000Z';
127+
await driver.create('event', { id: 'e4', label: 'D', starts_at: iso }, { bypassTenantAudit: true });
128+
const row: any = await driver.findOne('event', 'e4', { bypassTenantAudit: true });
129+
expect(row.starts_at).toBe(iso);
130+
});
131+
132+
// ── Legacy / raw rows (read-repair, no data migration) ──────────────────────
133+
134+
it('repairs a legacy naive CURRENT_TIMESTAMP row to canonical ISO-Z on read, interpreting it as UTC', async () => {
135+
// A row written before this fix (or by a raw insert that took the OLD naive
136+
// `CURRENT_TIMESTAMP` default), bypassing the driver write path entirely.
137+
await raw('event').insert({ id: 'legacy', label: 'L', starts_at: '2026-01-15 08:30:00' });
138+
const row: any = await driver.findOne('event', 'legacy', { bypassTenantAudit: true });
139+
expect(row.starts_at).toBe('2026-01-15T08:30:00.000Z');
140+
});
141+
142+
it('REGRESSION (host-timezone independence): the repaired instant equals the UTC wall-clock', async () => {
143+
// The zone-naive `2026-01-15 08:30:00` must mean 08:30 UTC, NOT 08:30 local.
144+
await raw('event').insert({ id: 'tz', label: 'T', starts_at: '2026-01-15 08:30:00' });
145+
const row: any = await driver.findOne('event', 'tz', { bypassTenantAudit: true });
146+
expect(new Date(row.starts_at).getTime()).toBe(Date.parse('2026-01-15T08:30:00.000Z'));
147+
});
148+
149+
it('find() (list path) normalizes datetime identically to findOne(), across mixed storage', async () => {
150+
await raw('event').insert({ id: 'list1', label: 'L1', starts_at: '2026-02-02 02:02:02.200' });
151+
await driver.create('event', { id: 'list2', label: 'L2', starts_at: new Date('2026-02-02T02:02:02.200Z') }, { bypassTenantAudit: true });
152+
const rows = await driver.find('event', { orderBy: [{ field: 'id', order: 'asc' }] });
153+
const byId = Object.fromEntries(rows.map((r: any) => [r.id, r]));
154+
expect(byId.list1.starts_at).toBe('2026-02-02T02:02:02.200Z');
155+
expect(byId.list2.starts_at).toBe('2026-02-02T02:02:02.200Z');
156+
});
157+
158+
it('leaves an explicit null datetime untouched', async () => {
159+
const d2 = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
160+
try {
161+
await d2.initObjects([{ name: 'evt2', fields: { dt: { type: 'datetime' }, label: { type: 'string' } } }]);
162+
await d2.create('evt2', { id: 'n1', label: 'N', dt: null }, { bypassTenantAudit: true });
163+
const row: any = await d2.findOne('evt2', 'n1', { bypassTenantAudit: true });
164+
expect(row.dt).toBeNull();
165+
} finally {
166+
await d2.disconnect();
167+
}
168+
});
169+
170+
// ── Dialect gate (Postgres/MySQL unaffected) ────────────────────────────────
171+
172+
it('nowColumnDefault: SQLite emits canonical strftime; Postgres/MySQL keep native now()', () => {
173+
const sqlite = makeProbe('better-sqlite3');
174+
expect(sqlite.nowDefaultSql('datetime')).toContain('strftime');
175+
expect(sqlite.nowDefaultSql('datetime')).toContain('%Y-%m-%dT%H:%M:%fZ');
176+
expect(sqlite.nowDefaultSql('date')).toContain('%Y-%m-%d');
177+
expect(sqlite.nowDefaultSql('time')).toContain('%H:%M:%f');
178+
179+
for (const client of ['pg', 'mysql2']) {
180+
const native = makeProbe(client);
181+
const sql = native.nowDefaultSql('datetime');
182+
expect(sql).not.toContain('strftime');
183+
expect(sql.toUpperCase()).toContain('CURRENT_TIMESTAMP'); // = knex.fn.now()
184+
}
185+
});
186+
});

0 commit comments

Comments
 (0)