diff --git a/.changeset/datetime-canonical-utc-storage.md b/.changeset/datetime-canonical-utc-storage.md new file mode 100644 index 0000000000..85f8fdb3b6 --- /dev/null +++ b/.changeset/datetime-canonical-utc-storage.md @@ -0,0 +1,56 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/service-analytics": patch +"@objectstack/spec": patch +--- + +fix(driver-sql): give `Field.datetime` one UTC storage form per dialect (#3912, #3942) + +Any window filter on a `Field.datetime` column returned an empty set on SQLite — +a dashboard `dateRange: last_30_days` on `created_date` read 0 while 29 matching +rows existed. + +There was never a storage *convention*, only a description of what better-sqlite3 +happened to do with a bound JS `Date`. Nothing enforced it — `formatInput` +deliberately left `datetime` untouched — so the form was decided by whichever +writer got there first: a JS `Date` landed as INTEGER epoch ms, while a REST/JSON +write (JSON has no `Date` type), a `defaultValue: 'NOW()'` slot, and the +platform's own `created_at` / `updated_at` all landed as ISO **TEXT**. One column +held both forms while the read path coerced comparands to epoch ms purely from +the *declared* type. On SQLite's type ordering (`INTEGER < TEXT`) a two-sided +window collapsed to zero rows, and a one-sided `>=` matched every TEXT row +regardless of the bound. + +`Field.datetime` now has one canonical instant per dialect, produced by one +function applied on write **and** to every filter comparand, so the two sides of +a comparison cannot disagree about shape: + +- **SQLite** — `YYYY-MM-DDTHH:MM:SS.sssZ` text. Lexicographic order *is* + chronological order, so range filters and `ORDER BY` read the column directly + and can use an index; `strftime` parses it, so the date-bucket expression needs + no CASE. +- **Postgres** — `timestamptz`, unchanged. The fix here is on the write and + comparand side: a zone-naive write was previously resolved against the + *server's* timezone (measured 8 hours off on `Asia/Shanghai`), and an + un-anchored `YYYY-MM-DD` comparand meant the server's local midnight, so the + identical query over the identical instant landed a row on a different calendar + day than SQLite did. +- **MySQL** — `DATETIME(3)` instead of `TIMESTAMP`, a connection pinned to UTC on + both the mysql2 and the server layer, and a MySQL-spelled bind carrying the + same UTC wall clock. MySQL accepts neither the `T` separator nor the `Z` suffix + in a datetime literal, so datetime writes over REST had always failed outright; + `TIMESTAMP` additionally truncated milliseconds and could not store an instant + outside 1970..2038. + +Existing rows converge at schema sync. Both migrations are allowed to fail: they +log, mark nothing, and the read paths keep a repair expression, so an un-migrated +column still compares and buckets **correctly** — just unindexed. Neither can +repair instants the old timezone-ambiguous write path recorded wrongly; they +preserve what is on disk. + +Also closes #3928 (datetime `ORDER BY` mis-sorted on mixed storage) by +construction. Rationale is recorded as ADR-0053 addendum D-B1..D-B4. + +The analytics change is additive: a `coerceTemporalFilterColumn` companion to the +existing `coerceTemporalFilterValue` hook, so a raw-SQL strategy can normalise the +column side too. Absent hook → byte-identical SQL. diff --git a/content/docs/automation/webhooks.mdx b/content/docs/automation/webhooks.mdx index 6548cde80b..2d83fc5ba7 100644 --- a/content/docs/automation/webhooks.mdx +++ b/content/docs/automation/webhooks.mdx @@ -148,8 +148,8 @@ writable. | `response_code` | number | Last HTTP status code received. | | `response_body` | textarea | Truncated to the first 16 KB. | | `error` | textarea | Last transport-level error (DNS, connect, timeout). | -| `created_at` | datetime | Native TIMESTAMP column — written as a `Date`, not an epoch-ms number. | -| `updated_at` | datetime | Native TIMESTAMP column — written as a `Date`, not an epoch-ms number. | +| `created_at` | datetime | A `Field.datetime` UTC instant — not an epoch-ms number like the columns above. | +| `updated_at` | datetime | A `Field.datetime` UTC instant — not an epoch-ms number like the columns above. | > **Why store full payload?** Receivers may be down for hours; we must > retry the *exact* bytes we promised to send. Recomputing payload from diff --git a/content/docs/references/data/date-macros.mdx b/content/docs/references/data/date-macros.mdx index 938aeb2506..547991e5c8 100644 --- a/content/docs/references/data/date-macros.mdx +++ b/content/docs/references/data/date-macros.mdx @@ -53,9 +53,13 @@ Either way the DRIVER only ever sees ISO date / timestamp strings, never `\{tokens\}`. Translating an ISO comparand into a column's on-disk -form (SQLite epoch-ms, `YYYY-MM-DD` text, native timestamp) is the +form — canonical UTC text on SQLite, a native `timestamptz` on -driver's job — see `SqlDriver.temporalFilterValue`. +Postgres, a `DATETIME(3)` literal on MySQL, and `YYYY-MM-DD` text for + +a calendar day on every dialect — is the driver's job; see + +`SqlDriver.temporalFilterValue`. A token OUTSIDE this vocabulary is rejected rather than passed diff --git a/docs/adr/0053-date-and-datetime-semantics.md b/docs/adr/0053-date-and-datetime-semantics.md index 3f0b041add..b5ab1d8358 100644 --- a/docs/adr/0053-date-and-datetime-semantics.md +++ b/docs/adr/0053-date-and-datetime-semantics.md @@ -1,6 +1,6 @@ # ADR-0053: `date` is a timezone-naive calendar day; `datetime` is an instant rendered in a reference timezone -**Status**: Accepted (2026-06-16) — Phase 1 + addendum D-A1 implemented (`sql-driver.ts` `toDateOnly` write/read/filter normalization; analytics `coerceTemporalFilterValue`), Phase 2 landing incrementally; D-A2 (`temporalFilterValue` promotion onto the `IDataDriver` contract) still open as the ADR predicted. +**Status**: Accepted (2026-06-16) — Phase 1 + addendum D-A1 implemented (`sql-driver.ts` `toDateOnly` write/read/filter normalization; analytics `coerceTemporalFilterValue`), Phase 2 landing incrementally; D-A2 (`temporalFilterValue` promotion onto the `IDataDriver` contract) still open as the ADR predicted. **Partly superseded (2026-07-29, addendum D-B1..D-B4):** Phase 1's "`Field.datetime` stays stored as UTC epoch ms" is replaced by one canonical UTC instant per dialect — `YYYY-MM-DDTHH:MM:SS.sssZ` text on SQLite, `timestamptz` on Postgres, `DATETIME(3)` on MySQL — applied on write and to filter comparands alike (#3912, #3942). **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0032](./0032-unified-expression-layer.md) (unified expression layer — CEL dialect, `today()`/`daysFromNow()`), [ADR-0014](./0014-record-form-field-type.md) (field types) **Consumers**: `@objectstack/spec` (`Field.date`/`Field.datetime`), `@objectstack/driver-sql` (`coerceFilterValue`, `formatInput`/`formatOutput`, `dateFields`/`datetimeFields`), `@objectstack/formula` (`stdlib` time functions, `cel-engine` hydration), `@objectstack/objectql` (`applyFormulaPlan`), schedule/cron executors, report/analytics date bucketing, `sys-user-preference.timezone`. @@ -402,3 +402,160 @@ time, the matrix proves runtime correctness across drivers. inventing a semantic" stance. No change to Phase 2's reference-timezone plan. - Until D-A2 lands, the hook depends on a duck-typed driver method — a known, intentionally-temporary seam tracked here. + +--- + +## Addendum (2026-07-29) — `Field.datetime` has ONE storage form per dialect, always a UTC instant + +> **Status:** landed. This addendum **revises** the storage half of Phase 1 (which +> left `Field.datetime` "stored as UTC epoch ms" on SQLite) and **extends** +> D-A1 from the 2026-06-18 addendum to the column side of the comparison. +> Closes #3912; supersedes the epoch-ms convention referenced there. + +### What the epoch-ms convention actually was + +It was never a convention — it was a description of what better-sqlite3 happened +to do with a bound JS `Date`. Nothing enforced it: `formatInput` deliberately left +`datetime` untouched, so the storage form was decided by whichever writer got +there first. + +- A JS `Date` (seed loader, import, server-side code) → INTEGER epoch ms. +- A REST/JSON write → ISO **TEXT**, because JSON has no `Date` type. +- A `defaultValue: 'NOW()'` slot → ISO TEXT. +- The platform's own `created_at` / `updated_at` → ISO TEXT (they are stamped + with `toISOString()`), on **every** object in the system. + +One column therefore held both forms at once, while the read path coerced filter +comparands to epoch ms purely from the DECLARED type. On SQLite's type ordering +(`INTEGER < TEXT`) that made a two-sided window collapse to zero rows and a +one-sided `>=` match every TEXT row regardless of the bound — the reported symptom +in #3912 (a dashboard `last_30_days` reading 0 with 29 rows in range). It also +left `ORDER BY` sorting all INTEGER rows before all TEXT ones (#3928). + +### D-B1 — The canonical storage form is `YYYY-MM-DDTHH:MM:SS.sssZ` + +`Field.datetime` is stored as fixed-width, zone-explicit UTC text on SQLite, and +written to Postgres/MySQL as that same string. `canonicalUtcDatetime()` is the one +function that produces it, applied on write (`formatInput`) and to every filter +comparand (`coerceFilterValue`) so the two sides of a comparison cannot disagree +about shape. + +Chosen over the epoch integer because: + +- Lexicographic order **is** chronological order, so range filters and `ORDER BY` + read the column directly and can use an index. An epoch convention forces an + expression wrapper on every temporal predicate — it moves the cost rather than + removing it. +- `strftime`/`julianday` parse it, so the date-bucket expression needs no + epoch↔text CASE (#3773). +- It is what `formatOutput` already presents, so storage and presentation stop + disagreeing — the asymmetry Phase 1 removed for `date`, now removed for + `datetime`. +- It matches the `Field.date` convention, so the platform has one temporal + storage story instead of one per field type. +- It was already the majority of rows on disk, so the migration is the smaller + one. + +### D-B2 — The comparand rule is dialect-INDEPENDENT, which fixes Postgres too + +`coerceFilterValue` previously canonicalised only on SQLite and passed the value +through on native-timestamp dialects. That was not neutral. Measured against +PostgreSQL 16 with `TimeZone = Asia/Shanghai`: + +- A zone-**naive** write (`'2026-03-20 12:00:00'`) bound into `timestamptz` was + resolved against the SERVER's timezone and stored as `2026-03-20T04:00:00Z` — + 8 hours off the instant SQLite records for the same write, in direct violation + of this ADR's "a naive wall clock is UTC" rule. +- A bare `YYYY-MM-DD` comparand (what a `{30_days_ago}` token expands to) 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. + +Stating the `Z` on both sides removes the server's timezone from the answer. +Postgres storage itself needed no change — knex's `table.timestamp` already +creates `timestamptz` (`useTz` defaults true, verified) — so this is a write- and +comparand-side fix there, not a migration. Regression cover is +`sql-driver-datetime-postgres-timezone.test.ts`, opt-in via `OS_TEST_POSTGRES_URL` +because CI provisions no server; it asserts it is pointed at a non-UTC server so +it cannot pass vacuously. + +### D-B3 — Existing rows converge at schema sync; correctness never depends on it + +`backfillCanonicalDatetimes` runs inside `initObjects`, rewriting SQLite rows that +are not already canonical (INTEGER/REAL epoch, zone-naive text, offset-bearing +text). It is idempotent, skips freshly-created tables entirely, and preserves +values SQLite cannot parse rather than nulling them. + +It is allowed to fail. On error it logs and marks nothing, and the read paths keep +the `sqliteCanonicalDatetimeSql` repair that makes an un-migrated column compare +and bucket correctly — just without an index. A migration must never be able to +take boot down, and query correctness must never be contingent on one having run. +The corollary is that the repair expression stays in the codebase permanently: it +also covers external/unmanaged tables (ADR-0015), which never get a backfill. + +`needsLegacyDatetimeRepair` is the single predicate every read path asks, so the +filter, bucket and analytics surfaces cannot drift apart about whether a column +is clean. + +### Consequences + +- D-A2 (promote `temporalFilterValue` onto the `IDataDriver` contract) now has a + second method to carry with it: `temporalFilterColumnSql`, the column-side + companion threaded to analytics as + `StrategyContext.coerceTemporalFilterColumn`. Coercing the value alone is not + sufficient on a mixed-form column, so any surface that binds a comparand into + raw SQL must wrap its column reference too. Both remain duck-typed until D-A2. +- D-A3's conformance matrix should gain a **storage-form** axis (canonical, + legacy-epoch, legacy-naive) and a **server-timezone** axis, since both are now + known to have produced dialect-divergent row results. +- #3928 (datetime `ORDER BY` mis-sorted on mixed storage) is closed by + construction rather than by a sort-side fix. +### D-B4 — MySQL stores the same instant, spelled the way MySQL parses it + +MySQL was the third dialect, and measurement (MariaDB 10.11, `default_time_zone += '+08:00'`, process on `America/New_York`) found it the worst off — including +one defect D-B1 *introduced*: + +- MySQL accepts neither the `T` separator nor the `Z` suffix in a datetime + literal, so the canonical form failed the statement outright with *Incorrect + datetime value*. An ISO comparand or REST/JSON write had **always** failed this + way; canonicalising the write path extended it to `Date` writes too. +- `table.timestamp` emits MySQL `TIMESTAMP`: a 32-bit epoch that cannot hold an + instant outside 1970..2038 (a contract end date in 2040 was rejected), carries + no fractional digits so the canonical form's milliseconds were truncated, and + converts on read/write using the session timezone. +- mysql2's `connection.timezone` defaults to the HOST's local zone and + `@@session.time_zone` to the server's, so a zone-naive value landed 12 hours + off with the two misconfigurations compounding. + +The resolution keeps the logical canon and changes only the physical spelling: + +1. `Field.datetime` maps to **`DATETIME(3)`** — range 1000..9999, milliseconds + kept, and no timezone conversion of its own, so the column holds the UTC wall + clock the driver writes. This is the ServiceNow model. Postgres deliberately + keeps `timestamptz`: asking for precision 3 there would *reduce* it from + microseconds. The builtin `created_at`/`updated_at` take the same type — the + registry declares them `Field.datetime`, and they are what most list views + sort by. +2. The connection is **pinned to UTC on both layers** — `connection.timezone = + 'Z'` for mysql2 and `SET time_zone = '+00:00'` via `pool.afterCreate` for the + server. This is what makes the wall clock *be* the instant, and it keeps a + not-yet-migrated `TIMESTAMP` column correct too. An explicit host choice is + left alone; an existing `afterCreate` is chained, not replaced. +3. `storageDatetimeValue` respells the canonical instant as a MySQL literal for + the bind, on the write path and the filter path alike. Deliberately strict: + only an exactly-canonical string is rewritten, so an unparseable value or a + year outside 1000..9999 reaches MySQL untouched and fails loudly. + +`migrateMysqlDatetimeColumns` widens legacy `TIMESTAMP` columns at schema sync, +under the same failure policy as D-B3 — a `TIMESTAMP` column keeps *working* +(the driver binds the same literal and the session is UTC), it merely keeps the +range and precision limits. Verified on a real server: the ALTER moves no stored +instant, correctly-stored legacy rows round-trip exactly, and it is idempotent. + +The same caveat as Postgres applies and is worth stating plainly: the migration +**cannot repair instants the old timezone-ambiguous write path recorded wrongly** +— that information is gone. It preserves what is on disk. + +Regression cover is `sql-driver-datetime-mysql-storage.test.ts`, opt-in via +`OS_TEST_MYSQL_URL` (CI provisions no server), asserting a non-UTC server so it +cannot pass vacuously. 10 of its 13 cases fail without this change. diff --git a/packages/plugins/driver-sql/src/legacy-datetime-storage.testkit.ts b/packages/plugins/driver-sql/src/legacy-datetime-storage.testkit.ts new file mode 100644 index 0000000000..e842b7ce82 --- /dev/null +++ b/packages/plugins/driver-sql/src/legacy-datetime-storage.testkit.ts @@ -0,0 +1,51 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Test double for a SQLite database that still holds PRE-canonical + * `Field.datetime` values — the shape every deployment had before #3912, and the + * shape one still has when `backfillCanonicalDatetimes` could not run (it logs + * and swallows, deliberately, so a migration failure cannot take boot down). + * + * Since #3912 the write path canonicalises, so the legacy forms cannot be + * produced through `create()` any more. They have to be inserted RAW, and the + * column's canonical marker has to be cleared, or the driver would correctly + * conclude the column is clean and skip the read-side repair. Both halves are + * needed for the fixture to be honest — which is exactly why they live in one + * helper rather than being re-improvised per suite. + * + * Not exported from the package entry (`index.ts`): this is test-only. + */ + +import { SqlDriver } from './sql-driver.js'; + +export class LegacyStorageDriver extends SqlDriver { + /** + * Insert rows bypassing `formatInput`, so each `datetime` value lands in + * whatever form it is given (a number → INTEGER epoch ms, a string → TEXT), + * then mark `field` as NOT known-canonical so the read paths apply their + * repair — the state of an un-migrated database. + */ + async seedLegacyRows( + table: string, + field: string, + rows: Array>, + ): Promise { + await this.knex(table).insert(rows); + this.forgetCanonical(table, field); + } + + /** Drop the "already backfilled" marker for one column. */ + forgetCanonical(table: string, field: string): void { + this.canonicalDatetimeFields[table]?.delete(field); + } + + /** Raw stored form of a column, for asserting on the fixture's premise. */ + async storedForms(table: string, field: string): Promise> { + const res: any = await this.knex.raw( + `select id, typeof(??) as t, ?? as v from ?? order by id`, + [field, field, table], + ); + const rows = Array.isArray(res) ? res : (res?.rows ?? []); + return rows.map((r: any) => ({ id: r.id, type: r.t, value: r.v })); + } +} diff --git a/packages/plugins/driver-sql/src/sql-driver-aggregate-temporal-output.test.ts b/packages/plugins/driver-sql/src/sql-driver-aggregate-temporal-output.test.ts index 69a3a2ce36..048fa982a7 100644 --- a/packages/plugins/driver-sql/src/sql-driver-aggregate-temporal-output.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-aggregate-temporal-output.test.ts @@ -22,6 +22,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; const TABLE = 'deal'; @@ -43,7 +44,7 @@ describe('temporal values leaving aggregate()/distinct() (#3797)', () => { { name: TABLE, fields: { - closed_at: { type: 'datetime' }, // INTEGER epoch ms under better-sqlite3 + closed_at: { type: 'datetime' }, // canonical ISO-Z TEXT since #3912 closed_on: { type: 'date' }, // YYYY-MM-DD TEXT region: { type: 'string' }, amount: { type: 'number' }, @@ -100,27 +101,38 @@ describe('temporal values leaving aggregate()/distinct() (#3797)', () => { }); it('collapses two storage forms of the same instant into one value', async () => { - // SQL `DISTINCT` compares STORED values, and one SQLite `Field.datetime` - // column holds both forms — `formatInput` passes datetime values through, - // so a `Date` lands as INTEGER epoch ms and an ISO string lands as TEXT. - // Two rows recording the SAME instant therefore survive `DISTINCT` as two - // rows and then present identically, which is a duplicate unless the - // presented values are re-deduplicated. - await driver.create( - TABLE, - { id: 'd4', closed_at: ISO, closed_on: '2026-01-10', region: 'east', amount: 8 }, - { bypassTenantAudit: true }, - ); - - const raw: any = await driver.execute( - `SELECT DISTINCT typeof("closed_at") AS t FROM "${TABLE}" ORDER BY t`, - ); - const forms = (Array.isArray(raw) ? raw : (raw?.rows ?? [])).map((r: any) => r.t); - expect(forms).toContain('text'); // the row just written - expect(forms.some((f: string) => f === 'integer' || f === 'real')).toBe(true); - - // Three stored rows for two instants → two values, not three. - expect((await driver.distinct(TABLE, 'closed_at')).sort()).toEqual([ISO, ISO_LATER]); + // SQL `DISTINCT` compares STORED values. A pre-#3912 SQLite + // `Field.datetime` column held both forms — `formatInput` passed datetime + // values through, so a `Date` landed as INTEGER epoch ms and an ISO string + // as TEXT. Two rows recording the SAME instant therefore survived + // `DISTINCT` as two rows and then presented identically: a duplicate + // unless the presented values are re-deduplicated. + // + // Canonical storage means new writes can no longer produce that pair, so + // the fixture is an UN-MIGRATED database — where this dedup is still what + // stands between a legacy row and a duplicated chart axis label. + const legacy = new LegacyStorageDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }); + try { + await legacy.initObjects([ + { name: TABLE, fields: { closed_at: { type: 'datetime' }, region: { type: 'string' } } }, + ]); + await legacy.seedLegacyRows(TABLE, 'closed_at', [ + { id: 'l1', closed_at: Date.parse(ISO), region: 'east' }, // INTEGER epoch + { id: 'l2', closed_at: ISO, region: 'east' }, // canonical TEXT + { id: 'l3', closed_at: ISO_LATER, region: 'west' }, + ]); + + const forms = (await legacy.storedForms(TABLE, 'closed_at')).map((r) => r.type); + expect(forms).toContain('text'); + expect(forms.some((f) => f === 'integer' || f === 'real')).toBe(true); + + // Three stored rows for two instants → two values, not three. + expect((await legacy.distinct(TABLE, 'closed_at')).sort()).toEqual([ISO, ISO_LATER]); + } finally { + await legacy.disconnect(); + } }); }); diff --git a/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts b/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts index efa031ccdd..b88d129d65 100644 --- a/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts @@ -1,26 +1,29 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * End-to-end repro of the dashboard time-series "No rows" bug at the storage - * level, and proof of the fix. + * End-to-end cover of the dashboard time-series "No rows" bug at the storage + * level, and proof of the fix — now that the fix is canonical STORAGE (#3912) + * rather than a comparand cast. * * The analytics `NativeSQLStrategy` compiles dashboard relative-date tokens * (e.g. `{12_months_ago}`) to ISO date strings and binds them into a raw * `SELECT … WHERE col >= ?` that it runs through the driver's `execute()` — * bypassing the normal `find()` filter coercion. Under better-sqlite3 a - * `Field.datetime` column is stored as an INTEGER epoch (ms), so the ISO TEXT - * comparand never matches (TEXT sorts after every INTEGER) → 0 rows, even though - * the rows exist. A `Field.date` column stores ISO TEXT and matches fine. + * `Field.datetime` column USED to be stored as an INTEGER epoch (ms) whenever a + * `Date` was bound, so an ISO TEXT comparand never matched it (TEXT sorts after + * every INTEGER) → 0 rows even though the rows existed. * - * This test reproduces both the broken (raw ISO bind → 0) and fixed (epoch bind - * via the driver's public `temporalFilterValue` → N) behaviour against a real - * SQLite database, mirroring exactly what the analytics strategy now does. + * Both sides are one shape now: `formatInput` writes canonical UTC text and + * `temporalFilterValue` canonicalises the comparand to the same. These tests run + * that against a real SQLite database, and the MIXED-storage suite below covers + * what an un-migrated database still needs from the read-side repair. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; -describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => { +describe('Analytics datetime filter — SQLite canonical storage (E2E)', () => { let driver: SqlDriver; const TABLE = 'compliance_assessment'; const CUTOFF = '2025-06-18'; // ISO date token the dashboard expands to @@ -37,15 +40,15 @@ describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => name: TABLE, fields: { title: { type: 'string' }, - assessed_at: { type: 'datetime' }, // stored as INTEGER epoch ms - assessed_on: { type: 'date' }, // stored as YYYY-MM-DD text + assessed_at: { type: 'datetime' }, // canonical ISO-Z text since #3912 + assessed_on: { type: 'date' }, // YYYY-MM-DD text }, }, ]); // Four assessments AFTER the cutoff, one well before — inserted with real - // Date objects so better-sqlite3 stores `assessed_at` as INTEGER epoch ms, - // exactly the path the seed loader takes. + // Date objects, the path the seed loader takes. That used to land as INTEGER + // epoch ms; `formatInput` now canonicalises it on the way in. const rows = [ ['a1', new Date('2024-01-01T00:00:00Z'), '2024-01-01'], // before cutoff ['a2', new Date('2025-06-18T09:00:00Z'), '2025-06-18'], // on/after @@ -75,22 +78,33 @@ describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => return Number(row.n); }; - it('BUG: a raw ISO comparand against the epoch datetime column returns 0 rows', async () => { - // This is what the type-blind strategy used to bind — the silent failure. - expect(await countWhere('assessed_at', CUTOFF)).toBe(0); + it('the datetime column is stored as canonical text, like the date column', async () => { + const res: any = await driver.execute( + `SELECT DISTINCT typeof("assessed_at") AS t FROM "${TABLE}"`, + ); + const rows = Array.isArray(res) ? res : res?.rows ?? []; + expect(rows.map((r: any) => r.t)).toEqual(['text']); }); - it('FIX: the driver-coerced epoch comparand returns the 4 matching rows', async () => { - // `temporalFilterValue` is exactly the hook NativeSQLStrategy now calls. + it('FIX: the canonicalised comparand returns the 4 matching rows', async () => { + // `temporalFilterValue` is exactly the hook NativeSQLStrategy calls. const coerced = driver.temporalFilterValue(TABLE, 'assessed_at', CUTOFF); - expect(typeof coerced).toBe('number'); // epoch ms, not the ISO string + expect(coerced).toBe('2025-06-18T00:00:00.000Z'); // canonical text, not epoch ms expect(await countWhere('assessed_at', coerced)).toBe(4); }); + it('the bare ISO date matches too — both sides are text now (index-friendly)', async () => { + // Canonical storage is fixed-width UTC, so `'2025-06-18' <= '2025-06-18T…'` + // lexicographically. The comparand cast stops being load-bearing for a + // day-granular bound, which is what keeps the emitted SQL a plain + // `col >= ?` against an indexable column. + expect(await countWhere('assessed_at', CUTOFF)).toBe(4); + }); + it('CONTROL: the `Field.date` text column already matched the raw ISO comparand', async () => { // Proves the date/text path was never broken and is left untouched. const coerced = driver.temporalFilterValue(TABLE, 'assessed_on', CUTOFF); - expect(typeof coerced).toBe('string'); // YYYY-MM-DD, NOT coerced to epoch + expect(coerced).toBe('2025-06-18'); // YYYY-MM-DD, not an instant expect(await countWhere('assessed_on', coerced)).toBe(4); // and the raw ISO bind matches identically (no coercion needed for text) expect(await countWhere('assessed_on', CUTOFF)).toBe(4); @@ -100,3 +114,98 @@ describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => expect(driver.temporalFilterValue(TABLE, 'title', 'hello')).toBe('hello'); }); }); + +/** + * #3912 — what an UN-MIGRATED database still needs. + * + * Before canonical storage, one column legitimately held both forms: REST/JSON + * writes carry ISO strings (JSON has no `Date`) and `NOW()` defaults stamp ISO + * TEXT, while a bound `Date` landed as INTEGER epoch. An epoch comparand then + * matched the INTEGER half and missed every TEXT row — a dashboard + * `last_30_days` reading 0 with rows in range. + * + * New writes cannot produce that pair any more, and `backfillCanonicalDatetimes` + * converges existing rows at schema sync. But the backfill is allowed to fail + * (it logs and swallows, so a migration can never take boot down), and an + * external/unmanaged table never gets one — so `temporalFilterColumnSql` still + * has to make the comparison form-agnostic. These tests bind exactly what the + * analytics strategy binds, against a real mixed-storage SQLite table. + */ +describe('Analytics datetime filter — legacy MIXED storage (#3912)', () => { + let driver: LegacyStorageDriver; + const TABLE = 'compliance_assessment'; + const CUTOFF = '2025-06-18'; + + beforeEach(async () => { + driver = new LegacyStorageDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.initObjects([ + { + name: TABLE, + fields: { title: { type: 'string' }, assessed_at: { type: 'datetime' } }, + }, + ]); + + await driver.seedLegacyRows(TABLE, 'assessed_at', [ + { id: 'i1', title: 'i1', assessed_at: Date.parse('2024-01-01T00:00:00Z') }, // INTEGER, before + { id: 't1', title: 't1', assessed_at: '2024-02-01T00:00:00.000Z' }, // TEXT, before + { id: 'i2', title: 'i2', assessed_at: Date.parse('2025-09-01T09:00:00Z') }, // INTEGER, after + { id: 't2', title: 't2', assessed_at: '2025-10-01T09:00:00.000Z' }, // TEXT, after + { id: 't3', title: 't3', assessed_at: '2026-01-15T09:00:00.000Z' }, // TEXT, after + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + /** + * The `dateRange` shape the dashboard actually emits — a two-sided window, with + * both bounds coerced, optionally reading the column through the fix. + */ + const countInWindow = async (col: string, from: string, to: string, useColumnHook: boolean) => { + const ref = useColumnHook + ? driver.temporalFilterColumnSql(TABLE, col, `"${col}"`) + : `"${col}"`; + const res: any = await driver.execute( + `SELECT count(*) AS n FROM "${TABLE}" WHERE ${ref} >= ? AND ${ref} <= ?`, + [ + driver.temporalFilterValue(TABLE, col, from), + driver.temporalFilterValue(TABLE, col, to), + ], + ); + const row = Array.isArray(res) ? res[0] : res?.rows?.[0] ?? res; + return Number(row.n); + }; + + it('the fixture really is mixed-form', async () => { + const res: any = await driver.execute( + `SELECT typeof(assessed_at) AS t, count(*) AS n FROM "${TABLE}" GROUP BY 1 ORDER BY 1`, + ); + const rows = Array.isArray(res) ? res : res?.rows ?? []; + expect(rows.map((r: any) => [r.t, Number(r.n)])).toEqual([['integer', 2], ['text', 3]]); + }); + + it('BUG: reading the raw column silently drops the rows in the OTHER form', async () => { + // SQLite orders INTEGER before TEXT, so against a canonical TEXT comparand + // every legacy epoch row fails `>=` outright. The window quietly returns a + // SUBSET — the shape of #3912, and the reason the read-side repair cannot + // just be deleted once writes are canonical: a database that has not been + // backfilled still holds the other form. + expect(await countInWindow('assessed_at', CUTOFF, '2026-12-31', false)).toBe(2); // t2,t3 — i2 lost + expect(await countInWindow('assessed_at', '2024-01-01', '2024-06-30', false)).toBe(1); // t1 — i1 lost + }); + + it('FIX: normalising the column finds rows of BOTH forms in the window', async () => { + expect(await countInWindow('assessed_at', CUTOFF, '2026-12-31', true)).toBe(3); // i2, t2, t3 + expect(await countInWindow('assessed_at', '2025-10-01', '2026-12-31', true)).toBe(2); // t2, t3 + expect(await countInWindow('assessed_at', '2024-01-01', '2024-12-31', true)).toBe(2); // i1, t1 + }); + + it('leaves a non-temporal column reference untouched', () => { + expect(driver.temporalFilterColumnSql(TABLE, 'title', '"title"')).toBe('"title"'); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-connect-bound.test.ts b/packages/plugins/driver-sql/src/sql-driver-connect-bound.test.ts index 6b3fb4a25c..851532ee19 100644 --- a/packages/plugins/driver-sql/src/sql-driver-connect-bound.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-connect-bound.test.ts @@ -71,9 +71,37 @@ describe('SqlDriver — connection-attempt bound (framework#3769)', () => { expect((d as any).knex.client.config.connection).toEqual({ uri: 'mysql://u:p@host:3306/d', connectTimeout: 10_000, + // #3942 — mysql2 renders a bound `Date` and parses a returned DATETIME in + // `connection.timezone`, which defaults to the HOST's local zone. Pinned + // to UTC so the recorded instant cannot depend on which machine wrote it. + timezone: 'Z', }); }); + it('pins MySQL to UTC on both layers, and leaves an explicit choice alone (#3942)', () => { + const pinned = make({ client: 'mysql2', connection: { host: 'db', database: 'app' } }); + expect((pinned as any).knex.client.config.connection.timezone).toBe('Z'); + // The server side is the other half: `@@session.time_zone` decides how a + // zone-naive literal is read for a legacy TIMESTAMP column, and what + // CURRENT_TIMESTAMP renders. Measured 8h off on a `+08:00` server. + expect(typeof (pinned as any).knex.client.config.pool.afterCreate).toBe('function'); + + const explicit = make({ + client: 'mysql2', + connection: { host: 'db', database: 'app', timezone: '+08:00' }, + }); + expect((explicit as any).knex.client.config.connection.timezone).toBe('+08:00'); + }); + + it('does not touch the connection timezone on non-MySQL dialects', () => { + // Postgres resolves an explicit-offset literal itself and SQLite has no + // session zone at all, so neither needs (or should get) the mysql2 knob. + for (const client of ['pg', 'better-sqlite3']) { + const d = make({ client, connection: { host: 'db', database: 'app', filename: ':memory:' } }); + expect((d as any).knex.client.config.connection.timezone).toBeUndefined(); + } + }); + it('adds the timeout to an object connection without disturbing its fields', () => { const d = make({ client: 'pg', diff --git a/packages/plugins/driver-sql/src/sql-driver-date-bucket-storage.test.ts b/packages/plugins/driver-sql/src/sql-driver-date-bucket-storage.test.ts index 7f161a1eb6..42770a03d2 100644 --- a/packages/plugins/driver-sql/src/sql-driver-date-bucket-storage.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-date-bucket-storage.test.ts @@ -5,22 +5,26 @@ * * `sql-driver-date-bucket.test.ts` builds its fixture with * `knex.schema.createTable` + `t.string('ts')`, so every value it buckets is ISO - * TEXT. That is only half of what SQLite actually holds: a `Field.datetime` - * declared through `initObjects` becomes INTEGER epoch **milliseconds** (knex - * binds a JS `Date` as `.getTime()`), and `strftime` reads a bare integer as a - * Julian day number — epoch ms is orders of magnitude outside the legal range, - * so every row bucketed as NULL and any datetime trend chart rendered as one - * `(null)` bar carrying the whole total. + * TEXT. That was only half of what SQLite actually held: a `Field.datetime` + * declared through `initObjects` used to become INTEGER epoch **milliseconds** + * (knex binds a JS `Date` as `.getTime()`), and `strftime` reads a bare integer + * as a Julian day number — epoch ms is orders of magnitude outside the legal + * range, so every row bucketed as NULL and any datetime trend chart rendered as + * one `(null)` bar carrying the whole total. * - * So this suite goes through `driver.initObjects([...])` — the path a real - * object takes — and sweeps every supported granularity against BOTH storage - * forms, asserting against the same `bucketDateValue` labels the in-memory - * fallback produces. Anything that buckets differently depending on how the - * column happens to be stored is the bug this file exists to catch. + * Since #3912 writes are canonicalised, so a freshly written column is uniform + * TEXT — but the epoch form still sits in every database written by an earlier + * build, so both must keep bucketing identically. This suite goes through + * `driver.initObjects([...])` — the path a real object takes — and sweeps every + * supported granularity against BOTH storage forms, asserting against the same + * `bucketDateValue` labels the in-memory fallback produces. Anything that + * buckets differently depending on how the column happens to be stored is the + * bug this file exists to catch. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; type Granularity = 'day' | 'month' | 'quarter' | 'year'; @@ -115,7 +119,7 @@ describe('SqlDriver date bucketing is storage-form independent (#3773)', () => { { name: TABLE, fields: { - closed_at: { type: 'datetime' }, // INTEGER epoch ms under better-sqlite3 + closed_at: { type: 'datetime' }, // canonical ISO-Z TEXT since #3912 closed_on: { type: 'date' }, // YYYY-MM-DD TEXT amount: { type: 'number' }, }, @@ -137,20 +141,25 @@ describe('SqlDriver date bucketing is storage-form independent (#3773)', () => { await driver.disconnect(); }); - it('really does store the two columns in the two different forms', async () => { - // The premise of this whole file. If better-sqlite3 ever stops binding a - // `Date` as an integer, this fails first and explains the rest. + it('stores both temporal columns as canonical text (#3912)', async () => { + // The premise of this whole file. Since #3912 `formatInput` canonicalises a + // `Field.datetime` write to `YYYY-MM-DDTHH:MM:SS.sssZ`, so a bound `Date` no + // longer lands as an INTEGER epoch — the storage form is now the one + // `Field.date` already used, with a time part. The legacy epoch form still + // exists in un-migrated databases and is covered by the MIXED-form suite + // below, which writes it the only way it can still be produced: raw SQL. const res: any = await driver.execute( - `SELECT typeof("closed_at") AS at_t, typeof("closed_on") AS on_t FROM "${TABLE}" WHERE id = 'r3'`, + `SELECT typeof("closed_at") AS at_t, "closed_at" AS at_v, typeof("closed_on") AS on_t FROM "${TABLE}" WHERE id = 'r3'`, ); const row = Array.isArray(res) ? res[0] : (res?.rows?.[0] ?? res); - expect(['integer', 'real']).toContain(row.at_t); + expect(row.at_t).toBe('text'); + expect(row.at_v).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); expect(row.on_t).toBe('text'); }); for (const g of GRANULARITIES) { describe(`granularity '${g}'`, () => { - it('buckets the epoch-stored datetime column', async () => { + it('buckets the datetime column', async () => { expect(await bucketSums(driver, 'closed_at', g)).toEqual(expectedBuckets(g)); }); @@ -179,17 +188,23 @@ describe('SqlDriver date bucketing is storage-form independent (#3773)', () => { }); describe('SqlDriver date bucketing over a MIXED-form datetime column (#3773)', () => { - // One SQLite `Field.datetime` column legitimately holds both forms at once: - // `formatInput` leaves datetime values alone, so a `Date` lands as INTEGER - // epoch ms while an ISO string (what an unresolved `defaultValue: 'NOW()'` - // slot and any string-valued write produce) lands as TEXT. A bucket - // expression that assumed epoch for the whole column would divide the TEXT by - // 1000 — `'2026-01-10T…' / 1000.0` is 2.026 seconds past the epoch — and file - // live rows under 1970, which is worse than the NULL it replaced. - let driver: SqlDriver; + // A legacy, UN-MIGRATED database: before #3912 `formatInput` left datetime + // values alone, so a `Date` landed as INTEGER epoch ms while an ISO string + // (an unresolved `defaultValue: 'NOW()'` slot, any string-valued write) landed + // as TEXT — in the same column. A bucket expression that assumed epoch for the + // whole column would divide the TEXT by 1000 — `'2026-01-10T…' / 1000.0` is + // 2.026 seconds past the epoch — and file live rows under 1970, which is worse + // than the NULL it replaced. + // + // Writes are canonical now, so the only way to reach this state is a database + // whose backfill has not run: rows inserted raw, with the column's canonical + // marker cleared. That is exactly the state `backfillCanonicalDatetimes` logs + // and swallows into when it cannot run — where queries must stay CORRECT via + // the read-side repair, just unindexed. + let driver: LegacyStorageDriver; beforeEach(async () => { - driver = new SqlDriver({ + driver = new LegacyStorageDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, @@ -197,10 +212,12 @@ describe('SqlDriver date bucketing over a MIXED-form datetime column (#3773)', ( await driver.initObjects([ { name: TABLE, fields: { closed_at: { type: 'datetime' }, amount: { type: 'number' } } }, ]); - await driver.create(TABLE, { id: 'int', closed_at: new Date('2026-01-10T09:00:00Z'), amount: 1 }, { bypassTenantAudit: true }); - await driver.create(TABLE, { id: 'txt', closed_at: '2026-02-14T09:00:00Z', amount: 2 }, { bypassTenantAudit: true }); - await driver.create(TABLE, { id: 'naive', closed_at: '2026-02-20 09:00:00', amount: 4 }, { bypassTenantAudit: true }); - await driver.create(TABLE, { id: 'nil', closed_at: null, amount: 8 }, { bypassTenantAudit: true }); + await driver.seedLegacyRows(TABLE, 'closed_at', [ + { id: 'int', closed_at: Date.parse('2026-01-10T09:00:00Z'), amount: 1 }, // INTEGER epoch ms + { id: 'txt', closed_at: '2026-02-14T09:00:00Z', amount: 2 }, // zone-explicit TEXT + { id: 'naive', closed_at: '2026-02-20 09:00:00', amount: 4 }, // CURRENT_TIMESTAMP TEXT + { id: 'nil', closed_at: null, amount: 8 }, + ]); }); afterEach(async () => { diff --git a/packages/plugins/driver-sql/src/sql-driver-datetime-canonical-storage.test.ts b/packages/plugins/driver-sql/src/sql-driver-datetime-canonical-storage.test.ts new file mode 100644 index 0000000000..8ac02bb52b --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-canonical-storage.test.ts @@ -0,0 +1,211 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3912 — `Field.datetime` has ONE storage form: canonical UTC text + * (`YYYY-MM-DDTHH:MM:SS.sssZ`). + * + * The filter-level fix that came first (normalise the column in the comparison) + * made queries correct but left the disease: two write paths producing two + * shapes in one column. That still mis-ordered `ORDER BY` (#3928), still cost an + * unindexable expression on every window, and still meant storage disagreed with + * the value `find()` presents. + * + * So the write path canonicalises, the comparand canonicalises to the same + * function, and `backfillCanonicalDatetimes` converges existing rows at schema + * sync. This suite covers the three claims that buys: + * 1. every write shape lands as one canonical string; + * 2. the backfill converges a legacy database, and the read paths then drop + * their repair expression; + * 3. lexicographic order is chronological order, so ORDER BY is correct. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; + +const CANONICAL = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +const makeDriver = (Ctor: new (cfg: any) => T): T => + new Ctor({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + +const OBJECT = { + name: 'evt', + fields: { label: { type: 'string' }, at: { type: 'datetime' } }, +} as any; + +describe('Field.datetime writes land in ONE canonical form (#3912)', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = makeDriver(SqlDriver); + await driver.initObjects([OBJECT]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + const storedOf = async (id: string) => { + const res: any = await driver.execute(`select typeof(at) as t, at as v from evt where id = ?`, [id]); + const row = Array.isArray(res) ? res[0] : (res?.rows?.[0] ?? res); + return { type: row.t, value: row.v }; + }; + + it('folds every accepted input shape onto the same stored string', async () => { + const INSTANT = '2026-03-20T12:00:00.000Z'; + const inputs: Array<[string, unknown]> = [ + ['date-obj', new Date(INSTANT)], + ['iso-z', INSTANT], + ['iso-no-ms', '2026-03-20T12:00:00Z'], + ['offset', '2026-03-20T20:00:00+08:00'], // same instant, other offset + ['naive', '2026-03-20 12:00:00'], // wall clock IS UTC (ADR-0074) + ['epoch-num', Date.parse(INSTANT)], + ['epoch-str', String(Date.parse(INSTANT))], + ]; + for (const [id, at] of inputs) { + await driver.create('evt', { id, label: id, at }, { bypassTenantAudit: true }); + } + + for (const [id] of inputs) { + const stored = await storedOf(id); + expect(stored.type, `${id} stored form`).toBe('text'); + expect(stored.value, `${id} stored value`).toBe(INSTANT); + } + }); + + it('stores a bare calendar day as UTC midnight, not local midnight', async () => { + await driver.create('evt', { id: 'day', label: 'day', at: '2026-03-20' }, { bypassTenantAudit: true }); + expect((await storedOf('day')).value).toBe('2026-03-20T00:00:00.000Z'); + }); + + it('leaves an uninterpretable value alone rather than inventing an instant', async () => { + await driver.create('evt', { id: 'junk', label: 'junk', at: 'not-a-date' }, { bypassTenantAudit: true }); + expect((await storedOf('junk')).value).toBe('not-a-date'); + }); + + it('sorts chronologically as TEXT — lexicographic order IS time order (#3928)', async () => { + // The property that makes canonical UTC text the right choice over an epoch + // integer: ORDER BY needs no expression, so it can use an index, and a + // `Date`-written row can no longer sort ahead of an ISO-written one. + const instants = [ + ['e-1969', '1969-12-31T23:59:59.999Z'], + ['e-2025', '2025-11-15T09:00:00.000Z'], + ['e-2026a', '2026-01-10T09:00:00.000Z'], + ['e-2026b', '2026-06-30T23:59:59.000Z'], + ] as const; + // Alternate the write shape so a form-sensitive sort would interleave wrongly. + for (const [i, [id, iso]] of instants.entries()) { + await driver.create( + 'evt', { id, label: id, at: i % 2 === 0 ? new Date(iso) : iso }, { bypassTenantAudit: true }, + ); + } + + const rows = await driver.find('evt', { orderBy: [{ field: 'at', order: 'asc' }] }); + expect(rows.map((r: any) => r.id)).toEqual(instants.map(([id]) => id)); + + // …and the raw column sorts identically, i.e. the DB did the ordering. + const res: any = await driver.execute(`select id from evt order by at asc`); + const raw = Array.isArray(res) ? res : (res?.rows ?? []); + expect(raw.map((r: any) => r.id)).toEqual(instants.map(([id]) => id)); + }); + + it('emits a plain indexable comparison — no repair expression (#3912)', async () => { + // A table created in this process is canonical by construction, so + // `needsLegacyDatetimeRepair` is false and the SQL has no CASE in it. + const sql = (driver as any).knex('evt').where('at', '>=', '2026-01-01').toString(); + expect(sql).not.toContain('typeof'); + const compiled = (driver as any).knex.queryBuilder(); + (driver as any).applyFilters(compiled.table('evt'), { at: { $gte: '2026-01-01' } }); + expect(compiled.toString()).not.toContain('typeof'); + expect(compiled.toString()).toContain('2026-01-01T00:00:00.000Z'); + }); +}); + +describe('backfillCanonicalDatetimes converges a legacy database (#3912)', () => { + let driver: LegacyStorageDriver; + + beforeEach(async () => { + driver = makeDriver(LegacyStorageDriver); + await driver.initObjects([OBJECT]); + await driver.seedLegacyRows('evt', 'at', [ + { id: 'epoch', label: 'epoch', at: Date.parse('2026-03-20T12:00:00.000Z') }, + { id: 'naive', label: 'naive', at: '2026-03-20 12:00:00' }, + { id: 'offset', label: 'offset', at: '2026-03-20T20:00:00+08:00' }, + { id: 'canon', label: 'canon', at: '2026-03-20T12:00:00.000Z' }, + { id: 'junk', label: 'junk', at: 'not-a-date' }, + { id: 'nil', label: 'nil', at: null }, + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('rewrites every interpretable row to the canonical form', async () => { + await (driver as any).backfillCanonicalDatetimes('evt', true); + + const forms = Object.fromEntries( + (await driver.storedForms('evt', 'at')).map((r) => [r.id, r.value]), + ); + // All four interpretable shapes describe the same instant, so they converge + // on one byte-identical string. + expect(forms.epoch).toBe('2026-03-20T12:00:00.000Z'); + expect(forms.naive).toBe('2026-03-20T12:00:00.000Z'); + expect(forms.offset).toBe('2026-03-20T12:00:00.000Z'); + expect(forms.canon).toBe('2026-03-20T12:00:00.000Z'); + }); + + it('preserves what it cannot interpret instead of nulling it', async () => { + await (driver as any).backfillCanonicalDatetimes('evt', true); + const forms = Object.fromEntries( + (await driver.storedForms('evt', 'at')).map((r) => [r.id, r.value]), + ); + expect(forms.junk).toBe('not-a-date'); + expect(forms.nil).toBeNull(); + }); + + it('marks the column clean, so the read paths drop the repair expression', async () => { + expect((driver as any).needsLegacyDatetimeRepair('evt', 'at')).toBe(true); + expect((driver as any).filterColumnExpr('evt', 'at', 'at')).not.toBeNull(); + + await (driver as any).backfillCanonicalDatetimes('evt', true); + + expect((driver as any).needsLegacyDatetimeRepair('evt', 'at')).toBe(false); + expect((driver as any).filterColumnExpr('evt', 'at', 'at')).toBeNull(); + expect(driver.temporalFilterColumnSql('evt', 'at', '"at"')).toBe('"at"'); + }); + + it('is idempotent — a second run rewrites nothing', async () => { + await (driver as any).backfillCanonicalDatetimes('evt', true); + const before = await driver.storedForms('evt', 'at'); + driver.forgetCanonical('evt', 'at'); + await (driver as any).backfillCanonicalDatetimes('evt', true); + expect(await driver.storedForms('evt', 'at')).toEqual(before); + }); + + it('makes a window filter return the same rows before and after (#3912)', async () => { + // The repair keeps the un-migrated database CORRECT; the backfill only makes + // it fast. Both must agree, or the migration would be observable as a change + // in results — which is exactly what it must never be. + const window = { at: { $gte: '2026-03-20T00:00:00.000Z', $lte: '2026-03-21T00:00:00.000Z' } }; + const before = (await driver.find('evt', { where: window })).map((r: any) => r.id).sort(); + expect(before).toEqual(['canon', 'epoch', 'naive', 'offset']); + + await (driver as any).backfillCanonicalDatetimes('evt', true); + + const after = (await driver.find('evt', { where: window })).map((r: any) => r.id).sort(); + expect(after).toEqual(before); + }); + + it('runs automatically at schema sync', async () => { + // The fixture cleared the marker by hand; a real boot re-runs `initObjects`, + // which is where the backfill is wired in. + expect((driver as any).needsLegacyDatetimeRepair('evt', 'at')).toBe(true); + await driver.initObjects([OBJECT]); + expect((driver as any).needsLegacyDatetimeRepair('evt', 'at')).toBe(false); + for (const row of await driver.storedForms('evt', 'at')) { + if (row.id === 'junk' || row.id === 'nil') continue; + expect(String(row.value)).toMatch(CANONICAL); + } + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts b/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts new file mode 100644 index 0000000000..c3c2753b1e --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts @@ -0,0 +1,285 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression #3912 — the OTHER half of the SQLite `Field.datetime` storage mix. + * + * `sql-driver-datetime-filter.test.ts` covers rows written with a JS `Date`, + * which better-sqlite3 used to store as INTEGER epoch ms. But the REST / JSON + * write path cannot produce a `Date` (JSON has no such type) and a + * `defaultValue: 'NOW()'` slot stamps an ISO string, so a production table was + * dominated by ISO **TEXT** — and the filter path coerced its comparand to epoch + * ms purely from the DECLARED type. Every datetime window filter then compared + * INTEGER-vs-TEXT and returned nothing: a dashboard `last_30_days` on + * `created_date` read 0 while 29 rows matched. + * + * The fix made the storage form itself canonical, so these tests write the way + * REST does (ISO strings) and assert that every filter shape works against it — + * which is now the same shape a `Date` write produces. The legacy suite at the + * end covers the un-migrated database: one column holding both forms at once. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; + +const OBJECT = { + name: 'lead', + fields: { + name: { type: 'string' }, + created_date: { type: 'datetime' }, + closed_on: { type: 'date' }, + }, +} as any; + +describe('SqlDriver datetime filters on ISO-TEXT-stored columns (#3912)', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.initObjects([OBJECT]); + + // Written the way a REST/JSON create writes: ISO strings, which + // better-sqlite3 binds as TEXT. + await driver.create('lead', { + id: 'l1', name: 'Old', created_date: '2025-01-15T00:00:00.000Z', closed_on: '2025-01-15', + }, { bypassTenantAudit: true }); + await driver.create('lead', { + id: 'l2', name: 'New', created_date: '2026-03-20T12:00:00.000Z', closed_on: '2026-03-20', + }, { bypassTenantAudit: true }); + await driver.create('lead', { + id: 'l3', name: 'Newer', created_date: '2026-05-25T08:30:15.250Z', closed_on: '2026-05-25', + }, { bypassTenantAudit: true }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('stores the ISO write as TEXT (the premise these tests rest on)', async () => { + const rows: any = await (driver as any).knex.raw( + `select id, typeof(created_date) as t from lead order by id`, + ); + expect(rows.map((r: any) => r.t)).toEqual(['text', 'text', 'text']); + }); + + it('matches $gte against an ISO date string', async () => { + const rows = await driver.find('lead', { + where: { created_date: { $gte: '2026-01-01' } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(rows.map((r: any) => r.id)).toEqual(['l2', 'l3']); + }); + + it('matches a $gte / $lt window — the dashboard dateRange shape', async () => { + const rows = await driver.find('lead', { + where: { created_date: { $gte: '2026-01-01', $lt: '2026-05-01' } }, + }); + expect(rows.map((r: any) => r.id)).toEqual(['l2']); + }); + + it('matches a full ISO timestamp comparand, to the millisecond', async () => { + const inclusive = await driver.find('lead', { + where: { created_date: { $gte: '2026-05-25T08:30:15.250Z' } }, + }); + expect(inclusive.map((r: any) => r.id)).toEqual(['l3']); + + const exclusive = await driver.find('lead', { + where: { created_date: { $gt: '2026-05-25T08:30:15.250Z' } }, + }); + expect(exclusive).toEqual([]); + }); + + it('matches a JS Date comparand against a TEXT-stored row', async () => { + const rows = await driver.find('lead', { + where: { created_date: { $gte: new Date('2026-05-01T00:00:00Z') } }, + }); + expect(rows.map((r: any) => r.id)).toEqual(['l3']); + }); + + it('matches equality on the exact stored instant', async () => { + const rows = await driver.find('lead', { + where: { created_date: '2026-03-20T12:00:00.000Z' }, + }); + expect(rows.map((r: any) => r.id)).toEqual(['l2']); + }); + + it('matches $between, $in and $nin', async () => { + const between = await driver.find('lead', { + where: { created_date: { $between: ['2026-01-01', '2026-04-01'] } }, + }); + expect(between.map((r: any) => r.id)).toEqual(['l2']); + + const isIn = await driver.find('lead', { + where: { created_date: { $in: ['2026-03-20T12:00:00.000Z', '2026-05-25T08:30:15.250Z'] } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(isIn.map((r: any) => r.id)).toEqual(['l2', 'l3']); + + const notIn = await driver.find('lead', { + where: { created_date: { $nin: ['2026-03-20T12:00:00.000Z'] } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(notIn.map((r: any) => r.id)).toEqual(['l1', 'l3']); + }); + + it('matches $ne and the null predicates', async () => { + const ne = await driver.find('lead', { + where: { created_date: { $ne: '2026-03-20T12:00:00.000Z' } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(ne.map((r: any) => r.id)).toEqual(['l1', 'l3']); + + await driver.create('lead', { id: 'l4', name: 'Undated' }, { bypassTenantAudit: true }); + const missing = await driver.find('lead', { where: { created_date: { $null: true } } }); + expect(missing.map((r: any) => r.id)).toEqual(['l4']); + + const present = await driver.find('lead', { + where: { created_date: { $null: false } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(present.map((r: any) => r.id)).toEqual(['l1', 'l2', 'l3']); + }); + + it('matches the AST array filter form', async () => { + const rows = await driver.find('lead', { + where: [['created_date', '>=', '2026-01-01'], ['created_date', '<', '2026-05-01']], + } as any); + expect(rows.map((r: any) => r.id)).toEqual(['l2']); + }); + + it('matches inside an $or branch', async () => { + const rows = await driver.find('lead', { + where: { $or: [{ created_date: { $lt: '2025-06-01' } }, { name: 'Newer' }] }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(rows.map((r: any) => r.id)).toEqual(['l1', 'l3']); + }); + + it('leaves the Field.date column on its own YYYY-MM-DD rule', async () => { + const rows = await driver.find('lead', { + where: { closed_on: { $gte: '2026-01-01' } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(rows.map((r: any) => r.id)).toEqual(['l2', 'l3']); + }); + +}); + +describe('SqlDriver datetime filters on the created_at audit column (#3912)', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + // `withAuditFields` (objectql registry) declares `created_at` / `updated_at` + // as `datetime` on every audited object, so the driver sees them as declared + // datetime columns — exactly as spelled here. + await driver.initObjects([ + { + name: 'ticket', + fields: { + subject: { type: 'string' }, + created_at: { type: 'datetime' }, + updated_at: { type: 'datetime' }, + }, + }, + ] as any); + await driver.create('ticket', { id: 't1', subject: 'Stamped' }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('filters created_at even though the driver stamps it as ISO TEXT', async () => { + // The driver stamps the audit columns with `new Date().toISOString()` — never + // a `Date` — so they are TEXT on EVERY object in the system while being + // declared `datetime`. Filtering on them was unconditionally broken, not an + // edge case. + const stamped: any = await (driver as any).knex.raw( + `select typeof(created_at) as c, typeof(updated_at) as u from ticket`, + ); + expect(stamped[0]).toEqual({ c: 'text', u: 'text' }); + + const matched = await driver.find('ticket', { + where: { created_at: { $gte: '2000-01-01' } }, + }); + expect(matched.map((r: any) => r.id)).toEqual(['t1']); + + // …and a window that starts after the stamp excludes it, so the match above + // is a real comparison rather than "the predicate was dropped". + const future = await driver.find('ticket', { + where: { created_at: { $gte: '2999-01-01' } }, + }); + expect(future).toEqual([]); + }); +}); + +describe('SqlDriver datetime filters on a legacy MIXED-storage column (#3912)', () => { + let driver: LegacyStorageDriver; + + beforeEach(async () => { + driver = new LegacyStorageDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.initObjects([OBJECT]); + + // A database written by a pre-#3912 build: seeds/imports bound a `Date` + // (INTEGER epoch) while the REST API wrote ISO strings (TEXT) — into the same + // column. `formatInput` cannot produce this any more, and the backfill + // converges it at schema sync, so the fixture is seeded raw with the + // canonical marker cleared: the state of a database whose backfill has not + // run (it is allowed to fail — correctness must not depend on it). + await driver.seedLegacyRows('lead', 'created_date', [ + { id: 'int-old', name: 'int-old', created_date: Date.parse('2025-02-01T00:00:00Z') }, + { id: 'txt-old', name: 'txt-old', created_date: '2025-02-02T00:00:00.000Z' }, + { id: 'int-new', name: 'int-new', created_date: Date.parse('2026-06-01T00:00:00Z') }, + { id: 'txt-new', name: 'txt-new', created_date: '2026-06-02T00:00:00.000Z' }, + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('really does hold both storage forms', async () => { + const forms = await driver.storedForms('lead', 'created_date'); + expect(Object.fromEntries(forms.map((r) => [r.id, r.type]))).toEqual({ + 'int-new': 'integer', 'int-old': 'integer', 'txt-new': 'text', 'txt-old': 'text', + }); + }); + + it('returns rows of BOTH forms from one window filter', async () => { + const rows = await driver.find('lead', { + where: { created_date: { $gte: '2026-01-01' } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(rows.map((r: any) => r.id)).toEqual(['int-new', 'txt-new']); + }); + + it('excludes rows of BOTH forms that fall outside the window', async () => { + const rows = await driver.find('lead', { + where: { created_date: { $lt: '2026-01-01' } }, + orderBy: [{ field: 'id', order: 'asc' }], + }); + expect(rows.map((r: any) => r.id)).toEqual(['int-old', 'txt-old']); + }); + + it('counts rows of both forms in an aggregate window', async () => { + const rows: any = await driver.aggregate('lead', { + aggregations: [{ function: 'count', alias: 'n' }], + where: { created_date: { $gte: '2026-01-01' } }, + } as any); + // 4 means the window was dropped; 1 means only one storage form matched. + expect(Number(rows[0].n)).toBe(2); + }); +}); 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 new file mode 100644 index 0000000000..08b4f0a3fd --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts @@ -0,0 +1,273 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3942 — `Field.datetime` on MySQL. + * + * MySQL was the dialect the #3912 canonical-storage work did not reach, and it + * was the worst off of the three. Measured on MariaDB 10.11 before this change: + * + * - An ISO-8601 comparand or write value FAILED the statement outright — + * MySQL accepts neither the `T` separator nor the `Z` suffix in a datetime + * literal, so every REST/JSON write of a datetime errored with *Incorrect + * datetime value*, and canonicalising the write path in #3912 extended that + * to `Date` writes too. + * - A `Date` that did land was truncated to whole seconds: `table.timestamp` + * emits MySQL `TIMESTAMP`, which carries no fractional digits by default. + * - A zone-naive string landed at the wrong instant — 4 hours off with the + * process on `America/New_York` — because mysql2 renders and parses in the + * HOST's zone unless told otherwise. + * - Nothing outside 1970..2038 could be stored at all: `TIMESTAMP` is a 32-bit + * epoch. A contract end date in 2040 was simply rejected. + * + * The fix is three coordinated pieces, all asserted below: `DATETIME(3)` instead + * of `TIMESTAMP`, a connection pinned to UTC on both the mysql2 and the server + * layer, and a MySQL-spelled bind carrying the same UTC wall clock. + * + * Opt-in — needs a real server, which CI does not provision: + * + * OS_TEST_MYSQL_URL=mysql://root@127.0.0.1:3306/test pnpm --filter @objectstack/driver-sql test + * + * Point it at a server whose `time_zone` is NOT UTC (e.g. `default_time_zone = + * '+08:00'`) and run with a non-UTC `TZ`; the suite asserts the server side and + * tells you if it cannot prove anything. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +const URL = process.env.OS_TEST_MYSQL_URL; +const TABLE = 'os3942_probe'; + +const MIDDAY = '2026-03-20T12:34:56.789Z'; +/** 20:00Z is 04:00 the NEXT day at +08:00 — so a day window discriminates. */ +const BOUNDARY = '2026-03-20T20:00:00.000Z'; + +/** A driver whose connection this suite does NOT pin, to read raw server state. */ +const rawDriver = () => new SqlDriver({ client: 'mysql2', connection: URL }); + +describe.skipIf(!URL)('Field.datetime on MySQL (#3942)', () => { + let driver: SqlDriver; + let serverTimeZone = ''; + + beforeAll(async () => { + const probe = rawDriver(); + const rows = await rowsOf(probe, `select @@global.time_zone as tz`); + serverTimeZone = String((rows[0] as any).tz); + await probe.disconnect(); + }); + + beforeEach(async () => { + driver = new SqlDriver({ client: 'mysql2', connection: URL }); + await driver.execute(`drop table if exists ${TABLE}`); + await driver.initObjects([ + { name: TABLE, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, + ]); + }); + + afterEach(async () => { + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.disconnect(); + }); + + it('is pointed at a non-UTC server (otherwise this suite proves less)', () => { + expect( + serverTimeZone, + "set default_time_zone='+08:00' — on a UTC server the timezone bugs are invisible", + ).not.toMatch(/^(UTC|\+00:00|SYSTEM)$/i); + }); + + it('creates DATETIME(3), not TIMESTAMP — no 2038 ceiling, milliseconds kept', async () => { + const cols = await columnTypes(driver, TABLE); + expect(cols.at).toBe('datetime(3)'); + // The audit columns are declared `Field.datetime` by the objectql registry + // and are what most list views sort by, so they take the same type. + expect(cols.created_at).toBe('datetime(3)'); + expect(cols.updated_at).toBe('datetime(3)'); + }); + + it('pins the session to UTC, so the server timezone stops participating', async () => { + const rows = await rowsOf(driver, `select @@session.time_zone as tz`); + expect(String((rows[0] as any).tz)).toBe('+00:00'); + }); + + it('accepts an ISO write at all — it used to fail the statement', async () => { + // The headline regression: a REST/JSON payload carries an ISO string, and + // MySQL rejects it verbatim. This asserts the write completes; the next test + // asserts it landed on the right instant. + await expect( + driver.create(TABLE, { id: 'iso', label: 'iso', at: MIDDAY }, { bypassTenantAudit: true }), + ).resolves.toBeDefined(); + }); + + it('records the same instant for a Date, an ISO-Z string and a NAIVE string', async () => { + await driver.create(TABLE, { id: 'date-obj', label: 'a', at: new Date(MIDDAY) }, { bypassTenantAudit: true }); + await driver.create(TABLE, { id: 'iso-z', label: 'b', at: MIDDAY }, { bypassTenantAudit: true }); + await driver.create(TABLE, { id: 'naive', label: 'c', at: '2026-03-20 12:34:56.789' }, { bypassTenantAudit: true }); + await driver.create(TABLE, { id: 'offset', label: 'd', at: '2026-03-20T20:34:56.789+08:00' }, { bypassTenantAudit: true }); + + const expected = Date.parse(MIDDAY); + for (const id of ['date-obj', 'iso-z', 'naive', 'offset']) { + expect(await storedEpochMs(driver, id), `${id} stored instant`).toBe(expected); + } + }); + + it('keeps milliseconds — TIMESTAMP silently truncated them to whole seconds', async () => { + await driver.create(TABLE, { id: 'ms', label: 'ms', at: MIDDAY }, { bypassTenantAudit: true }); + expect(await storedEpochMs(driver, 'ms')).toBe(Date.parse(MIDDAY)); + expect(Date.parse(MIDDAY) % 1000).toBe(789); // the fixture would be vacuous otherwise + }); + + it('stores instants outside the 1970..2038 TIMESTAMP range', async () => { + // Asserted through the driver's own read rather than `unix_timestamp()`, + // which is itself a 32-bit epoch function and returns 0 for exactly the + // instants this test is about. + for (const [id, iso] of [['future', '2040-06-01T00:00:00.000Z'], ['past', '1960-06-01T00:00:00.000Z']] as const) { + await driver.create(TABLE, { id, label: id, at: iso }, { bypassTenantAudit: true }); + expect(await readInstant(driver, id), id).toBe(iso); + } + }); + + it('anchors a bare calendar-day comparand to UTC midnight, like the other dialects', async () => { + await driver.create(TABLE, { id: 'b1', label: 'x', at: BOUNDARY }, { bypassTenantAudit: true }); + const day = async (from: string, to: string) => + (await driver.find(TABLE, { where: { at: { $gte: from, $lt: to } } })).map((r: any) => r.id); + + // 20:00Z belongs to 2026-03-20 in UTC; on a `+08:00` server read as local + // midnight it would fall on the 21st — the divergence #3912 measured on PG. + expect(await day('2026-03-20', '2026-03-21')).toEqual(['b1']); + expect(await day('2026-03-21', '2026-03-22')).toEqual([]); + }); + + it('round-trips the instant through a read', async () => { + await driver.create(TABLE, { id: 'r', label: 'r', at: MIDDAY }, { bypassTenantAudit: true }); + const row: any = await driver.findOne(TABLE, 'r', { bypassTenantAudit: true }); + expect(new Date(row.at).toISOString()).toBe(MIDDAY); + }); +}); + +describe.skipIf(!URL)('MySQL TIMESTAMP → DATETIME(3) migration (#3942)', () => { + const LEGACY = 'os3942_legacy'; + const GOOD = '2026-03-20T12:34:56.000Z'; + let driver: SqlDriver; + + beforeEach(async () => { + // Build the table the way a pre-#3942 build did: TIMESTAMP columns, rows in + // it. The legacy connection is pinned to UTC so the fixture's instants are + // the ones we intend — the migration's job is to preserve them, not to + // second-guess what an older, timezone-ambiguous writer recorded. + 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'`); + await legacy.execute(`insert into ${LEGACY} (id, label, at) values (?, ?, ?)`, [ + 'old', 'old', '2026-03-20 12:34:56', + ]); + 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('widens the columns at schema sync without moving any instant', async () => { + const before = await storedEpochMs(driver, 'old', LEGACY); + expect((await columnTypes(driver, LEGACY)).at).toBe('timestamp'); + + await driver.initObjects([ + { name: LEGACY, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, + ]); + + const cols = await columnTypes(driver, LEGACY); + expect(cols.at).toBe('datetime(3)'); + expect(cols.created_at).toBe('datetime(3)'); + expect(cols.updated_at).toBe('datetime(3)'); + // The instant is what must not move. A migration that silently reinterprets + // stored data is worse than one that never runs. + expect(await storedEpochMs(driver, 'old', LEGACY)).toBe(before); + expect(before).toBe(Date.parse(GOOD)); + }); + + it('lets the migrated column hold what TIMESTAMP could not', async () => { + await driver.initObjects([ + { name: LEGACY, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, + ]); + await driver.create(LEGACY, { id: 'future', label: 'f', at: '2040-06-01T00:00:00.000Z' }, { bypassTenantAudit: true }); + expect(await readInstant(driver, 'future', LEGACY)).toBe('2040-06-01T00:00:00.000Z'); + }); + + it('is idempotent — a second sync leaves the schema alone', async () => { + const shape = { name: LEGACY, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }; + await driver.initObjects([shape]); + const first = await columnTypes(driver, LEGACY); + await driver.initObjects([shape]); + expect(await columnTypes(driver, LEGACY)).toEqual(first); + }); + + it('keeps the audit default, so inserts do not start writing NULL', async () => { + await driver.initObjects([ + { name: LEGACY, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, + ]); + await driver.create(LEGACY, { id: 'fresh', label: 'f' }, { bypassTenantAudit: true }); + const row: any = await driver.findOne(LEGACY, 'fresh', { bypassTenantAudit: true }); + expect(row.created_at ?? null, 'created_at must still default').not.toBeNull(); + }); +}); + +// ── helpers ───────────────────────────────────────────────────────────────── + +/** mysql2 hands back `[rows, fields]`; normalise to just the rows. */ +async function rowsOf(driver: SqlDriver, sql: string, bindings: unknown[] = []): Promise { + const res: any = await driver.execute(sql, bindings as any); + if (Array.isArray(res) && Array.isArray(res[0])) return res[0]; + return Array.isArray(res) ? res : (res?.rows ?? []); +} + +/** column → its full MySQL type, lower-cased and case-insensitive on the key. */ +async function columnTypes(driver: SqlDriver, table: string): Promise> { + const rows = await rowsOf( + driver, + `select column_name, column_type from information_schema.columns + where table_schema = database() and table_name = ?`, + [table], + ); + const out: Record = {}; + for (const r of rows as any[]) { + out[String(r.COLUMN_NAME ?? r.column_name)] = String(r.COLUMN_TYPE ?? r.column_type).toLowerCase(); + } + return out; +} + +/** + * The instant the driver presents for a row, as canonical ISO. The user-facing + * contract, and the only check that works outside 1970..2038 — `unix_timestamp()` + * is itself a 32-bit epoch function and returns 0 there. + */ +async function readInstant(driver: SqlDriver, id: string, table = TABLE): Promise { + const row: any = await driver.findOne(table, id, { bypassTenantAudit: true }); + return new Date(row.at).toISOString(); +} + +/** + * The stored instant as epoch ms, read under an explicitly-UTC session so the + * assertion cannot be satisfied by a compensating session-timezone error. + */ +async function storedEpochMs(driver: SqlDriver, id: string, table = TABLE): Promise { + await driver.execute(`set time_zone = '+00:00'`); + const rows = await rowsOf( + driver, + `select cast(unix_timestamp(at) * 1000 as signed) as ms from ${table} where id = ?`, + [id], + ); + return Number((rows[0] as any).ms); +} diff --git a/packages/plugins/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts b/packages/plugins/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts new file mode 100644 index 0000000000..6d62c1a4e9 --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts @@ -0,0 +1,121 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3912 on Postgres — the half that is NOT about storage form. + * + * SQLite's mixed INTEGER/TEXT storage is dialect-specific: `Field.datetime` maps + * to a real `timestamptz` here (knex's `table.timestamp` defaults `useTz: true`), + * so a `Date` and an ISO-`Z` string always resolved to the same instant. What + * Postgres HAD instead was a timezone bug of the same family, and it was quieter: + * + * - A zone-NAIVE value bound into `timestamptz` is resolved against the + * SERVER's `TimeZone`, not UTC. On an `Asia/Shanghai` server + * `'2026-03-20 12:00:00'` was stored as `2026-03-20T04:00:00Z` — 8 hours off + * the instant SQLite records for the same write (ADR-0053/ADR-0074: a naive + * wall clock IS UTC). + * - The filter comparand was passed through untouched, so a bare `YYYY-MM-DD` + * from a `{30_days_ago}` token meant midnight in the SERVER's timezone. The + * identical query over the identical instant put a row on a different + * calendar day than it did on SQLite — a silently wrong window, not an empty + * one. + * + * `canonicalUtcDatetime` closes both: every write and every comparand carries an + * explicit `Z`, so the server's timezone stops participating in the answer. + * + * Opt-in — needs a real server, which CI does not provision: + * + * OS_TEST_POSTGRES_URL=postgres://user@host:5432/db pnpm --filter @objectstack/driver-sql test + * + * Point it at a server whose `TimeZone` is NOT UTC to actually exercise the bug; + * the suite asserts that and tells you if it cannot. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +const URL = process.env.OS_TEST_POSTGRES_URL; +const TABLE = 'os3912_probe'; + +/** 20:00Z is 04:00 the NEXT day at +08:00 — so a day window discriminates. */ +const BOUNDARY = '2026-03-20T20:00:00.000Z'; +const MIDDAY = '2026-03-20T12:00:00.000Z'; + +describe.skipIf(!URL)('Field.datetime on Postgres is timezone-independent (#3912)', () => { + let driver: SqlDriver; + let serverTimeZone = ''; + + beforeAll(async () => { + const probe = new SqlDriver({ client: 'pg', connection: URL }); + const res: any = await probe.execute(`select current_setting('TimeZone') as tz`); + serverTimeZone = ((res?.rows ?? res)[0] as any).tz; + await probe.disconnect(); + }); + + beforeEach(async () => { + driver = new SqlDriver({ client: 'pg', connection: URL }); + await driver.execute(`drop table if exists "${TABLE}" cascade`); + await driver.initObjects([ + { name: TABLE, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, + ]); + }); + + afterEach(async () => { + await driver.execute(`drop table if exists "${TABLE}" cascade`).catch(() => {}); + await driver.disconnect(); + }); + + /** Epoch ms of the stored instant, read straight out of Postgres. */ + const storedEpochMs = async (id: string): Promise => { + const res: any = await driver.execute( + `select (extract(epoch from at) * 1000)::bigint as ms from "${TABLE}" where id = ?`, + [id], + ); + return Number(((res?.rows ?? res)[0] as any).ms); + }; + + it('is pointed at a non-UTC server (otherwise this suite proves nothing)', () => { + expect( + serverTimeZone, + 'set the server TimeZone to something like Asia/Shanghai — on UTC the bug is invisible', + ).not.toMatch(/^(UTC|Etc\/UTC|GMT)$/i); + }); + + it('maps Field.datetime to timestamptz, not a naive timestamp', async () => { + const res: any = await driver.execute( + `select data_type from information_schema.columns where table_name = ? and column_name = 'at'`, + [TABLE], + ); + expect(((res?.rows ?? res)[0] as any).data_type).toBe('timestamp with time zone'); + }); + + it('records the same instant for a Date, an ISO-Z string and a NAIVE string', async () => { + await driver.create(TABLE, { id: 'date-obj', label: 'a', at: new Date(MIDDAY) }, { bypassTenantAudit: true }); + await driver.create(TABLE, { id: 'iso-z', label: 'b', at: MIDDAY }, { bypassTenantAudit: true }); + // The one that used to be resolved in the server's timezone. + await driver.create(TABLE, { id: 'naive', label: 'c', at: '2026-03-20 12:00:00' }, { bypassTenantAudit: true }); + + const expected = Date.parse(MIDDAY); + for (const id of ['date-obj', 'iso-z', 'naive']) { + expect(await storedEpochMs(id), `${id} stored instant`).toBe(expected); + } + }); + + it('anchors a bare calendar-day comparand to UTC midnight, like SQLite', async () => { + await driver.create(TABLE, { id: 'b1', label: 'x', at: new Date(BOUNDARY) }, { bypassTenantAudit: true }); + + const day = async (from: string, to: string) => + (await driver.find(TABLE, { where: { at: { $gte: from, $lt: to } } })).map((r: any) => r.id); + + // 20:00Z belongs to 2026-03-20 in UTC. Read against the server's local + // midnight (Asia/Shanghai) it would fall on the 21st instead — which is + // exactly how the same dashboard window disagreed with SQLite. + expect(await day('2026-03-20', '2026-03-21')).toEqual(['b1']); + expect(await day('2026-03-21', '2026-03-22')).toEqual([]); + }); + + it('presents the stored instant as canonical UTC on read', async () => { + await driver.create(TABLE, { id: 'r', label: 'r', at: '2026-03-20 12:00:00' }, { bypassTenantAudit: true }); + const row: any = await driver.findOne(TABLE, 'r', { bypassTenantAudit: true }); + expect(new Date(row.at).toISOString()).toBe(MIDDAY); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts b/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts index 1215eb23a6..ec7a941d6a 100644 --- a/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts @@ -2,11 +2,22 @@ /** * Dialect-correctness of `temporalFilterValue` (the hook the analytics layer - * uses). The datetime → epoch-ms coercion is SQLite-ONLY: SQLite stores - * `Field.datetime` as an INTEGER epoch, but Postgres/MySQL map it to a native - * TIMESTAMP where an ISO string / Date binds correctly. Coercing to an epoch - * integer on a native-timestamp dialect would compare INTEGER vs TIMESTAMP and - * break the query — the exact Postgres regression we must NOT introduce. + * uses). + * + * Since #3912 a `Field.datetime` comparand takes ONE rule on every dialect: + * canonical UTC ISO — the same form `formatInput` now writes. It replaced a + * SQLite-only coercion to epoch ms, which assumed a storage form the write path + * never actually guaranteed. + * + * Making it uniform is a FIX on Postgres/MySQL too, not just a simplification. + * The comparand used to be passed through untouched there, so a bare + * `YYYY-MM-DD` from a `{30_days_ago}` token meant midnight in the SERVER's + * timezone — measured against a real PostgreSQL 16 on `Asia/Shanghai`, the same + * instant landed on a different calendar day than it did on SQLite. Stating the + * `Z` removes the server's timezone from the comparison. + * + * What must NOT come back is the epoch integer: binding one against a native + * TIMESTAMP compares INTEGER vs TIMESTAMP and breaks the query outright. * * No DB connection is needed: we seed the field-type maps the way `initObjects` * would and exercise the pure coercion logic across dialects. @@ -23,6 +34,10 @@ class ProbeDriver extends SqlDriver { seedDate(table: string, field: string): void { (this.dateFields[table] ??= new Set()).add(field); } + /** Mark a column as backfilled, the way `backfillCanonicalDatetimes` does. */ + markCanonical(table: string, field: string): void { + (this.canonicalDatetimeFields[table] ??= new Set()).add(field); + } } function makeDriver(client: string): ProbeDriver { @@ -31,25 +46,77 @@ function makeDriver(client: string): ProbeDriver { } const ISO = '2025-06-18'; -const EPOCH = Date.parse('2025-06-18T00:00:00.000Z'); +const CANONICAL = '2025-06-18T00:00:00.000Z'; +/** The same UTC wall clock, spelled the only way MySQL parses it (#3942). */ +const MYSQL_LITERAL = '2025-06-18 00:00:00.000'; + +/** What each dialect physically binds for one canonical instant. */ +const PHYSICAL: Record = { + 'better-sqlite3': CANONICAL, + pg: CANONICAL, + mysql2: MYSQL_LITERAL, +}; describe('temporalFilterValue dialect gating', () => { - it('SQLite: datetime ISO comparand → epoch ms', () => { - const d = makeDriver('better-sqlite3'); + it('every dialect anchors a bare calendar day to UTC midnight (#3912)', () => { + // The SEMANTICS are uniform — one instant, midnight UTC — even though MySQL + // spells it differently. On Postgres this is the fix for the measured + // cross-dialect divergence: an un-anchored '2025-06-18' was read as midnight + // in the server's timezone, so a window put the same instant on a different + // day than SQLite did. + for (const [client, expected] of Object.entries(PHYSICAL)) { + const d = makeDriver(client); + d.seedDatetime('t', 'at'); + expect(d.temporalFilterValue('t', 'at', ISO), client).toBe(expected); + } + }); + + it('MySQL gets the same instant in a MySQL-parseable literal (#3942)', () => { + // MySQL rejects both the `T` separator and the `Z` suffix — an ISO-8601 + // comparand fails the statement outright with *Incorrect datetime value* + // (measured on MariaDB 10.11), so the canonical form has to be respelled for + // the bind. It is the SAME UTC wall clock: the column is `DATETIME(3)`, which + // does no timezone conversion, and the connection is pinned to UTC. + const d = makeDriver('mysql2'); d.seedDatetime('t', 'at'); - expect(d.temporalFilterValue('t', 'at', ISO)).toBe(EPOCH); + expect(d.temporalFilterValue('t', 'at', ISO)).toBe(MYSQL_LITERAL); + expect(d.temporalFilterValue('t', 'at', '2025-06-18T08:00:00+08:00')).toBe(MYSQL_LITERAL); + expect(d.temporalFilterValue('t', 'at', new Date(CANONICAL))).toBe(MYSQL_LITERAL); + // Never ISO — that is precisely what MySQL cannot parse. + expect(String(d.temporalFilterValue('t', 'at', CANONICAL))).not.toMatch(/[TZ]/); }); - it('Postgres: datetime ISO comparand is LEFT UNCHANGED (no epoch coercion → no regression)', () => { - const d = makeDriver('pg'); + it('never binds an epoch integer — that would break a native TIMESTAMP compare', () => { + for (const client of ['better-sqlite3', 'pg', 'mysql2']) { + const d = makeDriver(client); + d.seedDatetime('t', 'at'); + for (const input of [ISO, new Date(CANONICAL), Date.parse(CANONICAL), CANONICAL]) { + expect(typeof d.temporalFilterValue('t', 'at', input)).toBe('string'); + } + } + }); + + it('folds every accepted input shape onto the same canonical instant', () => { + const d = makeDriver('better-sqlite3'); d.seedDatetime('t', 'at'); - expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO); + for (const input of [ + new Date(CANONICAL), // JS Date + Date.parse(CANONICAL), // epoch ms number + String(Date.parse(CANONICAL)), // epoch ms as text + '2025-06-18', // bare calendar day + '2025-06-18 00:00:00', // zone-naive wall clock (CURRENT_TIMESTAMP) + '2025-06-18T00:00:00Z', // zone-explicit, no millis + '2025-06-18T08:00:00+08:00', // same instant, offset form + ]) { + expect(d.temporalFilterValue('t', 'at', input)).toBe(CANONICAL); + } }); - it('MySQL: datetime ISO comparand is left unchanged', () => { - const d = makeDriver('mysql2'); + it('leaves an uninterpretable comparand alone rather than inventing an instant', () => { + const d = makeDriver('better-sqlite3'); d.seedDatetime('t', 'at'); - expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO); + expect(d.temporalFilterValue('t', 'at', 'not-a-date')).toBe('not-a-date'); + expect(d.temporalFilterValue('t', 'at', '')).toBe(''); }); it('Field.date normalises to YYYY-MM-DD text on every dialect', () => { @@ -70,7 +137,7 @@ describe('temporalFilterValue dialect gating', () => { /** * The same dialect gating for the aggregate BUCKET expression (#3773). It shares - * `isEpochStoredDatetime` with the filter coercion above, so the two can only be + * `needsLegacyDatetimeRepair` with the filter path above, so the two can only be * wrong together — which is the point: a window and a bucket that disagree about * storage is how the epoch column ended up correctly filtered and then entirely * bucketed as NULL. @@ -87,26 +154,42 @@ describe('buildDateBucketExpr dialect gating (#3773)', () => { const expr = (d: ProbeDriver, field: string, g: string, table?: string) => (d as any).buildDateBucketExpr(field, g, table) as { sql: string; bindings: any[] } | null; - it('SQLite: a declared Field.datetime is normalised from epoch ms', () => { + it('SQLite: an UN-BACKFILLED Field.datetime is repaired to canonical text', () => { + // `seedDatetime` mimics `initObjects` declaring the column WITHOUT the + // backfill having marked it canonical — an un-migrated database. const d = makeDriver('better-sqlite3'); d.seedDatetime('t', 'at'); for (const g of GRANULARITIES) { const e = expr(d, 'at', g, 't')!; - expect(e.sql).toContain(`julianday(??/1000.0, 'unixepoch')`); + expect(e.sql).toContain(`strftime('%Y-%m-%dT%H:%M:%fZ', ??/1000.0, 'unixepoch')`); // Real division, not integer: `-1 / 1000` truncates to 0 and moves a // pre-1970 instant forward a day. expect(e.sql).not.toContain('/1000,'); } }); + it('SQLite: a BACKFILLED Field.datetime drops the repair entirely (#3912)', () => { + // The payoff of canonicalising storage: once a column is known clean the + // bucket expression is a bare column again, so it can use an index. + const d = makeDriver('better-sqlite3'); + d.seedDatetime('t', 'at'); + d.markCanonical('t', 'at'); + for (const g of GRANULARITIES) { + const e = expr(d, 'at', g, 't')!; + expect(e.sql).not.toContain('unixepoch'); + expect(e.sql).not.toContain('typeof'); + expect(e.bindings).toEqual(expect.arrayContaining(['at'])); + } + }); + it('SQLite: Field.date and undeclared columns keep the plain column form', () => { const d = makeDriver('better-sqlite3'); d.seedDate('t', 'on'); for (const g of GRANULARITIES) { - expect(expr(d, 'on', g, 't')!.sql).not.toContain('julianday'); - expect(expr(d, 'anything', g, 't')!.sql).not.toContain('julianday'); + expect(expr(d, 'on', g, 't')!.sql).not.toContain('unixepoch'); + expect(expr(d, 'anything', g, 't')!.sql).not.toContain('unixepoch'); // No table key at all (a caller outside the aggregate path) → plain form. - expect(expr(d, 'at', g)!.sql).not.toContain('julianday'); + expect(expr(d, 'at', g)!.sql).not.toContain('unixepoch'); } }); diff --git a/packages/plugins/driver-sql/src/sql-driver-user-datetime-default-format.test.ts b/packages/plugins/driver-sql/src/sql-driver-user-datetime-default-format.test.ts index 7590ec4476..f717ad0ed6 100644 --- a/packages/plugins/driver-sql/src/sql-driver-user-datetime-default-format.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-user-datetime-default-format.test.ts @@ -90,31 +90,38 @@ describe('User NOW()-default temporal fields — canonical format (SQLite)', () // ── Read presentation: mixed storage → one canonical instant ──────────────── - it('an explicit Date (stored as INTEGER epoch ms) reads back as canonical ISO-8601-Z', async () => { + it('an explicit Date is STORED canonical, not just presented canonical (#3912)', async () => { const when = new Date('2026-03-20T12:34:56.789Z'); await driver.create('event', { id: 'e3', label: 'C', starts_at: when }, { bypassTenantAudit: true }); - // Raw on disk is the INTEGER epoch (better-sqlite3 binds a Date as getTime()). + // On disk it used to be the INTEGER epoch (better-sqlite3 binds a Date as + // getTime()); `formatInput` now canonicalises the write, so storage and + // presentation are the same string. That equality is the whole point — it is + // what lets a window filter and an ORDER BY read the column directly. const rawRow = await raw('event').where('id', 'e3').first(); - expect(typeof rawRow.starts_at).toBe('number'); - expect(rawRow.starts_at).toBe(when.getTime()); + expect(typeof rawRow.starts_at).toBe('string'); + expect(rawRow.starts_at).toBe('2026-03-20T12:34:56.789Z'); - // …but formatOutput presents the canonical instant. const row: any = await driver.findOne('event', 'e3', { bypassTenantAudit: true }); - expect(typeof row.starts_at).toBe('string'); - expect(row.starts_at).toBe('2026-03-20T12:34:56.789Z'); + expect(row.starts_at).toBe(rawRow.starts_at); }); - it('CONSISTENT PRESENTATION: an explicit-Date row and a defaulted row both read back as ISO-Z, despite genuinely mixed on-disk storage', async () => { + it('CONSISTENT STORAGE: an explicit-Date row and a defaulted row land in the SAME form (#3912)', async () => { await driver.create('event', { id: 'explicit', label: 'X', starts_at: new Date('2026-01-02T03:04:05.006Z') }, { bypassTenantAudit: true }); await driver.create('event', { id: 'defaulted', label: 'Y' }, { bypassTenantAudit: true }); // omitted → DDL default - // On disk: one INTEGER, one TEXT — exactly the mixed storage the fix targets. + // This assertion is the inverse of the one it replaces. The two write paths + // used to produce INTEGER and TEXT in one column — the mixed storage that + // broke every window filter (#3912) and still mis-orders ORDER BY (#3928). + // `formatInput` and `nowColumnDefault` now agree on one shape. const rawRows = await raw('event').whereIn('id', ['explicit', 'defaulted']).select('id', 'starts_at'); - const onDiskTypes = new Set(rawRows.map((r: any) => typeof r.starts_at)); - expect(onDiskTypes).toEqual(new Set(['number', 'string'])); + expect(new Set(rawRows.map((r: any) => typeof r.starts_at))).toEqual(new Set(['string'])); + for (const r of rawRows as any[]) expect(r.starts_at).toMatch(ISO_Z); + + // Lexicographic order is chronological order — what fixed-width UTC buys. + const sorted = [...rawRows].sort((a: any, b: any) => String(a.starts_at).localeCompare(String(b.starts_at))); + expect(sorted.map((r: any) => r.id)).toEqual(['explicit', 'defaulted']); - // On read: uniform canonical ISO-Z, both parse to a real instant. for (const id of ['explicit', 'defaulted']) { const row: any = await driver.findOne('event', id, { bypassTenantAudit: true }); expect(row.starts_at).toMatch(ISO_Z); @@ -122,6 +129,18 @@ describe('User NOW()-default temporal fields — canonical format (SQLite)', () } }); + it('still repairs a LEGACY epoch row on read (un-migrated database)', async () => { + // Rows written by a pre-#3912 build are INTEGER epoch ms. The read-side + // repair that presented them canonically has not gone anywhere — a database + // whose backfill has not run must keep reading correctly. + await raw('event').insert({ id: 'legacy', label: 'L', starts_at: Date.parse('2026-03-20T12:34:56.789Z') }); + const rawRow = await raw('event').where('id', 'legacy').first(); + expect(typeof rawRow.starts_at).toBe('number'); + + const row: any = await driver.findOne('event', 'legacy', { bypassTenantAudit: true }); + expect(row.starts_at).toBe('2026-03-20T12:34:56.789Z'); + }); + it('an explicit ISO-8601-Z string is preserved (idempotent) on read', async () => { const iso = '2026-05-25T08:00:00.000Z'; await driver.create('event', { id: 'e4', label: 'D', starts_at: iso }, { bypassTenantAudit: true }); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 6e841af3f2..2c8f93d246 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -203,6 +203,103 @@ function normalizeSqliteDatetimeOutput(value: unknown): unknown { return repairNaiveUtcAuditTimestamp(s); } +/** + * The CANONICAL on-disk form of a `Field.datetime` value: a fixed-width, + * zone-explicit UTC instant — `YYYY-MM-DDTHH:MM:SS.sssZ` (`Date#toISOString`). + * + * ADR-0053 already declares `datetime` to be "an instant stored as UTC"; this is + * the function that makes the STORAGE match the declaration instead of leaving + * it to whatever the caller happened to pass. It is applied on write + * ({@link SqlDriver.formatInput}) and to filter comparands + * ({@link SqlDriver.coerceFilterValue}) so both sides of every comparison are + * the same shape, on every dialect. + * + * Why THIS form (#3912): + * - Fixed width + UTC means lexicographic order IS chronological order, so a + * SQLite TEXT column sorts and range-compares correctly *through an index* — + * no expression wrapper, which is what an epoch-integer convention forces. + * - `strftime`/`julianday` parse it directly, so the date-bucket expression + * needs no epoch↔text CASE (#3773). + * - It is what `formatOutput`/`normalizeSqliteDatetimeOutput` ALREADY present + * on read, so storage and presentation stop disagreeing. + * - It matches the `Field.date` convention (ISO TEXT), so the platform has one + * temporal storage story rather than one per field type. + * - Postgres parses it into `timestamptz` unambiguously, which is precisely + * what a zone-naive string does NOT do (it is read in the SERVER's + * timezone — an 8-hour shift on an Asia/Shanghai server). + * + * Distinct from {@link repairNaiveUtcAuditTimestamp}, which is deliberately + * idempotent on any zone-EXPLICIT string and so preserves a `+08:00` offset. + * That is right for a read repair and wrong for a storage canon: `'…T12:00+08:00'` + * and `'…T04:00Z'` are the same instant but sort differently as text. Everything + * lands in `Z` here. + * + * Total: `null`/`undefined`, empty strings and unparseable junk pass through + * untouched rather than becoming `Invalid Date` — a value the driver cannot + * interpret is never silently rewritten. + */ +function canonicalUtcDatetime(value: unknown): unknown { + if (value == null) return value; + if (value instanceof Date) { + return Number.isNaN(value.getTime()) ? value : value.toISOString(); + } + if (typeof value === 'number') { + if (!Number.isFinite(value)) return value; + const d = new Date(value); + return Number.isNaN(d.getTime()) ? value : d.toISOString(); + } + if (typeof value !== 'string') return value; + const s = value.trim(); + if (s === '') return value; + // A bare integer (in either JS or string form) is epoch milliseconds — the + // shape better-sqlite3 wrote for every `Date` bound before this convention. + if (/^-?\d+$/.test(s)) { + const d = new Date(Number(s)); + return Number.isNaN(d.getTime()) ? value : d.toISOString(); + } + // A bare calendar day means midnight UTC. Stated explicitly so it cannot be + // re-read as midnight in the server's local zone — the Postgres divergence + // where the same query lands a row on a different calendar day. + const iso = /^\d{4}-\d{2}-\d{2}$/.test(s) + ? `${s}T00:00:00.000Z` + // Zone-naive `YYYY-MM-DD[ T]HH:MM[:SS[.fff]]` → its wall-clock IS UTC, the + // same rule `CURRENT_TIMESTAMP`-written rows take on read (ADR-0074). + : /^\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}(:\d{2}(\.\d+)?)?$/.test(s) + ? `${s.replace(' ', 'T')}Z` + : s; + const ms = Date.parse(iso); + return Number.isFinite(ms) ? new Date(ms).toISOString() : value; +} + +/** + * The canonical instant rendered as a MySQL datetime literal — the same UTC wall + * clock, spelled the only way MySQL will parse it (#3942). + * + * MySQL/MariaDB reject the ISO-8601 the rest of the platform speaks: neither the + * `T` separator nor the `Z` suffix is accepted in a datetime literal, so + * `'2026-03-20T12:34:56.789Z'` fails the INSERT outright with *Incorrect datetime + * value* (measured on MariaDB 10.11). MySQL 8.0.19+ added `±HH:MM` offsets, but + * still not `Z`, and the platform supports older servers — so the offset is + * dropped and the value is stored as the UTC wall clock in a `DATETIME(3)` + * column, which does no timezone conversion of its own. + * + * This is a PHYSICAL spelling, not a semantic change: the column still holds the + * same instant, and every layer above the bind — API payloads, filter authoring, + * CEL — keeps the canonical `…Z` form. Reads convert back (the connection is + * pinned to UTC, so mysql2 reconstructs the instant correctly). + * + * Total, and deliberately strict: only an exactly-canonical string is rewritten. + * Anything else — an unparseable value `canonicalUtcDatetime` passed through, or + * a year outside MySQL's 1000..9999 range, which `toISOString` renders in + * expanded `+0YYYYY` form — is handed to MySQL untouched, so it fails loudly + * rather than being silently reinterpreted. + */ +function mysqlDatetimeLiteral(canonical: unknown): unknown { + if (typeof canonical !== 'string') return canonical; + const m = /^(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2}\.\d{3})Z$/.exec(canonical); + return m ? `${m[1]} ${m[2]}` : canonical; +} + // ── Introspection Types ────────────────────────────────────────────────────── export interface IntrospectedColumn { @@ -344,6 +441,13 @@ export class SqlDriver implements IDataDriver { protected numericFields: Record = {}; protected dateFields: Record> = {}; protected datetimeFields: Record> = {}; + /** + * SQLite `Field.datetime` columns proven to hold ONLY canonical UTC text — + * either backfilled by {@link backfillCanonicalDatetimes} or created empty in + * this process. Read by {@link needsLegacyDatetimeRepair} to drop the repair + * expression, so a migrated deployment gets plain indexable `col op ?` SQL. + */ + protected canonicalDatetimeFields: Record> = {}; protected timeFields: Record> = {}; /** * Federation read path (ADR-0015). For external objects whose physical @@ -674,7 +778,59 @@ export class SqlDriver implements IDataDriver { } // A function-valued `connection` (knex's per-acquire provider) is left // alone: the host is building each connection itself and owns its timeouts. - return bounded; + return SqlDriver.withUtcSession(bounded); + } + + /** + * Pin a MySQL connection to UTC, in both directions (#3942). + * + * MySQL is the one supported dialect where the SESSION timezone participates + * in what a datetime means, on two independent layers, and both default to + * something machine-dependent: + * + * - **mysql2** (`connection.timezone`, default `'local'`) decides how a bound + * JS `Date` is rendered and how a returned `DATETIME` string is parsed back. + * Left at `'local'`, two app servers in different zones write the same + * instant as different values, and read the same row as different instants. + * - **The server** (`@@session.time_zone`, default `SYSTEM`) decides how a + * zone-naive literal is interpreted for a `TIMESTAMP` column, and what + * `CURRENT_TIMESTAMP` renders. Measured at 8 hours off on a server + * configured `+08:00`. + * + * Setting both to UTC makes the wall clock the driver writes *be* the instant, + * which is what {@link mysqlDatetimeLiteral} relies on — and it keeps legacy + * `TIMESTAMP` columns correct too, so a deployment stays right whether or not + * the `DATETIME(3)` migration has run. + * + * A host that set either one explicitly is left alone; an existing + * `pool.afterCreate` is chained rather than replaced, since it is a documented + * knex extension point the host may already be using. + */ + private static withUtcSession(knexConfig: Record): Record { + const client = String(knexConfig.client ?? ''); + if (client !== 'mysql' && client !== 'mysql2') return knexConfig; + + const out: Record = { ...knexConfig }; + const conn = out.connection; + if (conn && typeof conn === 'object' && (conn as any).timezone === undefined) { + out.connection = { ...(conn as object), timezone: 'Z' }; + } + + const pool = (out.pool ?? {}) as Record; + const hostAfterCreate = pool.afterCreate as + | ((conn: unknown, done: (err?: unknown) => void) => void) + | undefined; + out.pool = { + ...pool, + afterCreate(connection: any, done: (err?: unknown, conn?: unknown) => void) { + connection.query(`SET time_zone = '+00:00'`, (err: unknown) => { + if (err) return done(err); + if (!hostAfterCreate) return done(undefined, connection); + hostAfterCreate(connection, (hostErr?: unknown) => done(hostErr, connection)); + }); + }, + }; + return out; } /** @@ -2086,8 +2242,8 @@ export class SqlDriver implements IDataDriver { if (!exists) { await this.knex.schema.createTable(shardName, (table) => { table.string('id').primary(); - table.timestamp('created_at').defaultTo(this.knex.fn.now()); - table.timestamp('updated_at').defaultTo(this.knex.fn.now()); + this.createAuditTimestampColumn(table, 'created_at'); + this.createAuditTimestampColumn(table, 'updated_at'); for (const [name, field] of Object.entries(obj.fields ?? {})) { if (builtinColumns.has(name)) continue; this.createColumn(table, name, field); @@ -2413,8 +2569,8 @@ export class SqlDriver implements IDataDriver { if (!exists) { await this.knex.schema.createTable(tableName, (table) => { table.string('id').primary(); - table.timestamp('created_at').defaultTo(this.knex.fn.now()); - table.timestamp('updated_at').defaultTo(this.knex.fn.now()); + this.createAuditTimestampColumn(table, 'created_at'); + this.createAuditTimestampColumn(table, 'updated_at'); if (obj.fields) { for (const [name, field] of Object.entries(obj.fields)) { if (builtinColumns.has(name)) continue; @@ -2476,6 +2632,13 @@ export class SqlDriver implements IDataDriver { if (exists) { await this.reconcileAndWarnDrift(tableName, obj.fields ?? {}, declaredIndexes); } + + // #3912: converge this table's `Field.datetime` columns on the canonical + // UTC-text storage form. A table this call just CREATED has no rows, so it + // is canonical by construction — record that without touching the disk. + await this.backfillCanonicalDatetimes(tableName, exists); + // #3942: the MySQL twin — widen legacy `TIMESTAMP` columns to `DATETIME(3)`. + if (exists) await this.migrateMysqlDatetimeColumns(tableName, obj.fields ?? {}); } // Pre-create the auto_number counter table now, while we hold a fresh pooled @@ -2494,6 +2657,154 @@ export class SqlDriver implements IDataDriver { } } + /** + * Converge one table's `Field.datetime` columns on the canonical UTC-text + * storage form (#3912), then mark them clean so the read paths can drop their + * repair expression. + * + * SQLite only, and this is the whole reason the convention is affordable. + * Postgres/MySQL store a real temporal type, so their rows are already one + * shape — there is nothing on disk to rewrite. (What Postgres CANNOT recover is + * an instant written from a zone-naive string before this change: it was + * resolved against the server's timezone at write time and the original wall + * clock is gone. `formatInput` stops producing those going forward; existing + * rows are simply the instants the server recorded.) + * + * ONE `UPDATE` per column, whose SET expression is the very same + * {@link sqliteCanonicalDatetimeSql} the read paths use — so "what canonical + * means" has a single definition and the migration cannot drift from the repair + * it retires. It converts every non-canonical shape in one pass: INTEGER/REAL + * epoch ms, zone-naive `CURRENT_TIMESTAMP` output, an offset-bearing `+08:00` + * value, a bare `YYYY-MM-DD`. + * + * `col IS NOT ` is the whole `WHERE`. `IS NOT` rather than `<>` + * because it is null-safe AND type-aware: an INTEGER value is never equal to + * the text the expression yields, so epoch rows match; an already-canonical + * string equals it exactly and is skipped. A value SQLite cannot parse falls + * through the expression's `coalesce` unchanged, compares equal to itself, and + * is left alone rather than destroyed. So a converged table costs one scan and + * zero writes, and re-running is a no-op. + * + * Failures are logged and swallowed: the column simply stays un-marked, the + * read paths keep their repair, and queries stay CORRECT (just unindexed). A + * migration that cannot run must never be able to take the process down at + * boot, and correctness must never be contingent on one having run. + */ + protected async backfillCanonicalDatetimes(table: string, tableExisted: boolean): Promise { + const fields = this.datetimeFields[table]; + if (!this.isSqlite || !fields || fields.size === 0) return; + + const clean = (this.canonicalDatetimeFields[table] ??= new Set()); + // A table created by this very call is empty, so every datetime column in it + // is canonical without a single row being read. + if (!tableExisted) { + for (const field of fields) clean.add(field); + return; + } + + const canonical = this.sqliteCanonicalDatetimeSql('??'); + // The expression spells `??` 4×, and the statement uses it twice (the SET + // value and the WHERE guard) — hence the column name repeated per use. + const exprBindings = (field: string) => [field, field, field, field]; + for (const field of fields) { + try { + const res = await this.knex.raw( + `update ?? set ?? = ${canonical} where ?? is not null and ?? is not ${canonical}`, + [table, field, ...exprBindings(field), field, field, ...exprBindings(field)], + ); + const converted = (res as any)?.changes ?? 0; + if (converted) { + this.logger.info?.( + `[sql-driver] canonicalised datetime storage (#3912) for ${table}.${field}`, + { rowsConverted: converted }, + ); + } + clean.add(field); + } catch (err) { + // Correctness does not depend on this succeeding — only performance does. + this.logger.warn( + `[sql-driver] could not canonicalise datetime storage for ${table}.${field}; ` + + `queries stay correct via the read-side repair`, + { error: err instanceof Error ? err.message : String(err) }, + ); + } + } + } + + /** + * Widen a table's legacy MySQL `TIMESTAMP` datetime columns to `DATETIME(3)` + * (#3942) — the MySQL counterpart of {@link backfillCanonicalDatetimes}. + * + * `TIMESTAMP` cannot hold an instant past 2038-01-19 or before 1970, keeps no + * milliseconds, and converts using the session timezone. `ALTER … MODIFY` moves + * the column to `DATETIME(3)`, which has none of those properties. The instants + * survive because the connection pins `@@session.time_zone` to UTC + * ({@link withUtcSession}): MySQL renders each `TIMESTAMP` in the session zone + * to produce the `DATETIME` wall clock, so UTC in gives the UTC wall clock the + * driver's own writes use. + * + * Only the audit columns and declared `Field.datetime` columns are touched, and + * only when they are still `timestamp` — so this is idempotent and re-running + * costs one `information_schema` lookup. + * + * Failures are logged and swallowed, like the SQLite backfill and for the same + * reason: a `TIMESTAMP` column keeps working (the driver binds the same UTC + * wall-clock literal either way, and the session is pinned to UTC), it merely + * keeps the range and precision limits. Correctness must not depend on a + * migration having run, and a migration must never take boot down. + */ + protected async migrateMysqlDatetimeColumns( + 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; + + 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)); + if (legacy.length === 0) return; + + for (const col of legacy) { + // The default is re-stated because MySQL drops a column's DEFAULT when + // MODIFY does not repeat it, and an audit column without + // `CURRENT_TIMESTAMP(3)` would start inserting NULL. + const isAudit = (AUDIT_TIMESTAMP_COLUMNS as readonly string[]).includes(col.name); + const nullClause = col.nullable ? 'null' : 'not null'; + const defaultClause = isAudit ? ' default current_timestamp(3)' : ''; + await this.knex.raw( + `alter table ?? modify column ?? datetime(3) ${nullClause}${defaultClause}`, + [table, col.name], + ); + } + this.logger.info?.( + `[sql-driver] widened MySQL TIMESTAMP → DATETIME(3) (#3942) on ${table}`, + { columns: legacy.map((c) => c.name) }, + ); + } catch (err) { + this.logger.warn( + `[sql-driver] could not widen MySQL datetime columns on ${table}; ` + + `writes stay correct, but the 2038 ceiling and millisecond truncation remain`, + { error: err instanceof Error ? err.message : String(err) }, + ); + } + } + // ── Deferred schema DDL (#3917) ──────────────────────────────────────────── /** @@ -3530,111 +3841,104 @@ export class SqlDriver implements IDataDriver { } /** - * Normalise a filter value for a single column so the comparison the - * driver sends to SQLite matches the on-disk representation. + * Put a filter comparand into the same canonical form the column is STORED in, + * so a comparison can never be decided by the two sides' shapes disagreeing. * - * The platform stores `Field.datetime()` values as INTEGER milliseconds - * (the result of passing a JS `Date` through better-sqlite3) but date - * macros like `{last_quarter_start}` expand to an ISO `YYYY-MM-DD` string - * client-side. Without coercion the SQL becomes `published_at >= '2026-…'` - * which collapses to a TEXT-vs-INTEGER affinity compare and never - * matches. We translate the ISO/Date/numeric inputs into the storage - * type so the comparison works. + * `Field.datetime` → canonical UTC ISO ({@link canonicalUtcDatetime}), the + * exact function `formatInput` applies on write. One rule, every dialect: + * - It is what a SQLite column now holds, as TEXT that sorts chronologically, + * so the comparison is a plain indexable string compare. + * - It is unambiguous for a Postgres `timestamptz`. This part is a FIX, not a + * no-op: the comparand used to be passed through untouched there, so a bare + * `YYYY-MM-DD` from a `{30_days_ago}` token meant midnight in the SERVER's + * timezone. Measured on an `Asia/Shanghai` server, the identical query put + * the identical instant on a different calendar day than SQLite did — a + * silently wrong window rather than an empty one. * - * For `Field.date()` we keep ISO TEXT but normalise Date objects to - * `YYYY-MM-DD` for the same reason. + * The previous rule coerced to an epoch INTEGER on SQLite only, which assumed + * a storage form the write path never guaranteed; see #3912 for why that could + * not be made correct without also rewriting what the writer produces. + * + * `Field.date` keeps ISO TEXT, normalised to `YYYY-MM-DD` (ADR-0053 Phase 1). */ protected coerceFilterValue(table: string | null, field: string, value: any): any { if (value == null || !table) return value; if (Array.isArray(value)) return value.map((v) => this.coerceFilterValue(table, field, v)); - const isDatetime = this.datetimeFields[table]?.has(field); - const isDate = this.dateFields[table]?.has(field); - if (!isDatetime && !isDate) return value; - - const toMs = (v: any): number | null => { - if (v instanceof Date) return v.getTime(); - if (typeof v === 'number' && Number.isFinite(v)) return v; - if (typeof v === 'string') { - const trimmed = v.trim(); - if (trimmed === '') return null; - if (/^-?\d+$/.test(trimmed)) { - const n = Number(trimmed); - if (Number.isFinite(n)) return n; - } - // Treat bare YYYY-MM-DD as start-of-day UTC; full ISO is parsed - // as-is so timezones round-trip correctly. - const iso = /^\d{4}-\d{2}-\d{2}$/.test(trimmed) ? `${trimmed}T00:00:00.000Z` : trimmed; - const n = Date.parse(iso); - return Number.isFinite(n) ? n : null; - } - return null; - }; - - if (isDatetime) { - // Only SQLite stores `Field.datetime` as an INTEGER epoch (better-sqlite3 - // binds a JS `Date` as `.getTime()`); there the ISO/text comparand MUST be - // coerced to epoch ms or it collapses to a TEXT-vs-INTEGER affinity compare - // that never matches. Postgres/MySQL map datetime to a native TIMESTAMP - // (see `defineColumn` → `table.timestamp`), where Knex binds an ISO string - // or `Date` correctly — coercing to an epoch integer there would compare an - // INTEGER against a TIMESTAMP and break the query. So gate on the storage - // form, via the predicate the bucket expression reads too. - if (!this.isEpochStoredDatetime(table, field)) return value; - const ms = toMs(value); - return ms == null ? value : ms; - } - - // Field.date — normalise the comparand to YYYY-MM-DD (ADR-0053 Phase 1). - return this.toDateOnly(value); + const kind = this.temporalFieldKind(table, field); + if (!kind) return value; + return kind === 'datetime' ? this.storageDatetimeValue(value) : this.toDateOnly(value); } /** - * Does this column store instants as epoch **milliseconds** rather than as a - * temporal type SQL understands natively? + * Might this SQLite `Field.datetime` column still hold values written BEFORE + * the canonical-UTC-text convention (#3912) — an INTEGER/REAL epoch from a + * bound JS `Date`, a zone-naive `CURRENT_TIMESTAMP` string, an offset-bearing + * `+08:00` string — and therefore need + * {@link sqliteCanonicalDatetimeSql} wrapped around it before it is compared + * or bucketed? + * + * This is the ONE predicate for that question. Every consumer of it has already + * been bitten by disagreeing about storage — the filter comparand (#2034, then + * #3912) and the aggregate bucket expression (#3773) — so they share it rather + * than each carrying a copy of the rule. * - * True for a SQLite `Field.datetime` and nothing else: better-sqlite3 binds a - * JS `Date` as `.getTime()`, while Postgres/MySQL get a real TIMESTAMP column - * (`defineColumn` → `table.timestamp`) and `Field.date` is ISO TEXT on every - * dialect. + * `false` in three cases, and the third is the point of the whole exercise: + * - not SQLite (Postgres/MySQL have a real temporal type); + * - not a declared `Field.datetime`; + * - the column has been BACKFILLED to the canonical form in this process + * ({@link backfillCanonicalDatetimes}), so every row is already canonical + * text and the repair would only cost an unindexable expression. + * + * That last exit is what makes the convention pay off rather than just move the + * cost around: after migration the emitted SQL is a plain `col >= ?` again. + */ + /** + * The physical form a `Field.datetime` value takes on THIS dialect — what is + * actually bound, on both the write path (`formatInput`) and the filter path + * (`coerceFilterValue`), so the two can never disagree. * - * This is the ONE predicate for that storage convention. Both consumers of it - * have already been bitten by disagreeing about storage — the filter comparand - * (#2034: an ISO string compared against an INTEGER column matched nothing) - * and the aggregate bucket expression (#3773: `strftime` read the same INTEGER - * as a Julian day and bucketed everything as NULL) — so they share it rather - * than each carrying their own copy of the rule. + * The logical canon is {@link canonicalUtcDatetime}'s `…Z` string everywhere. + * MySQL is the one dialect that cannot parse it, so it gets the same instant + * spelled as a MySQL literal (see {@link mysqlDatetimeLiteral}); SQLite stores + * the canonical string as-is and Postgres parses it straight into `timestamptz`. */ - protected isEpochStoredDatetime(table: string | null | undefined, field: string): boolean { + protected storageDatetimeValue(value: unknown): unknown { + const canonical = canonicalUtcDatetime(value); + return this.isMysql ? mysqlDatetimeLiteral(canonical) : canonical; + } + + protected needsLegacyDatetimeRepair(table: string | null | undefined, field: string): boolean { if (!table || !this.isSqlite) return false; - return this.datetimeFields[table]?.has(field) === true; + if (this.datetimeFields[table]?.has(field) !== true) return false; + return this.canonicalDatetimeFields[table]?.has(field) !== true; } /** - * The value expression to hand SQLite's `strftime()` for a bucketed column. - * - * A TEXT-stored column is passed straight through — `strftime` already parses - * `YYYY-MM-DD`, `YYYY-MM-DDTHH:MM:SSZ` and the zone-naive `CURRENT_TIMESTAMP` - * form. An epoch-stored `Field.datetime` (see {@link isEpochStoredDatetime}) - * must first be converted, or `strftime` reads the bare integer as a Julian - * day number — epoch ms is far outside the legal range, so every row buckets - * as NULL and a trend chart collapses into a single `(null)` bar. + * Read a possibly-legacy SQLite `Field.datetime` column as canonical UTC text + * — the SQL twin of {@link canonicalUtcDatetime}, for rows written before the + * convention existed and not yet backfilled. * - * The conversion dispatches on the STORED value's type, not just the declared - * one, because a SQLite `Field.datetime` column is genuinely mixed-form: an - * explicit value bound as a JS `Date` lands as INTEGER/REAL epoch ms, while a - * `defaultValue: 'NOW()'` slot lands as TEXT (the same mix `formatOutput` → - * `normalizeSqliteDatetimeOutput` already repairs on read). Dividing a TEXT - * timestamp by 1000 would coerce it to its leading year — `'2026-01-10T…'/1000.0` - * is `2.026` seconds past the epoch, bucketing real rows into 1970 — which is - * strictly worse than the NULL it replaces, so the CASE is load-bearing. + * `strftime('%Y-%m-%dT%H:%M:%fZ', …)` is the same format string + * {@link nowColumnDefault} already writes, so a repaired value is + * byte-identical to a freshly written one — which is what lets the comparison + * be a plain text compare. The `typeof()` dispatch is load-bearing: an epoch + * INTEGER handed to `strftime` unconverted is read as a Julian day (#3773), + * while `'unixepoch'` applied to text would read its leading year as seconds. * - * Normalising to a julian day (rather than emitting `strftime(fmt, x/1000.0, - * 'unixepoch')`) keeps ONE reusable scalar the caller can drop into any format - * string, including the two-reference quarter expression. `/1000.0` is real - * division on purpose: integer `/1000` truncates toward zero, which pushes a - * pre-1970 instant forward a day (`-1` → 1970-01-01 instead of 1969-12-31). + * `coalesce(…, col)` preserves genuinely uninterpretable junk instead of + * turning it into NULL, matching `canonicalUtcDatetime`'s totality — a value + * the driver cannot parse keeps failing the comparison, rather than silently + * becoming "no value". */ + protected sqliteCanonicalDatetimeSql(columnSql: string): string { + return ( + `(case when typeof(${columnSql}) in ('integer','real') ` + + `then strftime('%Y-%m-%dT%H:%M:%fZ', ${columnSql}/1000.0, 'unixepoch') ` + + `else coalesce(strftime('%Y-%m-%dT%H:%M:%fZ', ${columnSql}), ${columnSql}) end)` + ); + } + /** * Which temporal presentation rule, if any, a declared field takes — * `null` for everything that is not a `Field.datetime` / `Field.date`. @@ -3726,15 +4030,112 @@ export class SqlDriver implements IDataDriver { return rows; } + /** + * The value expression to hand SQLite's `strftime()` for a bucketed column. + * + * Canonical UTC text (#3912) is passed straight through — `strftime` parses it, + * `YYYY-MM-DD`, and the zone-naive `CURRENT_TIMESTAMP` form alike. A column + * that may still hold PRE-canonical values gets {@link sqliteCanonicalDatetimeSql} + * wrapped around it, which is likewise text, so `strftime` sees one shape + * either way. + * + * Without that repair an epoch INTEGER reaches `strftime` as a bare number, + * which SQLite reads as a Julian DAY — epoch ms is far outside the legal range, + * so every row buckets as NULL and a trend chart collapses to one `(null)` bar + * (#3773). The `typeof()` dispatch inside the repair is equally load-bearing in + * the other direction: dividing a TEXT timestamp by 1000 coerces it to its + * leading year (`'2026-01-10T…'/1000.0` = 2.026 seconds past the epoch), + * bucketing live rows into 1970 — strictly worse than the NULL it replaces. + */ protected sqliteTemporalArg( field: string, table: string | null | undefined, ): { sql: string; bindings: any[] } { - if (!this.isEpochStoredDatetime(table, field)) return { sql: '??', bindings: [field] }; + if (!this.needsLegacyDatetimeRepair(table, field)) return { sql: '??', bindings: [field] }; + return { sql: this.sqliteCanonicalDatetimeSql('??'), bindings: [field, field, field, field] }; + } + + /** + * The left-hand side of a filter comparison on `column`, read in the same + * canonical form {@link coerceFilterValue} puts the comparand in. + * + * `null` — the answer for every dialect, every non-datetime column, and every + * SQLite datetime column that has been backfilled — means the plain identifier + * is already correct, so the caller keeps the ordinary Knex builder call and + * its indexable `col op ?` SQL. Only a column that may still hold PRE-canonical + * values ({@link needsLegacyDatetimeRepair}) is wrapped. + */ + protected filterColumnExpr( + table: string | null | undefined, + field: string, + column: string, + ): { sql: string; bindings: any[] } | null { + if (!this.needsLegacyDatetimeRepair(table, field)) return null; return { - sql: `(case when typeof(??) in ('integer','real') then julianday(??/1000.0, 'unixepoch') else julianday(??) end)`, - bindings: [field, field, field], + sql: this.sqliteCanonicalDatetimeSql('??'), + bindings: [column, column, column, column], + }; + } + + /** + * Compile one VALUE comparison against a storage-normalised column expression + * ({@link filterColumnExpr}), for the operators where the stored form actually + * matters. + * + * Returns `false` — "not handled, carry on" — for everything else, so the + * caller's normal Knex path still owns: null predicates (`IS NULL` reads the + * raw column and is form-independent), the `LIKE` family (a substring match on + * an instant is meaningless, and the raw column is what the user typed + * against), an empty `in`/`nin` set (Knex's `1 = 0` / `1 = 1` shortcuts), and a + * malformed `between` (so the caller still throws its descriptive error). + */ + private applyNormalizedComparison( + builder: any, + join: 'and' | 'or', + expr: { sql: string; bindings: any[] }, + op: string, + value: unknown, + ): boolean { + const raw = join === 'or' ? 'orWhereRaw' : 'whereRaw'; + const binary = (sqlOp: string): boolean => { + // A null comparand is a null PREDICATE, not a comparison — hand it back so + // the caller compiles `IS NULL` / `IS NOT NULL` as it always has. + if (value == null) return false; + builder[raw](`${expr.sql} ${sqlOp} ?`, [...expr.bindings, value]); + return true; }; + const list = (sqlOp: 'in' | 'not in'): boolean => { + if (!Array.isArray(value) || value.length === 0) return false; + const placeholders = value.map(() => '?').join(', '); + builder[raw](`${expr.sql} ${sqlOp} (${placeholders})`, [...expr.bindings, ...value]); + return true; + }; + + switch (op) { + case '=': case '==': case '$eq': + return binary('='); + case '!=': case '<>': case '$ne': + return binary('<>'); + case '>': case '$gt': + return binary('>'); + case '>=': case '$gte': + return binary('>='); + case '<': case '$lt': + return binary('<'); + case '<=': case '$lte': + return binary('<='); + case 'in': case '$in': + return list('in'); + case 'nin': case 'not_in': case 'notin': case '$nin': + return list('not in'); + case 'between': case '$between': { + if (!Array.isArray(value) || value.length !== 2) return false; + builder[raw](`${expr.sql} between ? and ?`, [...expr.bindings, value[0], value[1]]); + return true; + } + default: + return false; + } } /** @@ -3761,6 +4162,25 @@ export class SqlDriver implements IDataDriver { return this.coerceFilterValue(objectName, field, value); } + /** + * The companion of {@link temporalFilterValue} for the same outside-the-builder + * callers: given the SQL they were going to put on the LEFT of the comparison + * (an already-quoted, possibly join-qualified column reference), return the SQL + * they must use instead so the column reads in the storage form the coerced + * comparand is in. + * + * Everything but a SQLite `Field.datetime` gets its `columnSql` back verbatim. + * That one case gets the {@link sqliteEpochMsSql} CASE, because the column is + * mixed INTEGER-epoch / ISO-TEXT and coercing only the value matches whichever + * half the writer happened to produce (#3912). Coercing the value is therefore + * necessary but NOT sufficient — a caller that binds `temporalFilterValue` + * must wrap its column with this too, or it keeps half the bug. + */ + public temporalFilterColumnSql(objectName: string, field: string, columnSql: string): string { + if (!this.needsLegacyDatetimeRepair(objectName, field)) return columnSql; + return this.sqliteCanonicalDatetimeSql(columnSql); + } + protected applyFilters(builder: Knex.QueryBuilder, filters: any) { if (!filters) return; const table = this.coercionKey(builder); @@ -3781,7 +4201,11 @@ export class SqlDriver implements IDataDriver { for (const [key, value] of Object.entries(filters)) { if (['limit', 'offset', 'fields', 'orderBy'].includes(key)) continue; - builder.where(this.remoteColumn(table, key, key), this.coerceFilterValue(table, key, value) as any); + const column = this.remoteColumn(table, key, key); + const coerced = this.coerceFilterValue(table, key, value); + const expr = this.filterColumnExpr(table, key, column); + if (expr && this.applyNormalizedComparison(builder, 'and', expr, '=', coerced)) continue; + builder.where(column, coerced as any); } return; } @@ -3805,7 +4229,10 @@ export class SqlDriver implements IDataDriver { const localField = this.mapSortField(fieldRaw); const field = this.remoteColumn(table, fieldRaw, localField); const coerced = this.coerceFilterValue(table, localField, value); - this.applyAstComparison(builder, nextJoin, field, op, value, coerced); + this.applyAstComparison( + builder, nextJoin, field, op, value, coerced, + this.filterColumnExpr(table, localField, field), + ); } else { const method = nextJoin === 'or' ? 'orWhere' : 'where'; (builder as any)[method]((qb: any) => { @@ -3870,6 +4297,12 @@ export class SqlDriver implements IDataDriver { * null predicates compile to a real `IS NULL` / `IS NOT NULL` (unified with * the `{field, equals, null}` path), and any operator off the whitelist * throws instead of ever reaching Knex. + * + * `columnExpr` (from {@link filterColumnExpr}) is the storage-normalised form + * of `field` — non-null only for a SQLite `Field.datetime`, where comparing the + * raw column would compare against whichever of the two stored forms the writer + * happened to produce (#3912). It is optional so the protected signature stays + * source-compatible for subclasses; omitting it just keeps the raw column. */ protected applyAstComparison( builder: any, @@ -3878,12 +4311,18 @@ export class SqlDriver implements IDataDriver { op: string, rawValue: unknown, coerced: unknown, + columnExpr?: { sql: string; bindings: any[] } | null, ): void { const where = join === 'or' ? 'orWhere' : 'where'; const whereNull = join === 'or' ? 'orWhereNull' : 'whereNull'; const whereNotNull = join === 'or' ? 'orWhereNotNull' : 'whereNotNull'; const opLower = String(op).toLowerCase(); + // Value comparisons on a mixed-storage column read it through the CASE; every + // other operator (null predicates, the LIKE family, a malformed `between`) + // declines and falls through to the ordinary handling below. + if (columnExpr && this.applyNormalizedComparison(builder, join, columnExpr, opLower, coerced)) return; + switch (opLower) { // Equality — 2-arg form so Knex renders `IS NULL` for a null comparand, // keeping the `{field, equals, null}` path working. @@ -4028,9 +4467,13 @@ export class SqlDriver implements IDataDriver { } else if (typeof value === 'object' && value !== null && !Array.isArray(value)) { const localField = this.mapSortField(key); const field = this.remoteColumn(table, key, localField); + // Non-null only for a SQLite `Field.datetime`, whose two stored forms + // (INTEGER epoch / ISO TEXT) must be unified before comparing (#3912). + const columnExpr = this.filterColumnExpr(table, localField, field); for (const [op, opValue] of Object.entries(value as Record)) { const method = logicalOp === 'or' ? 'orWhere' : 'where'; const coerced = this.coerceFilterValue(table, localField, opValue); + if (columnExpr && this.applyNormalizedComparison(builder, logicalOp, columnExpr, op, coerced)) continue; switch (op) { case '$eq': (builder as any)[method](field, coerced); @@ -4116,7 +4559,10 @@ export class SqlDriver implements IDataDriver { const localField = this.mapSortField(key); const field = this.remoteColumn(table, key, localField); const method = logicalOp === 'or' ? 'orWhere' : 'where'; - (builder as any)[method](field, this.coerceFilterValue(table, localField, value) as any); + const coerced = this.coerceFilterValue(table, localField, value); + const columnExpr = this.filterColumnExpr(table, localField, field); + if (columnExpr && this.applyNormalizedComparison(builder, logicalOp, columnExpr, '=', coerced)) continue; + (builder as any)[method](field, coerced as any); } } } @@ -4233,6 +4679,25 @@ export class SqlDriver implements IDataDriver { } } + /** + * DDL for a builtin `created_at` / `updated_at` audit column. + * + * These are created directly rather than through {@link createColumn} (they are + * not declared fields), but the objectql registry DOES declare them as + * `Field.datetime` on every audited object — so they are filtered, sorted and + * bucketed like one, and must take the same physical type or they inherit the + * `TIMESTAMP` problems on MySQL: no milliseconds, and a 2038 ceiling on the + * column every list view sorts by (#3942). `CURRENT_TIMESTAMP` has to carry + * matching precision for a `DATETIME(3)` default, hence `now(3)`. + */ + protected createAuditTimestampColumn(table: Knex.CreateTableBuilder, name: string): void { + if (this.isMysql) { + table.datetime(name, { precision: 3 }).defaultTo(this.knex.fn.now(3)); + return; + } + table.timestamp(name).defaultTo(this.knex.fn.now()); + } + protected createColumn(table: Knex.CreateTableBuilder, name: string, field: any) { if (field.multiple) { table.json(name); @@ -4286,7 +4751,18 @@ export class SqlDriver implements IDataDriver { col = table.date(name); break; case 'datetime': - col = table.timestamp(name); + // MySQL's `TIMESTAMP` is a 32-bit epoch: it cannot represent an instant + // outside 1970-01-01..2038-01-19 (a contract end date, a subscription + // expiry, a retention horizon — all rejected outright), it carries no + // fractional seconds by default so the canonical form's milliseconds are + // silently truncated, and it CONVERTS on read/write using the session + // timezone, which makes the stored instant depend on server config. + // `DATETIME(3)` has none of those properties: 1000..9999, milliseconds + // kept, and stored verbatim — so the column holds the UTC wall clock the + // driver writes, exactly as ServiceNow stores its MySQL timestamps + // (#3942). Postgres deliberately keeps `table.timestamp` → `timestamptz`: + // asking for precision 3 there would REDUCE it from microseconds. + col = this.isMysql ? table.datetime(name, { precision: 3 }) : table.timestamp(name); break; case 'time': col = table.time(name); @@ -4468,12 +4944,43 @@ export class SqlDriver implements IDataDriver { } } + // ADR-0053: a `Field.datetime` is an instant stored as UTC. Make the STORAGE + // say so — collapse every accepted input shape (JS `Date`, epoch number, ISO + // string, zone-naive wall clock, bare calendar day) to one canonical + // `YYYY-MM-DDTHH:MM:SS.sssZ` before it hits the wire (#3912). + // + // This is the write half of the fix; `coerceFilterValue` applies the SAME + // function to comparands, so the two sides of a comparison can no longer + // disagree about shape. It runs on EVERY dialect, for two different reasons: + // - SQLite has no temporal type, so whatever is bound is what is stored. A + // `Date` landed as INTEGER epoch and a REST/JSON write as ISO TEXT, in + // the same column — the mixed storage that made every window filter + // return the wrong rows, and that still makes ORDER BY sort all INTEGER + // rows before all TEXT ones (#3928). + // - Postgres/MySQL do have one, but a zone-NAIVE string bound into it is + // interpreted in the SERVER's timezone, not UTC — measured at 8 hours off + // on an `Asia/Shanghai` server. Sending an explicit `Z` removes the + // server's timezone from the write path entirely. + const datetimeFields = this.datetimeFields[object]; + if (datetimeFields && datetimeFields.size > 0 && copy && typeof copy === 'object') { + for (const field of datetimeFields) { + const v = copy[field]; + if (v == null) continue; + // `NOW()` was already replaced with an ISO instant above; anything else + // that is not interpretable as a time passes through untouched. + const normalized = this.storageDatetimeValue(v); + if (normalized !== v) { + if (!copied) { copy = { ...copy }; copied = true; } + copy[field] = normalized; + } + } + } + // ADR-0053 Phase 1: a `Field.date` is a timezone-naive calendar day, not // an instant. Collapse any `Date` or full-ISO value to `YYYY-MM-DD` before // it hits the wire so storage matches the date-only contract the filter // layer (`coerceFilterValue`) already enforces — the write/filter // asymmetry was the root cause of the silent date-equality miss. - // `Field.datetime` is untouched (it keeps full-instant semantics). const dateFields = this.dateFields[object]; if (dateFields && dateFields.size > 0 && copy && typeof copy === 'object') { for (const field of dateFields) { diff --git a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts new file mode 100644 index 0000000000..18d9a9a583 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter-column.test.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression #3912 — the COLUMN half of the datetime storage-form fix. + * + * `native-sql-datetime-filter.test.ts` covers coercing the comparand to epoch ms. + * That alone is not enough: a SQLite `Field.datetime` column holds an INTEGER + * epoch (a `Date` write) and ISO TEXT (a REST/JSON write, a `NOW()` default) at + * the same time, so an epoch comparand matches the INTEGER rows and misses every + * TEXT one — a dashboard `dateRange: last_30_days` reading 0 with rows in range. + * + * The fix threads a companion `StrategyContext.coerceTemporalFilterColumn` hook + * that lets the driver normalise the column reference. These tests assert the + * strategy applies it to exactly the value comparisons, leaves the null and LIKE + * predicates on the raw column, and emits byte-identical SQL when the hook is + * absent (Postgres, non-SQL drivers, legacy wiring). + */ + +import { describe, it, expect } from 'vitest'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; + +const cube: Cube = { + name: 'compliance', + title: 'Compliance', + sql: 'compliance_assessment', + measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } }, + dimensions: { + // Dimension id deliberately differs from the column, so a hook that fires on + // `assessed_at` proves the storage target resolved the real column. + assessed: { name: 'assessed', label: 'Assessed', type: 'time', sql: 'assessed_at' }, + title: { name: 'title', label: 'Title', type: 'string', sql: 'title' }, + }, + public: false, +}; + +/** Stand-in for `SqlDriver.temporalFilterColumnSql` under better-sqlite3. */ +function sqliteColumnHook(object: string, field: string, columnSql: string): string { + if (object === 'compliance_assessment' && field === 'assessed_at') { + return `EPOCH_MS(${columnSql})`; + } + return columnSql; // date text / non-temporal / native timestamp → unchanged +} + +function ctxWith(overrides: Partial): StrategyContext { + return { + getCube: (name) => (name === 'compliance' ? cube : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [], + ...overrides, + }; +} + +const withHook = () => ctxWith({ coerceTemporalFilterColumn: sqliteColumnHook }); + +const gen = async (query: AnalyticsQuery, ctx: StrategyContext) => + new NativeSQLStrategy().generateSql(query, ctx); + +describe('NativeSQLStrategy — datetime filter column normalisation (#3912)', () => { + it('normalises the column on a scalar comparison', async () => { + const { sql } = await gen( + { cube: 'compliance', measures: ['total'], where: { assessed: { $gte: '2025-06-18' } } }, + withHook(), + ); + expect(sql).toContain('EPOCH_MS(assessed_at) >= $1'); + }); + + it('normalises the column on an `in` set', async () => { + const { sql } = await gen( + { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $in: ['2025-06-18', '2025-06-19'] } }, + }, + withHook(), + ); + expect(sql).toContain('EPOCH_MS(assessed_at) IN ($1, $2)'); + }); + + it('normalises the column on a timeDimension dateRange — the dashboard shape', async () => { + const { sql } = await gen( + { + cube: 'compliance', + measures: ['total'], + timeDimensions: [{ dimension: 'assessed', dateRange: ['2025-06-18', '2025-07-01'] }], + }, + withHook(), + ); + expect(sql).toContain( + 'EPOCH_MS(assessed_at) BETWEEN $1 AND $2', + ); + }); + + it('leaves the null predicates on the raw column (storage-independent)', async () => { + const set = await gen( + { cube: 'compliance', measures: ['total'], where: { assessed: { $exists: true } } }, + withHook(), + ); + expect(set.sql).toContain('assessed_at IS NOT NULL'); + expect(set.sql).not.toContain('EPOCH_MS'); + + const notSet = await gen( + { cube: 'compliance', measures: ['total'], where: { assessed: null } }, + withHook(), + ); + expect(notSet.sql).toContain('assessed_at IS NULL'); + expect(notSet.sql).not.toContain('EPOCH_MS'); + }); + + it('leaves a LIKE match on the raw column (a substring match reads the text)', async () => { + const { sql } = await gen( + { cube: 'compliance', measures: ['total'], where: { assessed: { $contains: '2025-06' } } }, + withHook(), + ); + expect(sql).toContain('assessed_at LIKE $1'); + expect(sql).not.toContain('EPOCH_MS'); + }); + + it('does not touch a non-temporal column', async () => { + const { sql } = await gen( + { cube: 'compliance', measures: ['total'], where: { title: { $eq: 'SOC2' } } }, + withHook(), + ); + expect(sql).toContain('title = $1'); + expect(sql).not.toContain('EPOCH_MS'); + }); + + it('is backward-compatible: no hook → the bare column, exactly as before', async () => { + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18' } }, + timeDimensions: [{ dimension: 'assessed', dateRange: ['2025-06-18', '2025-07-01'] }], + }; + const { sql } = await gen(query, ctxWith({})); + expect(sql).toContain('assessed_at >= $1'); + expect(sql).toContain('assessed_at BETWEEN $2 AND $3'); + }); + + it('falls back to the bare column when the hook returns nothing usable', async () => { + const { sql } = await gen( + { cube: 'compliance', measures: ['total'], where: { assessed: { $gte: '2025-06-18' } } }, + ctxWith({ coerceTemporalFilterColumn: () => '' as unknown as string }), + ); + expect(sql).toContain('assessed_at >= $1'); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index b45b3e058e..45c1fb26db 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -175,6 +175,14 @@ export interface AnalyticsServiceConfig { * `StrategyContext.coerceTemporalFilterValue` for the full rationale. */ coerceTemporalFilterValue?: (objectName: string, fieldName: string, value: unknown) => unknown; + /** + * Normalise the COLUMN side of the same comparison to that storage form — the + * other half of the fix, needed because a SQLite `Field.datetime` holds both an + * INTEGER epoch (a `Date` write) and ISO TEXT (a REST/JSON write, a `NOW()` + * default) at once, so coercing only the comparand matches one of them and + * misses the other (#3912). See `StrategyContext.coerceTemporalFilterColumn`. + */ + coerceTemporalFilterColumn?: (objectName: string, fieldName: string, columnSql: string) => string; /** * ADR-0062 D6 — report whether an object is federated (external datasource). * Threaded into the StrategyContext so `NativeSQLStrategy` declines external @@ -331,6 +339,7 @@ export class AnalyticsService implements IAnalyticsService { this.datasetRegistry.get(cubeName)?.allowedRelationships ?? config.getAllowedRelationships?.(cubeName), coerceTemporalFilterValue: config.coerceTemporalFilterValue, + coerceTemporalFilterColumn: config.coerceTemporalFilterColumn, isExternalObject: config.isExternalObject, }; diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 44d7e9646e..5adbb5a582 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -61,6 +61,14 @@ interface DriverLike { * timestamp / non-temporal → unchanged). Optional — only SqlDriver implements it. */ temporalFilterValue?(objectName: string, field: string, value: unknown): unknown; + /** + * Normalise the column reference to that same storage form. Required alongside + * `temporalFilterValue` on any dialect whose column is mixed-form — a SQLite + * `Field.datetime` holds an INTEGER epoch and ISO TEXT at once, so coercing + * only the value fixes one half and leaves the other empty (#3912). Optional — + * only SqlDriver implements it. + */ + temporalFilterColumnSql?(objectName: string, field: string, columnSql: string): string; } /** @@ -473,6 +481,29 @@ export class AnalyticsServicePlugin implements Plugin { return value; }; + // The column half of the same fix (#3912). A SQLite `Field.datetime` column + // holds BOTH storage forms — INTEGER epoch from a `Date` write, ISO TEXT from + // a REST/JSON write or a `NOW()` default — so coercing the comparand alone + // matched whichever half the writer produced and returned an empty window for + // the other. Ask the driver for the column expression that normalises both. + const coerceTemporalFilterColumn = ( + objectName: string, + fieldName: string, + columnSql: string, + ): string => { + try { + const svc = ctx.getService('data'); + const driver = svc?.getDriverForObject?.(objectName); + if (driver && typeof driver.temporalFilterColumnSql === 'function') { + return driver.temporalFilterColumnSql(objectName, fieldName, columnSql); + } + } catch { + // Same tiering as above — an unresolvable driver emits the bare column, + // which is today's behaviour and correct on every non-mixed dialect. + } + return columnSql; + }; + const config: AnalyticsServiceConfig = { cubes: this.options.cubes, logger: ctx.logger, @@ -483,6 +514,7 @@ export class AnalyticsServicePlugin implements Plugin { getReadScope, getAllowedRelationships: this.options.getAllowedRelationships, coerceTemporalFilterValue, + coerceTemporalFilterColumn, relationshipResolver, labelResolver, // ADR-0053 — source-field currency metadata for the measure currency chain. diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 13f982f333..a9a35543c3 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -119,13 +119,18 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (range.length === 2) { // Same epoch-vs-text root cause as buildFilterClause: a dateRange on a // SQLite `Field.datetime` column compares ISO TEXT against an INTEGER - // epoch and matches nothing. Coerce both bounds to the storage form. + // epoch and matches nothing. Coerce both bounds to the storage form — + // and normalise the column to that form too, because the column holds + // BOTH forms at once and coercing only the bounds still empties the + // half the writer stored the other way (#3912). const td2 = this.resolveStorageTarget(cube, td.dimension, tableName); params.push( this.coerceTemporal(ctx, td2, range[0]), this.coerceTemporal(ctx, td2, range[1]), ); - whereClauses.push(`${colExpr} BETWEEN $${params.length - 1} AND $${params.length}`); + whereClauses.push( + `${this.temporalColumn(ctx, td2, colExpr)} BETWEEN $${params.length - 1} AND $${params.length}`, + ); } } } @@ -436,8 +441,28 @@ export class NativeSQLStrategy implements AnalyticsStrategy { return coerceFilterValueForSql(value); } - private buildFilterClause( + /** + * The column side of {@link coerceTemporal}: normalise the reference so it + * reads in the storage form the comparand was coerced into. + * + * A SQLite `Field.datetime` column carries an INTEGER epoch (a `Date` write) + * and ISO TEXT (a REST/JSON write, a `NOW()` default — including the platform's + * own `created_at`) at the SAME time, so coercing the value alone fixes one half + * and empties the other. That is #3912: a `dateRange: last_30_days` on + * `created_date` read 0 with 29 rows in range. Every other column and dialect + * gets its reference back verbatim. + */ + private temporalColumn( + ctx: StrategyContext, + target: { object: string; field: string }, col: string, + ): string { + if (typeof ctx.coerceTemporalFilterColumn !== 'function') return col; + return ctx.coerceTemporalFilterColumn(target.object, target.field, col) || col; + } + + private buildFilterClause( + rawCol: string, operator: string, values: string[] | undefined, params: unknown[], @@ -449,8 +474,11 @@ export class NativeSQLStrategy implements AnalyticsStrategy { contains: 'LIKE', notContains: 'NOT LIKE', }; - if (operator === 'set') return `${col} IS NOT NULL`; - if (operator === 'notSet') return `${col} IS NULL`; + // Null predicates and the LIKE family read the column as stored — the former + // is storage-independent, the latter is a substring match on the raw text — + // so only the value comparisons take the normalised reference. + if (operator === 'set') return `${rawCol} IS NOT NULL`; + if (operator === 'notSet') return `${rawCol} IS NULL`; if (operator === 'in' || operator === 'notIn') { if (!values || values.length === 0) return null; @@ -458,7 +486,7 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // KPI), so coerce each element to the column's storage form too — same // SQLite epoch-vs-text root cause as the scalar operators below. const placeholders = values.map(v => { params.push(this.coerceTemporal(ctx, target, v)); return `$${params.length}`; }).join(', '); - return `${col} ${operator === 'in' ? 'IN' : 'NOT IN'} (${placeholders})`; + return `${this.temporalColumn(ctx, target, rawCol)} ${operator === 'in' ? 'IN' : 'NOT IN'} (${placeholders})`; } const sqlOp = opMap[operator]; @@ -466,16 +494,17 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (operator === 'contains' || operator === 'notContains') { params.push(`%${values[0]}%`); - } else { - // Coerce so booleans/numbers bind as their native SQL types AND so a - // relative-date / ISO-string comparand on a SQLite `Field.datetime` - // column is converted to its INTEGER epoch storage form. Without this a - // dashboard filter like `assessed_at >= '2025-06-18'` compiles to a - // TEXT-vs-INTEGER affinity compare that is always false → "No rows", - // even though the rows exist (the confirmed time-series chart bug). - params.push(this.coerceTemporal(ctx, target, values[0])); + return `${rawCol} ${sqlOp} $${params.length}`; } - return `${col} ${sqlOp} $${params.length}`; + + // Coerce so booleans/numbers bind as their native SQL types AND so a + // relative-date / ISO-string comparand on a SQLite `Field.datetime` + // column is converted to its INTEGER epoch storage form. Without this a + // dashboard filter like `assessed_at >= '2025-06-18'` compiles to a + // TEXT-vs-INTEGER affinity compare that is always false → "No rows", + // even though the rows exist (the confirmed time-series chart bug). + params.push(this.coerceTemporal(ctx, target, values[0])); + return `${this.temporalColumn(ctx, target, rawCol)} ${sqlOp} $${params.length}`; } private extractObjectName(cube: Cube): string { diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index 2deb994ac9..2db04ffb91 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -394,6 +394,32 @@ export interface StrategyContext { */ coerceTemporalFilterValue?(objectName: string, fieldName: string, value: unknown): unknown; + /** + * The companion of {@link StrategyContext.coerceTemporalFilterValue} for the + * LEFT side of the same comparison: given the SQL the strategy was going to + * emit for the column (an already-quoted, possibly alias-qualified reference), + * return the SQL it must emit instead so the column reads in the same storage + * form the coerced comparand is in. + * + * Why coercing the value alone is not enough: a SQLite `Field.datetime` column + * is MIXED-form in practice. A JS `Date` binds as an INTEGER epoch, but a REST + * / JSON write carries an ISO string (JSON has no `Date`) and a `NOW()` default + * — including the platform's own `created_at` / `updated_at` stamps — lands as + * ISO TEXT. Coercing the comparand to epoch ms therefore fixes the INTEGER rows + * and breaks the TEXT ones, which is why a dashboard `dateRange: last_30_days` + * still read 0 while the rows existed (#3912). The driver answers with an + * expression that normalises whatever is stored, so both halves match. + * + * Everything else — `Field.date`, native-timestamp dialects, non-temporal + * columns — gets `columnSql` back verbatim, and when the hook is absent the + * strategy emits the bare column exactly as before, so it is purely additive. + * + * @param objectName Logical object / table backing the cube. + * @param fieldName Bare column name the filter targets. + * @param columnSql The SQL reference the strategy resolved for that column. + */ + coerceTemporalFilterColumn?(objectName: string, fieldName: string, columnSql: string): string; + /** * ADR-0062 D6 — is `objectName` a federated (external-datasource) object? * diff --git a/packages/spec/src/data/date-macros.zod.ts b/packages/spec/src/data/date-macros.zod.ts index 5f8e9c3f08..35ecef1bb5 100644 --- a/packages/spec/src/data/date-macros.zod.ts +++ b/packages/spec/src/data/date-macros.zod.ts @@ -33,8 +33,10 @@ import { z } from 'zod'; * * Either way the DRIVER only ever sees ISO date / timestamp strings, * never `{tokens}`. Translating an ISO comparand into a column's on-disk - * form (SQLite epoch-ms, `YYYY-MM-DD` text, native timestamp) is the - * driver's job — see `SqlDriver.temporalFilterValue`. + * form — canonical UTC text on SQLite, a native `timestamptz` on + * Postgres, a `DATETIME(3)` literal on MySQL, and `YYYY-MM-DD` text for + * a calendar day on every dialect — is the driver's job; see + * `SqlDriver.temporalFilterValue`. * * A token OUTSIDE this vocabulary is rejected rather than passed * through: `@objectstack/lint`'s `validate-filter-tokens` fails the