fix(analytics,driver-sql): coerce datetime filter comparands to storage form#2034
Merged
Conversation
…ge form
Dashboard time-series charts and "last N months" KPIs that filter/group by a
`Field.datetime` column silently returned "No rows" even though data existed;
`Field.date` columns worked.
Root cause: `NativeSQLStrategy` expands dashboard relative-date tokens
(`{12_months_ago}`, `{today}`, …) to ISO date strings and binds them directly
into raw SQL, bypassing the driver's CRUD filter coercion. Under better-sqlite3
a `Field.datetime` column is stored as an INTEGER epoch (ms), so
`assessed_at >= '2025-06-18'` is a TEXT-vs-INTEGER affinity compare that is
always false → empty result. `Field.date` stores ISO TEXT and compared fine.
Fix: make the comparand coercion type- and storage-aware via a new optional
`StrategyContext.coerceTemporalFilterValue` hook, wired by the analytics plugin
to the driver's public `SqlDriver.temporalFilterValue` — the single source of
truth for the storage convention (reuses the existing `coerceFilterValue`).
Coercion is dialect-correct: SQLite `Field.datetime` → epoch ms; `Field.date`
text and native-timestamp dialects (Postgres/MySQL) are left unchanged, so
Postgres is never handed an epoch integer. Also gated the driver's existing
datetime→epoch coercion on `isSqlite` to make the native-timestamp path correct.
Applied to gte/lte/gt/lt/equals, in/notIn, and the dateRange/timeDimension path.
Tests:
- service-analytics: NativeSQLStrategy binds epoch for a datetime `gte`/range/in,
leaves ISO text unchanged when the hook reports no coercion (Postgres/date),
and is backward-compatible with no hook.
- driver-sql: E2E SQLite repro proving 0→4 rows once coerced; dialect-gating unit
test asserting Postgres/MySQL are NOT epoch-coerced (no regression).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ 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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 95 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Dashboard time-series charts and "last N months/days" KPIs that filter or group by a
Field.datetimecolumn silently returned 0 rows on SQLite, even though the data existed (charts onField.datecolumns worked).Root cause: the analytics
NativeSQLStrategybuilds raw SQL and runs it viaengine.execute, which bypasses the driver's dialect-aware filter coercion (coerceFilterValueindriver-sql). Dashboard relative-date tokens ({12_months_ago}, …) resolve to ISO date strings ("2025-06-18");Field.datetimeunder better-sqlite3 stores an integer epoch (ms), and in SQLite an INTEGER always sorts before any TEXT, soepoch >= 'ISO'is always false. This is thedatetimeanalogue of thedatewrite/filter asymmetry that ADR-0053 Phase 1 fixed — on the analytics-raw-SQL path ADR-0053 didn't reach (same bug family #1874).Fix
The driver is the single source of dialect truth for filter-value coercion; the analytics raw-SQL path now routes through it instead of re-deriving types from value shape.
driver-sql: gate the datetime→epoch coercion behindisSqlite(native-timestamp dialects leave the ISO/Date comparand intact → Postgres/MySQL correct), and exposetemporalFilterValue(object, field, value)delegating to the existingcoerceFilterValue(one code path).service-analytics:NativeSQLStrategyresolves the real(object, column)each member binds against and coerces temporal filter values via the driver, threaded intobuildFilterClause(gte/lte/gt/lt/equals,in/notIn) and thedateRangeBETWEEN path. NewStrategyContext.coerceTemporalFilterValuehook wired from the data engine.spec: added thecoerceTemporalFilterValuecontract hook.No
CAST(col …)is emitted (indexes preserved). When the hook is absent, behaviour is byte-for-byte identical to before.Verification
assessed_at >= '2025-06-18'(datetime) 0 → 4 after driver coercion;Field.datetext path unchanged (4).dateare not (no regression).main:driver-sql169 passed,service-analytics134 passed; spec/driver-sql/service-analytics build green.ADR
Records this as the first increment in a new Addendum to ADR-0053 (driver-as-single-source-of-dialect-truth; formalize
temporalFilterValueonto theIDataDrivercontract; add atype × operator × token × driverconformance matrix as the runtime regression backstop).🤖 Generated with Claude Code