Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/analytics-datetime-epoch-filter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@objectstack/service-analytics": patch
"@objectstack/driver-sql": patch
"@objectstack/spec": patch
---

Fix dashboard time-series charts / "last N months" KPIs that filter or group by a `Field.datetime` column silently returning "No rows".

The analytics `NativeSQLStrategy` compiles dashboard relative-date tokens (`{12_months_ago}`, `{today}`, …) to ISO date strings and binds them directly into raw SQL, bypassing the driver's own filter coercion. Under better-sqlite3 a `Field.datetime` column is stored as an INTEGER epoch (ms), so `assessed_at >= '2025-06-18'` became a TEXT-vs-INTEGER affinity compare that is always false — an empty result even though the rows exist. `Field.date` columns store ISO TEXT and were unaffected.

The strategy now coerces a temporal comparand to the column's on-disk storage form via a new optional `StrategyContext.coerceTemporalFilterValue` hook, wired to the driver's public `SqlDriver.temporalFilterValue` (the single source of truth for the storage convention). 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. Applied to `gte`/`lte`/`gt`/`lt`/`equals`, `in`/`notIn`, and the `dateRange`/timeDimension `BETWEEN` path.
70 changes: 70 additions & 0 deletions docs/adr/0053-date-and-datetime-semantics.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,73 @@ Cron day-boundaries (`sys_job`) need **no change** — already tz-wired via cron
Every slice is feature-flaggable behind "reference timezone unset → UTC". With no org
reference timezone configured, the resolver returns `'UTC'` and all compute/render
paths are byte-for-byte today's behavior — the safe default and the rollback target.

---

## Addendum (2026-06-18) — the analytics raw-SQL filter path and the temporal-coercion contract

> **Status:** first increment landed (commit `6f4cf856e`, branch
> `fix/analytics-datetime-epoch-filter`). This addendum records a gap ADR-0053 did
> **not** reach and the contract follow-ups it implies. It extends, and does not
> revise, the decision above.

### The gap

ADR-0053 fixed the `date`-as-string-vs-instant family (#1874) on the driver CRUD
path, and Phase 1 explicitly left `Field.datetime` stored as UTC epoch ms
(`sql-driver.ts:1500`, decision step 4). But analytics has a **second filter
surface that never touches that coercion**: `NativeSQLStrategy` builds raw SQL and
runs it via `engine.execute`, bypassing the driver's dialect-aware
`coerceFilterValue` (`sql-driver.ts:1543`). `buildFilterClause` emits
`${col} <op> $N` and binds the comparand directly
(`native-sql-strategy.ts:385-425`); the only type recovery was
`coerceFilterValueForSql`, which re-derives a type by **regex on the value's
shape** — no schema type, no date branch (`filter-normalizer.ts:127-140`).

So a dashboard relative-date token resolved to an ISO string (`"2025-06-18"`),
filtered against a `Field.datetime` column stored as an INTEGER epoch on SQLite,
compiled to `epoch >= 'ISO'` — a TEXT-vs-INTEGER affinity compare that is **always
false → 0 rows / empty chart**. This is the `datetime` analogue of the `date`
equality miss Phase 1 fixed, on the one path 0053 did not address.

### Decisions

**D-A1 — The driver is the single source of dialect truth for filter-value
coercion; every raw-SQL surface routes through it.** Invariant: any surface that
binds a filter comparand into raw SQL (analytics `NativeSQLStrategy` today, and any
future raw-SQL strategy) **must** coerce through the driver's dialect-aware
temporal coercion, never re-derive a type from the value's textual shape. This
closes the `datetime` analogue of Phase 1's `date` fix on the path 0053 did not
reach. The first increment — commit `6f4cf856e` (branch
`fix/analytics-datetime-epoch-filter`) — exposes the driver's coercion to
analytics via a new `StrategyContext.coerceTemporalFilterValue(object, field,
value)` hook delegating to the driver, applied across `gte/lte/gt/lt/equals`,
`in/notIn`, and the `dateRange`/timeDimension path (`native-sql-strategy.ts:371`,
`:88-106`). SQLite `datetime` → epoch ms; `date` text and native-timestamp
dialects (Postgres/MySQL) pass through unchanged. **Record this PR as ledger
evidence** — the same enforce-resolution pattern D3 uses for the dead schedule
fields.

**D-A2 — Formalize `temporalFilterValue` onto the `IDataDriver` contract.** The
hook currently delegates to a **duck-typed** `driver.temporalFilterValue(...)`
that is not on the driver contract. Promote it to a first-class
`IDataDriver`-contract method so every consumer relies on a stable surface, and
**demote the regex-shape `coerceFilterValueForSql` to a last-resort fallback (or
retire it)** once the contract method is universal. (If an in-flight
`IDataDriver`-interface change is open, align this with it; do not block on it.)

**D-A3 — Add a temporal conformance matrix as the runtime regression backstop.**
Cover `field-type {date, datetime} × operator {eq, gte/lte/gt/lt, in, dateRange} ×
relative-token {today, N_days_ago, N_months_ago, …} × driver {SQLite, Postgres at
minimum}`, asserting correct **row results** — not just emitted SQL. Analytics has
been refactored repeatedly; this seam must not silently regress. This complements
the #1950 build-time lint ADR-0053 already references: the lint warns at author
time, the matrix proves runtime correctness across drivers.

### Consequences

- The `datetime`-on-raw-SQL filter bug is closed at the driver boundary, mirroring
Phase 1's "align the consumer with the driver's existing contract rather than
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.
102 changes: 102 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// 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.
*
* 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.
*
* 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.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';

describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => {
let driver: SqlDriver;
const TABLE = 'compliance_assessment';
const CUTOFF = '2025-06-18'; // ISO date token the dashboard expands to

beforeEach(async () => {
driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
});

await driver.initObjects([
{
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
},
},
]);

// 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.
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
['a3', new Date('2025-09-01T09:00:00Z'), '2025-09-01'],
['a4', new Date('2026-01-15T09:00:00Z'), '2026-01-15'],
['a5', new Date('2026-05-20T09:00:00Z'), '2026-05-20'],
] as const;
for (const [id, at, on] of rows) {
await driver.create(
TABLE,
{ id, title: id, assessed_at: at, assessed_on: on },
{ bypassTenantAudit: true },
);
}
});

afterEach(async () => {
await driver.disconnect();
});

const countWhere = async (col: string, bind: unknown): Promise<number> => {
const res: any = await driver.execute(
`SELECT count(*) AS n FROM "${TABLE}" WHERE "${col}" >= ?`,
[bind],
);
const row = Array.isArray(res) ? res[0] : res?.rows?.[0] ?? res;
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('FIX: the driver-coerced epoch comparand returns the 4 matching rows', async () => {
// `temporalFilterValue` is exactly the hook NativeSQLStrategy now calls.
const coerced = driver.temporalFilterValue(TABLE, 'assessed_at', CUTOFF);
expect(typeof coerced).toBe('number'); // epoch ms, not the ISO string
expect(await countWhere('assessed_at', coerced)).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(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);
});

it('does not touch a non-temporal column', () => {
expect(driver.temporalFilterValue(TABLE, 'title', 'hello')).toBe('hello');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* 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.
*
* No DB connection is needed: we seed the field-type maps the way `initObjects`
* would and exercise the pure coercion logic across dialects.
*/

import { describe, it, expect } from 'vitest';
import { SqlDriver } from '../src/index.js';

/** Test double that injects the field-type metadata without a live connection. */
class ProbeDriver extends SqlDriver {
seedDatetime(table: string, field: string): void {
(this.datetimeFields[table] ??= new Set()).add(field);
}
seedDate(table: string, field: string): void {
(this.dateFields[table] ??= new Set()).add(field);
}
}

function makeDriver(client: string): ProbeDriver {
// Connection is never opened — we only call the synchronous coercion path.
return new ProbeDriver({ client, connection: { filename: ':memory:' }, useNullAsDefault: true } as any);
}

const ISO = '2025-06-18';
const EPOCH = Date.parse('2025-06-18T00:00:00.000Z');

describe('temporalFilterValue dialect gating', () => {
it('SQLite: datetime ISO comparand → epoch ms', () => {
const d = makeDriver('better-sqlite3');
d.seedDatetime('t', 'at');
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(EPOCH);
});

it('Postgres: datetime ISO comparand is LEFT UNCHANGED (no epoch coercion → no regression)', () => {
const d = makeDriver('pg');
d.seedDatetime('t', 'at');
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO);
});

it('MySQL: datetime ISO comparand is left unchanged', () => {
const d = makeDriver('mysql2');
d.seedDatetime('t', 'at');
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO);
});

it('Field.date normalises to YYYY-MM-DD text on every dialect', () => {
for (const client of ['better-sqlite3', 'pg', 'mysql2']) {
const d = makeDriver(client);
d.seedDate('t', 'on');
expect(d.temporalFilterValue('t', 'on', '2025-06-18T12:00:00Z')).toBe('2025-06-18');
}
});

it('non-temporal fields pass through unchanged on every dialect', () => {
for (const client of ['better-sqlite3', 'pg', 'mysql2']) {
const d = makeDriver(client);
expect(d.temporalFilterValue('t', 'name', 'hello')).toBe('hello');
}
});
});
32 changes: 32 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,14 @@ export class SqlDriver implements IDataDriver {
};

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 dialect.
if (!this.isSqlite) return value;
const ms = toMs(value);
return ms == null ? value : ms;
}
Expand All @@ -1622,6 +1630,30 @@ export class SqlDriver implements IDataDriver {
return this.toDateOnly(value);
}

/**
* Public, dialect-correct temporal filter-value coercion for callers that
* build SQL *outside* the normal `find()`/`applyFilters()` path — chiefly the
* analytics native-SQL strategy, which compiles a raw `SELECT … WHERE col >= $N`
* and binds the value directly, bypassing `coerceFilterValue`.
*
* Given a logical object (table) name, a field name and a filter value
* (typically an ISO date/datetime string from a dashboard relative-date
* token like `{12_months_ago}`), this returns the value in the column's
* on-disk storage form:
* - SQLite `Field.datetime` → epoch milliseconds (INTEGER), so the
* comparison matches the stored integer rather than failing a
* TEXT-vs-INTEGER affinity compare.
* - `Field.date` (any dialect) → `YYYY-MM-DD` text.
* - Native-timestamp dialects / non-temporal fields → value unchanged.
*
* This is a thin, intentionally narrow wrapper over the same `coerceFilterValue`
* the driver already uses, so there is exactly one source of truth for the
* storage convention and the analytics path can never drift from CRUD.
*/
public temporalFilterValue(objectName: string, field: string, value: any): any {
return this.coerceFilterValue(objectName, field, value);
}

protected applyFilters(builder: Knex.QueryBuilder, filters: any) {
if (!filters) return;
const table = this.tableNameForBuilder(builder);
Expand Down
Loading
Loading