Skip to content

Commit bcd8d88

Browse files
os-zhuangclaude
andcommitted
docs(adr-0053): record analytics raw-SQL temporal-coercion increment + follow-ups
Append an addendum to ADR-0053 covering the gap it did not reach: the analytics NativeSQLStrategy raw-SQL path bypasses the driver's dialect-aware filter coercion, producing the datetime analogue of Phase 1's date equality miss (epoch-vs-ISO-string → 0 rows). Records the hotfix increment (6f4cf856e) and two follow-ups: formalize temporalFilterValue onto the IDataDriver contract, and add a cross-driver temporal conformance matrix as the runtime regression backstop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c2e2e1e commit bcd8d88

1 file changed

Lines changed: 70 additions & 0 deletions

File tree

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,3 +332,73 @@ Cron day-boundaries (`sys_job`) need **no change** — already tz-wired via cron
332332
Every slice is feature-flaggable behind "reference timezone unset → UTC". With no org
333333
reference timezone configured, the resolver returns `'UTC'` and all compute/render
334334
paths are byte-for-byte today's behavior — the safe default and the rollback target.
335+
336+
---
337+
338+
## Addendum (2026-06-18) — the analytics raw-SQL filter path and the temporal-coercion contract
339+
340+
> **Status:** first increment landed (commit `6f4cf856e`, branch
341+
> `fix/analytics-datetime-epoch-filter`). This addendum records a gap ADR-0053 did
342+
> **not** reach and the contract follow-ups it implies. It extends, and does not
343+
> revise, the decision above.
344+
345+
### The gap
346+
347+
ADR-0053 fixed the `date`-as-string-vs-instant family (#1874) on the driver CRUD
348+
path, and Phase 1 explicitly left `Field.datetime` stored as UTC epoch ms
349+
(`sql-driver.ts:1500`, decision step 4). But analytics has a **second filter
350+
surface that never touches that coercion**: `NativeSQLStrategy` builds raw SQL and
351+
runs it via `engine.execute`, bypassing the driver's dialect-aware
352+
`coerceFilterValue` (`sql-driver.ts:1543`). `buildFilterClause` emits
353+
`${col} <op> $N` and binds the comparand directly
354+
(`native-sql-strategy.ts:385-425`); the only type recovery was
355+
`coerceFilterValueForSql`, which re-derives a type by **regex on the value's
356+
shape** — no schema type, no date branch (`filter-normalizer.ts:127-140`).
357+
358+
So a dashboard relative-date token resolved to an ISO string (`"2025-06-18"`),
359+
filtered against a `Field.datetime` column stored as an INTEGER epoch on SQLite,
360+
compiled to `epoch >= 'ISO'` — a TEXT-vs-INTEGER affinity compare that is **always
361+
false → 0 rows / empty chart**. This is the `datetime` analogue of the `date`
362+
equality miss Phase 1 fixed, on the one path 0053 did not address.
363+
364+
### Decisions
365+
366+
**D-A1 — The driver is the single source of dialect truth for filter-value
367+
coercion; every raw-SQL surface routes through it.** Invariant: any surface that
368+
binds a filter comparand into raw SQL (analytics `NativeSQLStrategy` today, and any
369+
future raw-SQL strategy) **must** coerce through the driver's dialect-aware
370+
temporal coercion, never re-derive a type from the value's textual shape. This
371+
closes the `datetime` analogue of Phase 1's `date` fix on the path 0053 did not
372+
reach. The first increment — commit `6f4cf856e` (branch
373+
`fix/analytics-datetime-epoch-filter`) — exposes the driver's coercion to
374+
analytics via a new `StrategyContext.coerceTemporalFilterValue(object, field,
375+
value)` hook delegating to the driver, applied across `gte/lte/gt/lt/equals`,
376+
`in/notIn`, and the `dateRange`/timeDimension path (`native-sql-strategy.ts:371`,
377+
`:88-106`). SQLite `datetime` → epoch ms; `date` text and native-timestamp
378+
dialects (Postgres/MySQL) pass through unchanged. **Record this PR as ledger
379+
evidence** — the same enforce-resolution pattern D3 uses for the dead schedule
380+
fields.
381+
382+
**D-A2 — Formalize `temporalFilterValue` onto the `IDataDriver` contract.** The
383+
hook currently delegates to a **duck-typed** `driver.temporalFilterValue(...)`
384+
that is not on the driver contract. Promote it to a first-class
385+
`IDataDriver`-contract method so every consumer relies on a stable surface, and
386+
**demote the regex-shape `coerceFilterValueForSql` to a last-resort fallback (or
387+
retire it)** once the contract method is universal. (If an in-flight
388+
`IDataDriver`-interface change is open, align this with it; do not block on it.)
389+
390+
**D-A3 — Add a temporal conformance matrix as the runtime regression backstop.**
391+
Cover `field-type {date, datetime} × operator {eq, gte/lte/gt/lt, in, dateRange} ×
392+
relative-token {today, N_days_ago, N_months_ago, …} × driver {SQLite, Postgres at
393+
minimum}`, asserting correct **row results** — not just emitted SQL. Analytics has
394+
been refactored repeatedly; this seam must not silently regress. This complements
395+
the #1950 build-time lint ADR-0053 already references: the lint warns at author
396+
time, the matrix proves runtime correctness across drivers.
397+
398+
### Consequences
399+
400+
- The `datetime`-on-raw-SQL filter bug is closed at the driver boundary, mirroring
401+
Phase 1's "align the consumer with the driver's existing contract rather than
402+
inventing a semantic" stance. No change to Phase 2's reference-timezone plan.
403+
- Until D-A2 lands, the hook depends on a duck-typed driver method — a known,
404+
intentionally-temporary seam tracked here.

0 commit comments

Comments
 (0)