Skip to content

Commit 8646fb1

Browse files
committed
test(driver-sql): run the D-A3 temporal matrix across the driver axis, not four SQLite drivers (#4245)
ADR-0053 D-A3 declares the conformance matrix over `driver {SQLite, Postgres at minimum}`, and D-B3 added a server-timezone axis after D-B2 measured a dialect-divergent ROW RESULT on PG 16 @ Asia/Shanghai: a bare `YYYY-MM-DD` comparand meant midnight in the SERVER's timezone, so the identical query over the identical instant put a row on a different calendar day than it did on SQLite. Neither axis existed for the matrix. `sql-driver-temporal-conformance.test.ts` hard-coded `client: 'better-sqlite3'` in all four of its sweeps and never read `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL`, while its own head note claimed it ran "against real Postgres and MySQL too, with no workflow change needed" — the declared-not-enforced shape #4191 recorded for `mongodb-temporal.ts`. The three live suites that DO connect assert their own storage forms / NOW() defaults / round-trip precision, not the shared `TEMPORAL_CASES` row-id sets, so the matrix's "one backend drifts, one named case goes red" signal did not exist for PG or MySQL at all. - `live-dialect-matrix.testkit.ts`: the driver axis as data (`DIALECT_CELLS`) — SQLite always, live PG and MySQL when their URL is provisioned — plus the server-timezone probe and the three-way skew assertion. A cell list you opt out of, rather than a literal per suite. - All four sweeps run once per cell over the same `TEMPORAL_CASES` / `TEMPORAL_TIME_CASES`, asserting the same `expected` row-id sets cell for cell. - Non-vacuity: each live cell asserts server ≠ UTC, process ≠ UTC and server ≠ process, so identical answers cannot be answers no timezone could perturb; an unprovisioned cell is a NAMED skip, and `OS_EXPECT_LIVE_DIALECT_MATRIX=1` (set by the Temporal Conformance job) turns that skip into a red, closing the URL half of the vacuous-pass hole the job's `node -e` closes for the zone. - The legacy-storage sweeps stay SQLite-only because a pre-canonical storage form can only exist there — and that is now ASSERTED per cell against the driver's own `needsLegacy*Repair` rule, so the day a dialect grows a repair path the missing grid says so instead of staying invisible. - Head note rewritten to describe what the code does. Verified against a live PG 16 (`Asia/Shanghai`) and MariaDB 10.11 (`+08:00`) under `TZ=America/New_York`: 141 matrix tests green on all three dialects, whole package 752/752. Each guard sabotage-checked: UTC process, UTC server, process==server, missing URL under OS_EXPECT_LIVE_DIALECT_MATRIX=1, and a cell falsely claiming a legacy storage form all go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
1 parent 16fc124 commit 8646fb1

5 files changed

Lines changed: 627 additions & 154 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
---
3+
4+
Test-only: run the ADR-0053 D-A3 temporal conformance matrix across the DRIVER
5+
axis the ADR declares — `driver {SQLite, Postgres at minimum}` — instead of four
6+
hard-coded `better-sqlite3` drivers (#4245). All four sweeps (canonical
7+
datetime, relative tokens, `Field.time`, legacy storage) now run once per cell
8+
of `DIALECT_CELLS`: SQLite always, live Postgres and MySQL whenever
9+
`OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` are provisioned, over the same
10+
`TEMPORAL_CASES` / `TEMPORAL_TIME_CASES` and asserting the same row-id sets cell
11+
for cell. Adds the D-B3 server-timezone axis as an executed guard (server ≠ UTC
12+
≠ process, and server ≠ process) so a cell cannot pass vacuously, and
13+
`OS_EXPECT_LIVE_DIALECT_MATRIX=1` in the `Temporal Conformance (live PG +
14+
MySQL)` CI job so a lost `OS_TEST_*_URL` is a red rather than a silent return to
15+
SQLite-only coverage. Releases nothing.

.github/workflows/ci.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,14 @@ jobs:
435435
TZ: America/New_York
436436
OS_TEST_POSTGRES_URL: postgres://postgres:postgres@127.0.0.1:5432/postgres
437437
OS_TEST_MYSQL_URL: mysql://root:root@127.0.0.1:3306/conformance
438+
# The URL half of the same vacuous-pass hole the `node -e` below closes
439+
# for the zone (#4245). The temporal conformance MATRIX now runs one
440+
# cell per dialect (ADR-0053 D-A3 `driver {SQLite, Postgres at
441+
# minimum}`), and a cell whose URL is missing is an ordinary skip — so
442+
# losing either of the two lines above would silently return the matrix
443+
# to SQLite-only coverage and stay green. This flag says "this runner
444+
# provisioned the servers", which turns that skip into a named red.
445+
OS_EXPECT_LIVE_DIALECT_MATRIX: '1'
438446
run: |
439447
# The axis is only real if the zone actually took. Without this, a
440448
# dropped `TZ:` line silently returns the job to UTC coverage and

packages/plugins/driver-sql/src/legacy-datetime-storage.testkit.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,25 @@ export class LegacyStorageDriver extends SqlDriver {
5757
this.canonicalTimeFields[table]?.delete(field);
5858
}
5959

60+
/**
61+
* Does this dialect still owe a `Field.datetime` column the read-side legacy
62+
* repair? (#4245)
63+
*
64+
* The driver's own rule, exposed so a matrix consumer can ASSERT which
65+
* dialects have a legacy storage form instead of asserting it in a comment.
66+
* True only on SQLite — the typeless store where INTEGER epoch ms can sit
67+
* next to zone-naive TEXT in one column — and only while the column's
68+
* canonical marker is clear, so call it after {@link forgetCanonical}.
69+
*/
70+
legacyDatetimeRepairApplies(table: string, field: string): boolean {
71+
return this.needsLegacyDatetimeRepair(table, field);
72+
}
73+
74+
/** The `Field.time` twin of {@link legacyDatetimeRepairApplies} (#4245). */
75+
legacyTimeRepairApplies(table: string, field: string): boolean {
76+
return this.needsLegacyTimeRepair(table, field);
77+
}
78+
6079
/** Raw stored form of a column, for asserting on the fixture's premise. */
6180
async storedForms(table: string, field: string): Promise<Array<{ id: string; type: string; value: unknown }>> {
6281
const res: any = await this.knex.raw(
Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The DRIVER axis of the shared conformance matrices (ADR-0053 D-A3: the matrix
5+
* is `driver {SQLite, Postgres at minimum}` × …), plus the SERVER-TIMEZONE axis
6+
* D-B3 added to it — one definition of "which backends does a matrix consumer
7+
* run on, and what makes that run non-vacuous", so every consumer spells it the
8+
* same way instead of hard-coding `client: 'better-sqlite3'` (#4245).
9+
*
10+
* Why this exists as a helper rather than a literal per suite: a hard-coded
11+
* client is invisible. `sql-driver-temporal-conformance.test.ts` carried four of
12+
* them while its own head note claimed it ran "against real Postgres and MySQL
13+
* too" — declared ≠ enforced, and the ADR's `Postgres at minimum` never
14+
* executed for the matrix at all. A cell list you have to *opt out of* fails
15+
* loudly the moment a new sweep forgets a dialect.
16+
*
17+
* ## The non-vacuity contract
18+
*
19+
* A live cell proves nothing unless the three clocks actually disagree, which is
20+
* exactly the configuration D-B2 measured the dialect divergence under:
21+
*
22+
* - the SERVER's timezone (CI: PG `Asia/Shanghai`, MySQL `+08:00`),
23+
* - the PROCESS's timezone (CI: `TZ=America/New_York`),
24+
* - UTC, which is what the canon says every stored instant and every comparand
25+
* denotes.
26+
*
27+
* {@link assertThreeWayZoneSkew} asserts all three are pairwise different. On a
28+
* UTC server, or a UTC process, the identical answers a green matrix reports are
29+
* answers no timezone could have perturbed — a pass that means nothing. The
30+
* guard turns that into a red with the fix in the message.
31+
*
32+
* ## Skips are visible, and can be made fatal
33+
*
34+
* Without `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` a live cell is reported
35+
* as a named SKIP (never a silent pass). A runner that *knows* it provisioned
36+
* the servers — the `Temporal Conformance (live PG + MySQL)` CI job — sets
37+
* `OS_EXPECT_LIVE_DIALECT_MATRIX=1`, which turns a missing URL into a failure:
38+
* without it, dropping the `env:` block from that job would silently return the
39+
* whole matrix to SQLite-only coverage and stay green, which is the same
40+
* vacuous-pass hole the job's own process-zone assertion closes.
41+
*
42+
* Test-only: not exported from `index.ts`.
43+
*/
44+
45+
import { expect } from 'vitest';
46+
import type { SqlDriver, SqlDriverConfig } from './sql-driver.js';
47+
48+
/** The dialects `driver-sql` speaks that the matrices are run across. */
49+
export type DialectId = 'sqlite' | 'pg' | 'mysql';
50+
51+
export interface DialectCell {
52+
id: DialectId;
53+
/** Human label, used in suite names. */
54+
label: string;
55+
/** The env var that provisions this cell — `null` for the embedded SQLite one. */
56+
env: string | null;
57+
/** Provisioned connection string, when this cell needs one. */
58+
url?: string;
59+
/** Can this cell run right now? (SQLite always can.) */
60+
available: boolean;
61+
/** Does the cell talk to a separate server that carries its own timezone? */
62+
live: boolean;
63+
/**
64+
* Can rows in a PRE-canonical storage form still exist on this dialect — i.e.
65+
* does the driver keep a read-side repair for them?
66+
*
67+
* SQLite only, and not by convention: SQLite has no temporal type, so a
68+
* pre-#3912/#3994 database really does hold INTEGER epoch ms next to
69+
* zone-naive TEXT in one column, and `needsLegacyDatetimeRepair` /
70+
* `needsLegacyTimeRepair` gate the repair on `isSqlite`. Postgres and MySQL
71+
* store a real `timestamptz` / `DATETIME(3)` / `TIME(3)`, so their rows are
72+
* already one shape and there is nothing on disk to repair — see
73+
* `backfillCanonicalDatetimes`, which says exactly this and returns early.
74+
*
75+
* This flag is what a legacy sweep selects cells by, and consumers must ASSERT
76+
* the driver agrees with it (see `LegacyStorageDriver.legacyDatetimeRepairApplies`)
77+
* rather than trusting the constant — otherwise it decays into the same
78+
* unverified claim the hard-coded client was.
79+
*/
80+
hasLegacyStorageForm: boolean;
81+
/** Fresh driver config for this cell. */
82+
config(): SqlDriverConfig;
83+
}
84+
85+
const PG_URL = process.env.OS_TEST_POSTGRES_URL;
86+
const MYSQL_URL = process.env.OS_TEST_MYSQL_URL;
87+
88+
/**
89+
* `1` when the runner has provisioned the live servers and a missing URL is
90+
* therefore a defect in the runner, not a developer running without Docker.
91+
*/
92+
export const EXPECT_LIVE_DIALECTS = process.env.OS_EXPECT_LIVE_DIALECT_MATRIX === '1';
93+
94+
/**
95+
* Every cell of the driver axis, available or not — a consumer iterates the
96+
* whole list so an unprovisioned dialect is *reported*, not omitted.
97+
*/
98+
export const DIALECT_CELLS: readonly DialectCell[] = [
99+
{
100+
id: 'sqlite',
101+
label: 'sqlite',
102+
env: null,
103+
available: true,
104+
live: false,
105+
hasLegacyStorageForm: true,
106+
config: () => ({
107+
client: 'better-sqlite3',
108+
connection: { filename: ':memory:' },
109+
useNullAsDefault: true,
110+
}),
111+
},
112+
{
113+
id: 'pg',
114+
label: 'live postgres',
115+
env: 'OS_TEST_POSTGRES_URL',
116+
url: PG_URL,
117+
available: !!PG_URL,
118+
live: true,
119+
hasLegacyStorageForm: false,
120+
config: () => ({ client: 'pg', connection: PG_URL }),
121+
},
122+
{
123+
id: 'mysql',
124+
label: 'live mysql',
125+
env: 'OS_TEST_MYSQL_URL',
126+
url: MYSQL_URL,
127+
available: !!MYSQL_URL,
128+
live: true,
129+
hasLegacyStorageForm: false,
130+
config: () => ({ client: 'mysql2', connection: MYSQL_URL }),
131+
},
132+
] as const;
133+
134+
/** The live cells only — the ones the server-timezone axis applies to. */
135+
export const LIVE_DIALECT_CELLS = DIALECT_CELLS.filter((c) => c.live);
136+
137+
/** What a server reports about its own timezone. */
138+
export interface ServerZone {
139+
/** The dialect's own spelling: `Asia/Shanghai`, `+08:00`, `SYSTEM`, … */
140+
setting: string;
141+
/**
142+
* Minutes east of UTC the server is currently at, or `NaN` when the dialect
143+
* could not be made to say. `NaN` fails the skew guard on purpose: a zone we
144+
* cannot compare is a zone we cannot prove is skewed.
145+
*/
146+
offsetMinutes: number;
147+
}
148+
149+
/** Unwrap a raw result across knex's three dialect shapes. */
150+
function rowsOf(res: any): any[] {
151+
if (Array.isArray(res) && Array.isArray(res[0])) return res[0]; // mysql2: [rows, fields]
152+
if (Array.isArray(res)) return res; // better-sqlite3
153+
return res?.rows ?? []; // pg
154+
}
155+
156+
/**
157+
* Read the SERVER's timezone through an already-connected driver.
158+
*
159+
* Both queries deliberately read the server's own setting rather than anything
160+
* the driver configured: `driver-sql` pins the mysql2 *session* to UTC (#3942)
161+
* and Postgres reads back whatever `TimeZone` the server was started with, so
162+
* asking the session would report the fix instead of the hazard the fix exists
163+
* for.
164+
*/
165+
export async function readServerZone(cell: DialectCell, driver: SqlDriver): Promise<ServerZone> {
166+
if (cell.id === 'pg') {
167+
const rows = rowsOf(
168+
await driver.execute(
169+
`select current_setting('TimeZone') as tz, extract(timezone from now())::int as off_seconds`,
170+
),
171+
);
172+
const row = rows[0] ?? {};
173+
return { setting: String(row.tz ?? ''), offsetMinutes: eastOfUtc(Number(row.off_seconds) / 60) };
174+
}
175+
if (cell.id === 'mysql') {
176+
// `convert_tz` resolves a numeric `+08:00` zone without the (usually
177+
// unloaded) mysql tz tables; a NAMED global zone yields NULL there, so fall
178+
// back to parsing the setting and let the guard fail if neither can answer.
179+
const rows = rowsOf(
180+
await driver.execute(
181+
`select @@global.time_zone as tz,
182+
timestampdiff(second, utc_timestamp(),
183+
convert_tz(utc_timestamp(), '+00:00', @@global.time_zone)) as off_seconds`,
184+
),
185+
);
186+
const row = rows[0] ?? {};
187+
const setting = String(row.tz ?? '');
188+
const seconds = row.off_seconds == null ? Number.NaN : Number(row.off_seconds);
189+
return {
190+
setting,
191+
offsetMinutes: eastOfUtc(
192+
Number.isFinite(seconds) ? seconds / 60 : parseUtcOffsetMinutes(setting),
193+
),
194+
};
195+
}
196+
// SQLite is in-process: there is no server, and therefore no server zone.
197+
return { setting: '', offsetMinutes: Number.NaN };
198+
}
199+
200+
/**
201+
* Collapse `-0` onto `+0`.
202+
*
203+
* Not cosmetic: `expect(x).not.toBe(0)` is `Object.is`, and `Object.is(-0, 0)`
204+
* is FALSE — so a UTC zone that arrives as `-0` (which is what negating a
205+
* zero `getTimezoneOffset()` produces) sails through the "not UTC" guard. This
206+
* was measured by sabotage: `TZ=UTC` passed the guard until this existed.
207+
*/
208+
const eastOfUtc = (minutes: number): number => (minutes === 0 ? 0 : minutes);
209+
210+
/** `+08:00` / `-05:30` → minutes east of UTC; anything else → `NaN`. */
211+
function parseUtcOffsetMinutes(setting: string): number {
212+
const m = /^([+-])(\d{1,2}):(\d{2})$/.exec(setting.trim());
213+
if (!m) return Number.NaN;
214+
const minutes = Number(m[2]) * 60 + Number(m[3]);
215+
return eastOfUtc(m[1] === '-' ? -minutes : minutes);
216+
}
217+
218+
/** The Node process's timezone, as the two facts the guard compares. */
219+
export function processZone(): { name: string; offsetMinutes: number } {
220+
return {
221+
name: Intl.DateTimeFormat().resolvedOptions().timeZone || '(unknown)',
222+
// `getTimezoneOffset` is minutes WEST of UTC; flip it so both sides of the
223+
// comparison are "minutes east", the sign every server reports. Subtracting
224+
// rather than negating keeps a UTC process at `+0` — see {@link eastOfUtc}.
225+
offsetMinutes: eastOfUtc(0 - new Date().getTimezoneOffset()),
226+
};
227+
}
228+
229+
/**
230+
* The non-vacuity guard: server ≠ UTC ≠ process, and server ≠ process.
231+
*
232+
* Call it from an `it()` so a mis-provisioned run is a named red rather than a
233+
* green nobody reads. Every failure message carries the command that fixes it,
234+
* because the usual cause is a local run that simply never set `TZ`.
235+
*/
236+
export function assertThreeWayZoneSkew(cell: DialectCell, server: ServerZone): void {
237+
const proc = processZone();
238+
const seen = `server=${cell.id}:${server.setting || '(unreported)'} (${server.offsetMinutes} min), ` +
239+
`process=${proc.name} (${proc.offsetMinutes} min)`;
240+
241+
expect(
242+
Number.isFinite(server.offsetMinutes),
243+
`could not determine the ${cell.label} server's UTC offset (${seen}) — point it at a server ` +
244+
`with an explicit non-UTC timezone (PG: timezone=Asia/Shanghai, MySQL: default_time_zone='+08:00')`,
245+
).toBe(true);
246+
247+
expect(
248+
server.offsetMinutes,
249+
`the ${cell.label} server runs at UTC (${seen}) — on UTC the D-B2 divergence is invisible and ` +
250+
`this cell proves nothing; start it with timezone=Asia/Shanghai / default_time_zone='+08:00'`,
251+
).not.toBe(0);
252+
253+
expect(
254+
proc.offsetMinutes,
255+
`the process runs at UTC (${seen}) — re-run with a skewed zone, e.g. TZ=America/New_York, ` +
256+
`so a process-zone leak cannot hide behind an agreeing server`,
257+
).not.toBe(0);
258+
259+
expect(
260+
server.offsetMinutes,
261+
`the ${cell.label} server and the process share one UTC offset (${seen}) — the two zones must ` +
262+
`disagree, or a value folded through the wrong one still lands on the right answer`,
263+
).not.toBe(proc.offsetMinutes);
264+
}

0 commit comments

Comments
 (0)