Skip to content

Commit c8124e5

Browse files
authored
fix(driver-sql): give Field.datetime one UTC storage form per dialect (#3912) (#3953)
Any window filter on a `Field.datetime` column returned an empty set on SQLite — a dashboard `dateRange: last_30_days` 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 `Date` landed as INTEGER epoch ms, while a REST/JSON write (JSON has no Date type), a `NOW()` default, and the platform's own created_at/updated_at 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: - 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 bucket expression needs no CASE. - Postgres → `timestamptz`, unchanged. The fix is on the write/comparand side: a zone-naive write resolved against the SERVER's timezone (8h off on Asia/Shanghai), and an un-anchored `YYYY-MM-DD` comparand meant the server's local midnight, putting 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 server layer, and a MySQL-spelled bind. MySQL accepts neither the T separator nor the Z suffix, so datetime writes over REST had always failed outright; TIMESTAMP additionally truncated milliseconds and could not store an instant outside 1970..2038. Both other dialects were measured against real servers (PostgreSQL 16 and MariaDB 10.11, both non-UTC), not assumed. 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, and #3942 (MySQL TIMESTAMP range/precision/timezone). Rationale is recorded as ADR-0053 addendum D-B1..D-B4. Closes #3912 Closes #3942 Closes #3928
1 parent 6e141bc commit c8124e5

22 files changed

Lines changed: 2405 additions & 226 deletions
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/service-analytics": patch
4+
"@objectstack/spec": patch
5+
---
6+
7+
fix(driver-sql): give `Field.datetime` one UTC storage form per dialect (#3912, #3942)
8+
9+
Any window filter on a `Field.datetime` column returned an empty set on SQLite —
10+
a dashboard `dateRange: last_30_days` on `created_date` read 0 while 29 matching
11+
rows existed.
12+
13+
There was never a storage *convention*, only a description of what better-sqlite3
14+
happened to do with a bound JS `Date`. Nothing enforced it — `formatInput`
15+
deliberately left `datetime` untouched — so the form was decided by whichever
16+
writer got there first: a JS `Date` landed as INTEGER epoch ms, while a REST/JSON
17+
write (JSON has no `Date` type), a `defaultValue: 'NOW()'` slot, and the
18+
platform's own `created_at` / `updated_at` all landed as ISO **TEXT**. One column
19+
held both forms while the read path coerced comparands to epoch ms purely from
20+
the *declared* type. On SQLite's type ordering (`INTEGER < TEXT`) a two-sided
21+
window collapsed to zero rows, and a one-sided `>=` matched every TEXT row
22+
regardless of the bound.
23+
24+
`Field.datetime` now has one canonical instant per dialect, produced by one
25+
function applied on write **and** to every filter comparand, so the two sides of
26+
a comparison cannot disagree about shape:
27+
28+
- **SQLite**`YYYY-MM-DDTHH:MM:SS.sssZ` text. Lexicographic order *is*
29+
chronological order, so range filters and `ORDER BY` read the column directly
30+
and can use an index; `strftime` parses it, so the date-bucket expression needs
31+
no CASE.
32+
- **Postgres**`timestamptz`, unchanged. The fix here is on the write and
33+
comparand side: a zone-naive write was previously resolved against the
34+
*server's* timezone (measured 8 hours off on `Asia/Shanghai`), and an
35+
un-anchored `YYYY-MM-DD` comparand meant the server's local midnight, so the
36+
identical query over the identical instant landed a row on a different calendar
37+
day than SQLite did.
38+
- **MySQL**`DATETIME(3)` instead of `TIMESTAMP`, a connection pinned to UTC on
39+
both the mysql2 and the server layer, and a MySQL-spelled bind carrying the
40+
same UTC wall clock. MySQL accepts neither the `T` separator nor the `Z` suffix
41+
in a datetime literal, so datetime writes over REST had always failed outright;
42+
`TIMESTAMP` additionally truncated milliseconds and could not store an instant
43+
outside 1970..2038.
44+
45+
Existing rows converge at schema sync. Both migrations are allowed to fail: they
46+
log, mark nothing, and the read paths keep a repair expression, so an un-migrated
47+
column still compares and buckets **correctly** — just unindexed. Neither can
48+
repair instants the old timezone-ambiguous write path recorded wrongly; they
49+
preserve what is on disk.
50+
51+
Also closes #3928 (datetime `ORDER BY` mis-sorted on mixed storage) by
52+
construction. Rationale is recorded as ADR-0053 addendum D-B1..D-B4.
53+
54+
The analytics change is additive: a `coerceTemporalFilterColumn` companion to the
55+
existing `coerceTemporalFilterValue` hook, so a raw-SQL strategy can normalise the
56+
column side too. Absent hook → byte-identical SQL.

content/docs/automation/webhooks.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ writable.
148148
| `response_code` | number | Last HTTP status code received. |
149149
| `response_body` | textarea | Truncated to the first 16 KB. |
150150
| `error` | textarea | Last transport-level error (DNS, connect, timeout). |
151-
| `created_at` | datetime | Native TIMESTAMP column — written as a `Date`, not an epoch-ms number. |
152-
| `updated_at` | datetime | Native TIMESTAMP column — written as a `Date`, not an epoch-ms number. |
151+
| `created_at` | datetime | A `Field.datetime` UTC instant — not an epoch-ms number like the columns above. |
152+
| `updated_at` | datetime | A `Field.datetime` UTC instant — not an epoch-ms number like the columns above. |
153153

154154
> **Why store full payload?** Receivers may be down for hours; we must
155155
> retry the *exact* bytes we promised to send. Recomputing payload from

content/docs/references/data/date-macros.mdx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,13 @@ Either way the DRIVER only ever sees ISO date / timestamp strings,
5353

5454
never `\{tokens\}`. Translating an ISO comparand into a column's on-disk
5555

56-
form (SQLite epoch-ms, `YYYY-MM-DD` text, native timestamp) is the
56+
form — canonical UTC text on SQLite, a native `timestamptz` on
5757

58-
driver's job — see `SqlDriver.temporalFilterValue`.
58+
Postgres, a `DATETIME(3)` literal on MySQL, and `YYYY-MM-DD` text for
59+
60+
a calendar day on every dialect — is the driver's job; see
61+
62+
`SqlDriver.temporalFilterValue`.
5963

6064
A token OUTSIDE this vocabulary is rejected rather than passed
6165

docs/adr/0053-date-and-datetime-semantics.md

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-0053: `date` is a timezone-naive calendar day; `datetime` is an instant rendered in a reference timezone
22

3-
**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.
3+
**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).
44
**Deciders**: ObjectStack Protocol Architects
55
**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)
66
**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.
402402
inventing a semantic" stance. No change to Phase 2's reference-timezone plan.
403403
- Until D-A2 lands, the hook depends on a duck-typed driver method — a known,
404404
intentionally-temporary seam tracked here.
405+
406+
---
407+
408+
## Addendum (2026-07-29) — `Field.datetime` has ONE storage form per dialect, always a UTC instant
409+
410+
> **Status:** landed. This addendum **revises** the storage half of Phase 1 (which
411+
> left `Field.datetime` "stored as UTC epoch ms" on SQLite) and **extends**
412+
> D-A1 from the 2026-06-18 addendum to the column side of the comparison.
413+
> Closes #3912; supersedes the epoch-ms convention referenced there.
414+
415+
### What the epoch-ms convention actually was
416+
417+
It was never a convention — it was a description of what better-sqlite3 happened
418+
to do with a bound JS `Date`. Nothing enforced it: `formatInput` deliberately left
419+
`datetime` untouched, so the storage form was decided by whichever writer got
420+
there first.
421+
422+
- A JS `Date` (seed loader, import, server-side code) → INTEGER epoch ms.
423+
- A REST/JSON write → ISO **TEXT**, because JSON has no `Date` type.
424+
- A `defaultValue: 'NOW()'` slot → ISO TEXT.
425+
- The platform's own `created_at` / `updated_at` → ISO TEXT (they are stamped
426+
with `toISOString()`), on **every** object in the system.
427+
428+
One column therefore held both forms at once, while the read path coerced filter
429+
comparands to epoch ms purely from the DECLARED type. On SQLite's type ordering
430+
(`INTEGER < TEXT`) that made a two-sided window collapse to zero rows and a
431+
one-sided `>=` match every TEXT row regardless of the bound — the reported symptom
432+
in #3912 (a dashboard `last_30_days` reading 0 with 29 rows in range). It also
433+
left `ORDER BY` sorting all INTEGER rows before all TEXT ones (#3928).
434+
435+
### D-B1 — The canonical storage form is `YYYY-MM-DDTHH:MM:SS.sssZ`
436+
437+
`Field.datetime` is stored as fixed-width, zone-explicit UTC text on SQLite, and
438+
written to Postgres/MySQL as that same string. `canonicalUtcDatetime()` is the one
439+
function that produces it, applied on write (`formatInput`) and to every filter
440+
comparand (`coerceFilterValue`) so the two sides of a comparison cannot disagree
441+
about shape.
442+
443+
Chosen over the epoch integer because:
444+
445+
- Lexicographic order **is** chronological order, so range filters and `ORDER BY`
446+
read the column directly and can use an index. An epoch convention forces an
447+
expression wrapper on every temporal predicate — it moves the cost rather than
448+
removing it.
449+
- `strftime`/`julianday` parse it, so the date-bucket expression needs no
450+
epoch↔text CASE (#3773).
451+
- It is what `formatOutput` already presents, so storage and presentation stop
452+
disagreeing — the asymmetry Phase 1 removed for `date`, now removed for
453+
`datetime`.
454+
- It matches the `Field.date` convention, so the platform has one temporal
455+
storage story instead of one per field type.
456+
- It was already the majority of rows on disk, so the migration is the smaller
457+
one.
458+
459+
### D-B2 — The comparand rule is dialect-INDEPENDENT, which fixes Postgres too
460+
461+
`coerceFilterValue` previously canonicalised only on SQLite and passed the value
462+
through on native-timestamp dialects. That was not neutral. Measured against
463+
PostgreSQL 16 with `TimeZone = Asia/Shanghai`:
464+
465+
- A zone-**naive** write (`'2026-03-20 12:00:00'`) bound into `timestamptz` was
466+
resolved against the SERVER's timezone and stored as `2026-03-20T04:00:00Z`
467+
8 hours off the instant SQLite records for the same write, in direct violation
468+
of this ADR's "a naive wall clock is UTC" rule.
469+
- A bare `YYYY-MM-DD` comparand (what a `{30_days_ago}` token expands to) meant
470+
midnight in the SERVER's timezone, so the identical query over the identical
471+
instant put a row on a **different calendar day** than it did on SQLite.
472+
473+
Stating the `Z` on both sides removes the server's timezone from the answer.
474+
Postgres storage itself needed no change — knex's `table.timestamp` already
475+
creates `timestamptz` (`useTz` defaults true, verified) — so this is a write- and
476+
comparand-side fix there, not a migration. Regression cover is
477+
`sql-driver-datetime-postgres-timezone.test.ts`, opt-in via `OS_TEST_POSTGRES_URL`
478+
because CI provisions no server; it asserts it is pointed at a non-UTC server so
479+
it cannot pass vacuously.
480+
481+
### D-B3 — Existing rows converge at schema sync; correctness never depends on it
482+
483+
`backfillCanonicalDatetimes` runs inside `initObjects`, rewriting SQLite rows that
484+
are not already canonical (INTEGER/REAL epoch, zone-naive text, offset-bearing
485+
text). It is idempotent, skips freshly-created tables entirely, and preserves
486+
values SQLite cannot parse rather than nulling them.
487+
488+
It is allowed to fail. On error it logs and marks nothing, and the read paths keep
489+
the `sqliteCanonicalDatetimeSql` repair that makes an un-migrated column compare
490+
and bucket correctly — just without an index. A migration must never be able to
491+
take boot down, and query correctness must never be contingent on one having run.
492+
The corollary is that the repair expression stays in the codebase permanently: it
493+
also covers external/unmanaged tables (ADR-0015), which never get a backfill.
494+
495+
`needsLegacyDatetimeRepair` is the single predicate every read path asks, so the
496+
filter, bucket and analytics surfaces cannot drift apart about whether a column
497+
is clean.
498+
499+
### Consequences
500+
501+
- D-A2 (promote `temporalFilterValue` onto the `IDataDriver` contract) now has a
502+
second method to carry with it: `temporalFilterColumnSql`, the column-side
503+
companion threaded to analytics as
504+
`StrategyContext.coerceTemporalFilterColumn`. Coercing the value alone is not
505+
sufficient on a mixed-form column, so any surface that binds a comparand into
506+
raw SQL must wrap its column reference too. Both remain duck-typed until D-A2.
507+
- D-A3's conformance matrix should gain a **storage-form** axis (canonical,
508+
legacy-epoch, legacy-naive) and a **server-timezone** axis, since both are now
509+
known to have produced dialect-divergent row results.
510+
- #3928 (datetime `ORDER BY` mis-sorted on mixed storage) is closed by
511+
construction rather than by a sort-side fix.
512+
### D-B4 — MySQL stores the same instant, spelled the way MySQL parses it
513+
514+
MySQL was the third dialect, and measurement (MariaDB 10.11, `default_time_zone
515+
= '+08:00'`, process on `America/New_York`) found it the worst off — including
516+
one defect D-B1 *introduced*:
517+
518+
- MySQL accepts neither the `T` separator nor the `Z` suffix in a datetime
519+
literal, so the canonical form failed the statement outright with *Incorrect
520+
datetime value*. An ISO comparand or REST/JSON write had **always** failed this
521+
way; canonicalising the write path extended it to `Date` writes too.
522+
- `table.timestamp` emits MySQL `TIMESTAMP`: a 32-bit epoch that cannot hold an
523+
instant outside 1970..2038 (a contract end date in 2040 was rejected), carries
524+
no fractional digits so the canonical form's milliseconds were truncated, and
525+
converts on read/write using the session timezone.
526+
- mysql2's `connection.timezone` defaults to the HOST's local zone and
527+
`@@session.time_zone` to the server's, so a zone-naive value landed 12 hours
528+
off with the two misconfigurations compounding.
529+
530+
The resolution keeps the logical canon and changes only the physical spelling:
531+
532+
1. `Field.datetime` maps to **`DATETIME(3)`** — range 1000..9999, milliseconds
533+
kept, and no timezone conversion of its own, so the column holds the UTC wall
534+
clock the driver writes. This is the ServiceNow model. Postgres deliberately
535+
keeps `timestamptz`: asking for precision 3 there would *reduce* it from
536+
microseconds. The builtin `created_at`/`updated_at` take the same type — the
537+
registry declares them `Field.datetime`, and they are what most list views
538+
sort by.
539+
2. The connection is **pinned to UTC on both layers** — `connection.timezone =
540+
'Z'` for mysql2 and `SET time_zone = '+00:00'` via `pool.afterCreate` for the
541+
server. This is what makes the wall clock *be* the instant, and it keeps a
542+
not-yet-migrated `TIMESTAMP` column correct too. An explicit host choice is
543+
left alone; an existing `afterCreate` is chained, not replaced.
544+
3. `storageDatetimeValue` respells the canonical instant as a MySQL literal for
545+
the bind, on the write path and the filter path alike. Deliberately strict:
546+
only an exactly-canonical string is rewritten, so an unparseable value or a
547+
year outside 1000..9999 reaches MySQL untouched and fails loudly.
548+
549+
`migrateMysqlDatetimeColumns` widens legacy `TIMESTAMP` columns at schema sync,
550+
under the same failure policy as D-B3 — a `TIMESTAMP` column keeps *working*
551+
(the driver binds the same literal and the session is UTC), it merely keeps the
552+
range and precision limits. Verified on a real server: the ALTER moves no stored
553+
instant, correctly-stored legacy rows round-trip exactly, and it is idempotent.
554+
555+
The same caveat as Postgres applies and is worth stating plainly: the migration
556+
**cannot repair instants the old timezone-ambiguous write path recorded wrongly**
557+
— that information is gone. It preserves what is on disk.
558+
559+
Regression cover is `sql-driver-datetime-mysql-storage.test.ts`, opt-in via
560+
`OS_TEST_MYSQL_URL` (CI provisions no server), asserting a non-UTC server so it
561+
cannot pass vacuously. 10 of its 13 cases fail without this change.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Test double for a SQLite database that still holds PRE-canonical
5+
* `Field.datetime` values — the shape every deployment had before #3912, and the
6+
* shape one still has when `backfillCanonicalDatetimes` could not run (it logs
7+
* and swallows, deliberately, so a migration failure cannot take boot down).
8+
*
9+
* Since #3912 the write path canonicalises, so the legacy forms cannot be
10+
* produced through `create()` any more. They have to be inserted RAW, and the
11+
* column's canonical marker has to be cleared, or the driver would correctly
12+
* conclude the column is clean and skip the read-side repair. Both halves are
13+
* needed for the fixture to be honest — which is exactly why they live in one
14+
* helper rather than being re-improvised per suite.
15+
*
16+
* Not exported from the package entry (`index.ts`): this is test-only.
17+
*/
18+
19+
import { SqlDriver } from './sql-driver.js';
20+
21+
export class LegacyStorageDriver extends SqlDriver {
22+
/**
23+
* Insert rows bypassing `formatInput`, so each `datetime` value lands in
24+
* whatever form it is given (a number → INTEGER epoch ms, a string → TEXT),
25+
* then mark `field` as NOT known-canonical so the read paths apply their
26+
* repair — the state of an un-migrated database.
27+
*/
28+
async seedLegacyRows(
29+
table: string,
30+
field: string,
31+
rows: Array<Record<string, unknown>>,
32+
): Promise<void> {
33+
await this.knex(table).insert(rows);
34+
this.forgetCanonical(table, field);
35+
}
36+
37+
/** Drop the "already backfilled" marker for one column. */
38+
forgetCanonical(table: string, field: string): void {
39+
this.canonicalDatetimeFields[table]?.delete(field);
40+
}
41+
42+
/** Raw stored form of a column, for asserting on the fixture's premise. */
43+
async storedForms(table: string, field: string): Promise<Array<{ id: string; type: string; value: unknown }>> {
44+
const res: any = await this.knex.raw(
45+
`select id, typeof(??) as t, ?? as v from ?? order by id`,
46+
[field, field, table],
47+
);
48+
const rows = Array.isArray(res) ? res : (res?.rows ?? []);
49+
return rows.map((r: any) => ({ id: r.id, type: r.t, value: r.v }));
50+
}
51+
}

0 commit comments

Comments
 (0)