Skip to content

Commit c2e2e1e

Browse files
os-zhuangclaude
andcommitted
fix(analytics,driver-sql): coerce datetime filter comparands to storage 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>
1 parent 4b6430e commit c2e2e1e

9 files changed

Lines changed: 559 additions & 8 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
"@objectstack/driver-sql": patch
4+
"@objectstack/spec": patch
5+
---
6+
7+
Fix dashboard time-series charts / "last N months" KPIs that filter or group by a `Field.datetime` column silently returning "No rows".
8+
9+
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.
10+
11+
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.
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* End-to-end repro of the dashboard time-series "No rows" bug at the storage
5+
* level, and proof of the fix.
6+
*
7+
* The analytics `NativeSQLStrategy` compiles dashboard relative-date tokens
8+
* (e.g. `{12_months_ago}`) to ISO date strings and binds them into a raw
9+
* `SELECT … WHERE col >= ?` that it runs through the driver's `execute()` —
10+
* bypassing the normal `find()` filter coercion. Under better-sqlite3 a
11+
* `Field.datetime` column is stored as an INTEGER epoch (ms), so the ISO TEXT
12+
* comparand never matches (TEXT sorts after every INTEGER) → 0 rows, even though
13+
* the rows exist. A `Field.date` column stores ISO TEXT and matches fine.
14+
*
15+
* This test reproduces both the broken (raw ISO bind → 0) and fixed (epoch bind
16+
* via the driver's public `temporalFilterValue` → N) behaviour against a real
17+
* SQLite database, mirroring exactly what the analytics strategy now does.
18+
*/
19+
20+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
21+
import { SqlDriver } from '../src/index.js';
22+
23+
describe('Analytics datetime filter — SQLite epoch storage (E2E repro)', () => {
24+
let driver: SqlDriver;
25+
const TABLE = 'compliance_assessment';
26+
const CUTOFF = '2025-06-18'; // ISO date token the dashboard expands to
27+
28+
beforeEach(async () => {
29+
driver = new SqlDriver({
30+
client: 'better-sqlite3',
31+
connection: { filename: ':memory:' },
32+
useNullAsDefault: true,
33+
});
34+
35+
await driver.initObjects([
36+
{
37+
name: TABLE,
38+
fields: {
39+
title: { type: 'string' },
40+
assessed_at: { type: 'datetime' }, // stored as INTEGER epoch ms
41+
assessed_on: { type: 'date' }, // stored as YYYY-MM-DD text
42+
},
43+
},
44+
]);
45+
46+
// Four assessments AFTER the cutoff, one well before — inserted with real
47+
// Date objects so better-sqlite3 stores `assessed_at` as INTEGER epoch ms,
48+
// exactly the path the seed loader takes.
49+
const rows = [
50+
['a1', new Date('2024-01-01T00:00:00Z'), '2024-01-01'], // before cutoff
51+
['a2', new Date('2025-06-18T09:00:00Z'), '2025-06-18'], // on/after
52+
['a3', new Date('2025-09-01T09:00:00Z'), '2025-09-01'],
53+
['a4', new Date('2026-01-15T09:00:00Z'), '2026-01-15'],
54+
['a5', new Date('2026-05-20T09:00:00Z'), '2026-05-20'],
55+
] as const;
56+
for (const [id, at, on] of rows) {
57+
await driver.create(
58+
TABLE,
59+
{ id, title: id, assessed_at: at, assessed_on: on },
60+
{ bypassTenantAudit: true },
61+
);
62+
}
63+
});
64+
65+
afterEach(async () => {
66+
await driver.disconnect();
67+
});
68+
69+
const countWhere = async (col: string, bind: unknown): Promise<number> => {
70+
const res: any = await driver.execute(
71+
`SELECT count(*) AS n FROM "${TABLE}" WHERE "${col}" >= ?`,
72+
[bind],
73+
);
74+
const row = Array.isArray(res) ? res[0] : res?.rows?.[0] ?? res;
75+
return Number(row.n);
76+
};
77+
78+
it('BUG: a raw ISO comparand against the epoch datetime column returns 0 rows', async () => {
79+
// This is what the type-blind strategy used to bind — the silent failure.
80+
expect(await countWhere('assessed_at', CUTOFF)).toBe(0);
81+
});
82+
83+
it('FIX: the driver-coerced epoch comparand returns the 4 matching rows', async () => {
84+
// `temporalFilterValue` is exactly the hook NativeSQLStrategy now calls.
85+
const coerced = driver.temporalFilterValue(TABLE, 'assessed_at', CUTOFF);
86+
expect(typeof coerced).toBe('number'); // epoch ms, not the ISO string
87+
expect(await countWhere('assessed_at', coerced)).toBe(4);
88+
});
89+
90+
it('CONTROL: the `Field.date` text column already matched the raw ISO comparand', async () => {
91+
// Proves the date/text path was never broken and is left untouched.
92+
const coerced = driver.temporalFilterValue(TABLE, 'assessed_on', CUTOFF);
93+
expect(typeof coerced).toBe('string'); // YYYY-MM-DD, NOT coerced to epoch
94+
expect(await countWhere('assessed_on', coerced)).toBe(4);
95+
// and the raw ISO bind matches identically (no coercion needed for text)
96+
expect(await countWhere('assessed_on', CUTOFF)).toBe(4);
97+
});
98+
99+
it('does not touch a non-temporal column', () => {
100+
expect(driver.temporalFilterValue(TABLE, 'title', 'hello')).toBe('hello');
101+
});
102+
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Dialect-correctness of `temporalFilterValue` (the hook the analytics layer
5+
* uses). The datetime → epoch-ms coercion is SQLite-ONLY: SQLite stores
6+
* `Field.datetime` as an INTEGER epoch, but Postgres/MySQL map it to a native
7+
* TIMESTAMP where an ISO string / Date binds correctly. Coercing to an epoch
8+
* integer on a native-timestamp dialect would compare INTEGER vs TIMESTAMP and
9+
* break the query — the exact Postgres regression we must NOT introduce.
10+
*
11+
* No DB connection is needed: we seed the field-type maps the way `initObjects`
12+
* would and exercise the pure coercion logic across dialects.
13+
*/
14+
15+
import { describe, it, expect } from 'vitest';
16+
import { SqlDriver } from '../src/index.js';
17+
18+
/** Test double that injects the field-type metadata without a live connection. */
19+
class ProbeDriver extends SqlDriver {
20+
seedDatetime(table: string, field: string): void {
21+
(this.datetimeFields[table] ??= new Set()).add(field);
22+
}
23+
seedDate(table: string, field: string): void {
24+
(this.dateFields[table] ??= new Set()).add(field);
25+
}
26+
}
27+
28+
function makeDriver(client: string): ProbeDriver {
29+
// Connection is never opened — we only call the synchronous coercion path.
30+
return new ProbeDriver({ client, connection: { filename: ':memory:' }, useNullAsDefault: true } as any);
31+
}
32+
33+
const ISO = '2025-06-18';
34+
const EPOCH = Date.parse('2025-06-18T00:00:00.000Z');
35+
36+
describe('temporalFilterValue dialect gating', () => {
37+
it('SQLite: datetime ISO comparand → epoch ms', () => {
38+
const d = makeDriver('better-sqlite3');
39+
d.seedDatetime('t', 'at');
40+
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(EPOCH);
41+
});
42+
43+
it('Postgres: datetime ISO comparand is LEFT UNCHANGED (no epoch coercion → no regression)', () => {
44+
const d = makeDriver('pg');
45+
d.seedDatetime('t', 'at');
46+
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO);
47+
});
48+
49+
it('MySQL: datetime ISO comparand is left unchanged', () => {
50+
const d = makeDriver('mysql2');
51+
d.seedDatetime('t', 'at');
52+
expect(d.temporalFilterValue('t', 'at', ISO)).toBe(ISO);
53+
});
54+
55+
it('Field.date normalises to YYYY-MM-DD text on every dialect', () => {
56+
for (const client of ['better-sqlite3', 'pg', 'mysql2']) {
57+
const d = makeDriver(client);
58+
d.seedDate('t', 'on');
59+
expect(d.temporalFilterValue('t', 'on', '2025-06-18T12:00:00Z')).toBe('2025-06-18');
60+
}
61+
});
62+
63+
it('non-temporal fields pass through unchanged on every dialect', () => {
64+
for (const client of ['better-sqlite3', 'pg', 'mysql2']) {
65+
const d = makeDriver(client);
66+
expect(d.temporalFilterValue('t', 'name', 'hello')).toBe('hello');
67+
}
68+
});
69+
});

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,6 +1614,14 @@ export class SqlDriver implements IDataDriver {
16141614
};
16151615

16161616
if (isDatetime) {
1617+
// Only SQLite stores `Field.datetime` as an INTEGER epoch (better-sqlite3
1618+
// binds a JS `Date` as `.getTime()`); there the ISO/text comparand MUST be
1619+
// coerced to epoch ms or it collapses to a TEXT-vs-INTEGER affinity compare
1620+
// that never matches. Postgres/MySQL map datetime to a native TIMESTAMP
1621+
// (see `defineColumn` → `table.timestamp`), where Knex binds an ISO string
1622+
// or `Date` correctly — coercing to an epoch integer there would compare an
1623+
// INTEGER against a TIMESTAMP and break the query. So gate on dialect.
1624+
if (!this.isSqlite) return value;
16171625
const ms = toMs(value);
16181626
return ms == null ? value : ms;
16191627
}
@@ -1622,6 +1630,30 @@ export class SqlDriver implements IDataDriver {
16221630
return this.toDateOnly(value);
16231631
}
16241632

1633+
/**
1634+
* Public, dialect-correct temporal filter-value coercion for callers that
1635+
* build SQL *outside* the normal `find()`/`applyFilters()` path — chiefly the
1636+
* analytics native-SQL strategy, which compiles a raw `SELECT … WHERE col >= $N`
1637+
* and binds the value directly, bypassing `coerceFilterValue`.
1638+
*
1639+
* Given a logical object (table) name, a field name and a filter value
1640+
* (typically an ISO date/datetime string from a dashboard relative-date
1641+
* token like `{12_months_ago}`), this returns the value in the column's
1642+
* on-disk storage form:
1643+
* - SQLite `Field.datetime` → epoch milliseconds (INTEGER), so the
1644+
* comparison matches the stored integer rather than failing a
1645+
* TEXT-vs-INTEGER affinity compare.
1646+
* - `Field.date` (any dialect) → `YYYY-MM-DD` text.
1647+
* - Native-timestamp dialects / non-temporal fields → value unchanged.
1648+
*
1649+
* This is a thin, intentionally narrow wrapper over the same `coerceFilterValue`
1650+
* the driver already uses, so there is exactly one source of truth for the
1651+
* storage convention and the analytics path can never drift from CRUD.
1652+
*/
1653+
public temporalFilterValue(objectName: string, field: string, value: any): any {
1654+
return this.coerceFilterValue(objectName, field, value);
1655+
}
1656+
16251657
protected applyFilters(builder: Knex.QueryBuilder, filters: any) {
16261658
if (!filters) return;
16271659
const table = this.tableNameForBuilder(builder);

0 commit comments

Comments
 (0)