From c2e2e1e150ce2cbcf61c34e7aa19c47d57cb1235 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 18 Jun 2026 14:57:24 +0800 Subject: [PATCH 1/2] fix(analytics,driver-sql): coerce datetime filter comparands to storage form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/analytics-datetime-epoch-filter.md | 11 ++ .../src/sql-driver-analytics-datetime.test.ts | 102 ++++++++++ .../src/sql-driver-temporal-dialect.test.ts | 69 +++++++ packages/plugins/driver-sql/src/sql-driver.ts | 32 ++++ .../native-sql-datetime-filter.test.ts | 176 ++++++++++++++++++ .../src/analytics-service.ts | 10 + .../services/service-analytics/src/plugin.ts | 44 +++++ .../src/strategies/native-sql-strategy.ts | 93 ++++++++- .../spec/src/contracts/analytics-service.ts | 30 +++ 9 files changed, 559 insertions(+), 8 deletions(-) create mode 100644 .changeset/analytics-datetime-epoch-filter.md create mode 100644 packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts create mode 100644 packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts create mode 100644 packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts diff --git a/.changeset/analytics-datetime-epoch-filter.md b/.changeset/analytics-datetime-epoch-filter.md new file mode 100644 index 0000000000..8137a4bca7 --- /dev/null +++ b/.changeset/analytics-datetime-epoch-filter.md @@ -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. diff --git a/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts b/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts new file mode 100644 index 0000000000..efa031ccdd --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-analytics-datetime.test.ts @@ -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 => { + 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'); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts b/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts new file mode 100644 index 0000000000..bc5811c97e --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-temporal-dialect.test.ts @@ -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'); + } + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 17f5689119..84dc2f7d03 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -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; } @@ -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); diff --git a/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts new file mode 100644 index 0000000000..a020591679 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/native-sql-datetime-filter.test.ts @@ -0,0 +1,176 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression: dashboard time-series charts / "last N months" KPIs that filter a + * `Field.datetime` dimension must NOT silently return zero rows. + * + * Root cause (confirmed): the analytics layer expands relative-date tokens like + * `{12_months_ago}` to ISO date strings (`"2025-06-18"`). Under better-sqlite3 a + * `Field.datetime` column is stored as an INTEGER epoch (ms), so the compiled + * `WHERE col >= '2025-06-18'` is a TEXT-vs-INTEGER affinity compare that is + * ALWAYS false → empty result, even though the data exists. `Field.date` columns + * store ISO TEXT and compare fine. + * + * The fix threads the driver's storage-form coercion into NativeSQLStrategy via + * `StrategyContext.coerceTemporalFilterValue`. These tests assert the strategy: + * 1. binds the epoch-ms value when the hook reports a datetime column (SQLite), + * 2. leaves the ISO string untouched when the hook reports no coercion + * (a `Field.date` text column, OR a native-timestamp dialect like Postgres), + * proving no Postgres regression, + * 3. applies the same handling to `gte`/`lte`/`gt`/`lt`/`equals`, `in`, and the + * `dateRange` (timeDimension) path. + */ + +import { describe, it, expect } from 'vitest'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; + +/** compliance_assessment cube: `assessed` dimension → datetime column `assessed_at`. */ +const cube: Cube = { + name: 'compliance', + title: 'Compliance', + sql: 'compliance_assessment', + measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } }, + dimensions: { + // NB: dimension id `assessed` deliberately differs from column `assessed_at` + // to prove the storage target resolves the real column, not the member name. + assessed: { name: 'assessed', label: 'Assessed', type: 'time', sql: 'assessed_at' }, + score: { name: 'score', label: 'Score', type: 'number', sql: 'score' }, + }, + public: false, +}; + +const EPOCH_2025_06_18 = Date.parse('2025-06-18T00:00:00.000Z'); + +/** + * A hook that mimics the SqlDriver-under-SQLite behaviour: ISO → epoch ms for the + * datetime column, value untouched for everything else. + */ +function sqliteHook(object: string, field: string, value: unknown): unknown { + if (object === 'compliance_assessment' && field === 'assessed_at' && typeof value === 'string') { + const ms = Date.parse(/^\d{4}-\d{2}-\d{2}$/.test(value) ? `${value}T00:00:00.000Z` : value); + return Number.isFinite(ms) ? ms : value; + } + return value; // date text / non-temporal / native timestamp → unchanged +} + +function ctxWith(overrides: Partial): StrategyContext { + return { + getCube: (name) => (name === 'compliance' ? cube : undefined), + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [], + ...overrides, + }; +} + +describe('NativeSQLStrategy — datetime filter storage coercion', () => { + it('binds an ISO `gte` filter on a datetime column as epoch ms (SQLite fix)', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: sqliteHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18' } }, + }; + + const { sql, params } = await strategy.generateSql(query, ctx); + + expect(sql).toContain('assessed_at >= $1'); + // The ISO string was converted to its INTEGER epoch storage form — this is + // the exact value that matches the stored datetime and fixes "No rows". + expect(params).toEqual([EPOCH_2025_06_18]); + expect(typeof params[0]).toBe('number'); + }); + + it('leaves the ISO string untouched when the hook reports no coercion (Postgres / date text — no regression)', async () => { + const strategy = new NativeSQLStrategy(); + // Hook present but returns the value unchanged for this column — the contract + // for a native-timestamp dialect or a `Field.date` text column. + const ctx = ctxWith({ coerceTemporalFilterValue: (_o, _f, v) => v }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18' } }, + }; + + const { params } = await strategy.generateSql(query, ctx); + + // Bound as the ISO TEXT comparand — correct against a native TIMESTAMP or a + // YYYY-MM-DD text date; NOT coerced to an epoch integer (would break Postgres). + expect(params).toEqual(['2025-06-18']); + expect(typeof params[0]).toBe('string'); + }); + + it('is backward-compatible: no hook configured → value bound as-is', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({}); // no coerceTemporalFilterValue + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18' } }, + }; + const { params } = await strategy.generateSql(query, ctx); + expect(params).toEqual(['2025-06-18']); + }); + + it('coerces both bounds of a datetime range (gte + lt)', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: sqliteHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $gte: '2025-06-18', $lt: '2025-07-01' } }, + }; + const { params } = await strategy.generateSql(query, ctx); + expect(params).toEqual([ + EPOCH_2025_06_18, + Date.parse('2025-07-01T00:00:00.000Z'), + ]); + }); + + it('coerces each element of an `in` set on a datetime column', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: sqliteHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { assessed: { $in: ['2025-06-18', '2025-06-19'] } }, + }; + const { sql, params } = await strategy.generateSql(query, ctx); + expect(sql).toContain('IN ($1, $2)'); + expect(params).toEqual([ + EPOCH_2025_06_18, + Date.parse('2025-06-19T00:00:00.000Z'), + ]); + }); + + it('coerces a timeDimension dateRange (BETWEEN) on a datetime column', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: sqliteHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + timeDimensions: [{ dimension: 'assessed', dateRange: ['2025-06-18', '2025-07-01'] }], + }; + const { sql, params } = await strategy.generateSql(query, ctx); + expect(sql).toContain('BETWEEN $1 AND $2'); + expect(params).toEqual([ + EPOCH_2025_06_18, + Date.parse('2025-07-01T00:00:00.000Z'), + ]); + }); + + it('does NOT coerce a non-temporal numeric column', async () => { + const strategy = new NativeSQLStrategy(); + const ctx = ctxWith({ coerceTemporalFilterValue: sqliteHook }); + const query: AnalyticsQuery = { + cube: 'compliance', + measures: ['total'], + where: { score: { $gte: '80' } }, + }; + const { params } = await strategy.generateSql(query, ctx); + // hook returns the string unchanged → falls back to numeric recovery + expect(params).toEqual([80]); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index e6748f175e..4f0aadd5e1 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -107,6 +107,15 @@ export interface AnalyticsServiceConfig { * config hook is a fallback for legacy hand-authored cubes. */ getAllowedRelationships?: (cubeName: string) => Set | undefined; + /** + * Coerce a filter comparand to a temporal column's storage form so a + * relative-date / ISO-string value compares correctly on the active driver + * (SQLite `Field.datetime` → epoch ms; `Field.date` / native timestamp → + * unchanged). Threaded into the StrategyContext and consulted by + * `NativeSQLStrategy` when binding filter values. See the contract docs on + * `StrategyContext.coerceTemporalFilterValue` for the full rationale. + */ + coerceTemporalFilterValue?: (objectName: string, fieldName: string, value: unknown) => unknown; /** * ADR-0021 — optional object-graph resolver used when compiling datasets: * `(baseObject, relationshipName) => relatedObjectName | undefined`. When @@ -221,6 +230,7 @@ export class AnalyticsService implements IAnalyticsService { getAllowedRelationships: (cubeName: string) => this.datasetRegistry.get(cubeName)?.allowedRelationships ?? config.getAllowedRelationships?.(cubeName), + coerceTemporalFilterValue: config.coerceTemporalFilterValue, }; // Build strategy chain (built-in + custom, sorted by priority) diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 4c82001373..bc8002a485 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -33,6 +33,24 @@ interface DataEngineLike { options?: Array<{ value: unknown; label?: string }>; }>; } | undefined; + /** + * Resolve the storage driver backing an object (public ObjectQL accessor). + * Used to delegate temporal filter-value coercion to the driver, which is the + * single source of truth for how a `Field.date`/`Field.datetime` is stored on + * the active dialect. The driver may expose `temporalFilterValue(object, field, + * value)` (SqlDriver does); when absent we leave the value untouched. + */ + getDriverForObject?(objectName: string): DriverLike | undefined; +} + +/** Minimal driver surface the analytics layer probes for temporal coercion. */ +interface DriverLike { + /** + * Coerce a filter comparand to the column's on-disk storage form + * (SQLite `Field.datetime` → epoch ms; `Field.date` → YYYY-MM-DD; native + * timestamp / non-temporal → unchanged). Optional — only SqlDriver implements it. + */ + temporalFilterValue?(objectName: string, field: string, value: unknown): unknown; } /** @@ -381,6 +399,31 @@ export class AnalyticsServicePlugin implements Plugin { return pending ? rows : null; }; + // Temporal storage-form coercion (fixes the SQLite datetime "No rows" bug). + // The raw-SQL strategy binds dashboard relative-date tokens (already expanded + // to ISO strings) directly, bypassing the driver's CRUD coercion. Delegate to + // the driver — the single source of truth for the on-disk storage convention — + // so a `Field.datetime` ISO comparand becomes epoch ms on SQLite, while + // `Field.date` text and native-timestamp (Postgres) columns pass through + // unchanged. Resolved at call time so plugin-init order does not matter. + const coerceTemporalFilterValue = ( + objectName: string, + fieldName: string, + value: unknown, + ): unknown => { + try { + const svc = ctx.getService('data'); + const driver = svc?.getDriverForObject?.(objectName); + if (driver && typeof driver.temporalFilterValue === 'function') { + return driver.temporalFilterValue(objectName, fieldName, value); + } + } catch { + // No data engine / driver, or it doesn't support coercion — leave the + // value as-is (today's behaviour; safe for text/native-timestamp paths). + } + return value; + }; + const config: AnalyticsServiceConfig = { cubes: this.options.cubes, logger: ctx.logger, @@ -390,6 +433,7 @@ export class AnalyticsServicePlugin implements Plugin { fallbackService, getReadScope, getAllowedRelationships: this.options.getAllowedRelationships, + coerceTemporalFilterValue, relationshipResolver, labelResolver, draftRowsResolver, diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index fec0a70894..bf5f521574 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -82,7 +82,10 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (normalizedFilters.length > 0) { for (const filter of normalizedFilters) { const colExpr = this.resolveFieldSql(cube, filter.member, tableName, joins); - const clause = this.buildFilterClause(colExpr, filter.operator, filter.values, params); + // Resolve the (object, column) this member binds against so the value + // can be coerced to the column's storage form (see buildFilterClause). + const target = this.resolveStorageTarget(cube, filter.member, tableName); + const clause = this.buildFilterClause(colExpr, filter.operator, filter.values, params, ctx, target); if (clause) whereClauses.push(clause); } } @@ -94,7 +97,14 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (td.dateRange) { const range = Array.isArray(td.dateRange) ? td.dateRange : [td.dateRange, td.dateRange]; if (range.length === 2) { - params.push(range[0], range[1]); + // Same epoch-vs-text root cause as buildFilterClause: a dateRange on a + // SQLite `Field.datetime` column compares ISO TEXT against an INTEGER + // epoch and matches nothing. Coerce both bounds to the storage form. + const td2 = this.resolveStorageTarget(cube, td.dimension, tableName); + params.push( + this.coerceTemporal(ctx, td2, range[0]), + this.coerceTemporal(ctx, td2, range[1]), + ); whereClauses.push(`${colExpr} BETWEEN $${params.length - 1} AND $${params.length}`); } } @@ -318,7 +328,68 @@ export class NativeSQLStrategy implements AnalyticsStrategy { return fieldName; } - private buildFilterClause(col: string, operator: string, values: string[] | undefined, params: unknown[]): string | null { + /** + * Resolve the (object, column) a filter member binds against, so its + * comparand can be coerced to that column's on-disk storage form. + * + * Mirrors `resolveFieldSql`'s `sql` resolution but yields the *logical* + * target rather than the qualified SQL: + * - A dotted column (`account.region`, emitted for a relation traversal) + * belongs to the JOINED object — resolve the alias → target table via the + * cube's `joins` map (alias `account` → object `crm_account` when + * namespaced) and take the tail as the column. + * - Otherwise the column lives on the cube's BASE table. Use the dimension's + * resolved `sql` (the real column, which may differ from the member name, + * e.g. dimension `assessed` → column `assessed_at`) rather than the member. + */ + private resolveStorageTarget( + cube: Cube, + member: string, + baseTable: string, + ): { object: string; field: string } { + const dim = this.lookupMember(cube, member, 'dimension'); + const measure = dim ? undefined : this.lookupMember(cube, member, 'measure'); + const rawSql = dim?.sql ?? measure?.sql ?? (member.includes('.') ? member.split('.').slice(1).join('.') : member); + + if (rawSql.includes('.')) { + const [alias, ...rest] = rawSql.split('.'); + const object = cube.joins?.[alias]?.name ?? alias; + return { object, field: rest.join('.') }; + } + return { object: baseTable, field: rawSql }; + } + + /** + * Apply the storage-form coercion for a single comparand. Prefers the + * driver-backed `coerceTemporalFilterValue` hook (single source of truth for + * the date/datetime storage convention — see StrategyContext); when the hook + * is absent, or returns the value unchanged (the field is not a temporal + * column, or the dialect stores it as a native timestamp), falls back to the + * generic boolean/number recovery so non-temporal typed columns still bind + * correctly. + */ + private coerceTemporal( + ctx: StrategyContext, + target: { object: string; field: string }, + value: string, + ): unknown { + if (typeof ctx.coerceTemporalFilterValue === 'function') { + const coerced = ctx.coerceTemporalFilterValue(target.object, target.field, value); + // Hook returns the value untouched for non-temporal / native-timestamp + // columns; only short-circuit when it actually changed the value. + if (coerced !== value) return coerced; + } + return coerceFilterValueForSql(value); + } + + private buildFilterClause( + col: string, + operator: string, + values: string[] | undefined, + params: unknown[], + ctx: StrategyContext, + target: { object: string; field: string }, + ): string | null { const opMap: Record = { equals: '=', notEquals: '!=', gt: '>', gte: '>=', lt: '<', lte: '<=', contains: 'LIKE', notContains: 'NOT LIKE', @@ -329,7 +400,10 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (operator === 'in' || operator === 'notIn') { if (!values || values.length === 0) return null; - const placeholders = values.map(v => { params.push(coerceFilterValueForSql(v)); return `$${params.length}`; }).join(', '); + // Dates can legitimately appear in an `in`/`notIn` set (e.g. a multi-day + // KPI), so coerce each element to the column's storage form too — same + // SQLite epoch-vs-text root cause as the scalar operators below. + const placeholders = values.map(v => { params.push(this.coerceTemporal(ctx, target, v)); return `$${params.length}`; }).join(', '); return `${col} ${operator === 'in' ? 'IN' : 'NOT IN'} (${placeholders})`; } @@ -339,10 +413,13 @@ export class NativeSQLStrategy implements AnalyticsStrategy { if (operator === 'contains' || operator === 'notContains') { params.push(`%${values[0]}%`); } else { - // Coerce so booleans/numbers bind as their native SQL types - // (avoids '1' (text) vs 1 (integer) mismatches against typed - // boolean columns under SQLite/Postgres). - params.push(coerceFilterValueForSql(values[0])); + // Coerce so booleans/numbers bind as their native SQL types AND so a + // relative-date / ISO-string comparand on a SQLite `Field.datetime` + // column is converted to its INTEGER epoch storage form. Without this a + // dashboard filter like `assessed_at >= '2025-06-18'` compiles to a + // TEXT-vs-INTEGER affinity compare that is always false → "No rows", + // even though the rows exist (the confirmed time-series chart bug). + params.push(this.coerceTemporal(ctx, target, values[0])); } return `${col} ${sqlOp} $${params.length}`; } diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index 22c31b131c..d7c9dab12f 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -292,6 +292,36 @@ export interface StrategyContext { * Cube definitions that pre-date datasets). */ getAllowedRelationships?(cubeName: string): Set | undefined; + + /** + * Coerce a filter comparand to the storage form of a temporal column on the + * object backing the query, so a relative-date / ISO-string value (e.g. the + * `{12_months_ago}` dashboard token expanded to `"2025-06-18"`) compares + * correctly against the column on the active driver. + * + * Why this exists: `NativeSQLStrategy` compiles a raw `SELECT … WHERE col >= $N` + * and binds the value directly, bypassing the driver's own CRUD coercion. Under + * the better-sqlite3 driver a `Field.datetime` column is stored as an INTEGER + * epoch (ms), so `col >= '2025-06-18'` is a TEXT-vs-INTEGER affinity compare + * that is *always false* → empty result (the silent "No rows" bug). This hook + * lets the strategy ask the driver for the storage-correct value instead. + * + * Driver/dialect correctness lives entirely behind this hook (single source of + * truth = the driver): + * - SQLite `Field.datetime` → epoch milliseconds (number). + * - `Field.date` (any dialect) → `YYYY-MM-DD` text. + * - native-timestamp dialects (Postgres/MySQL) and non-temporal fields → + * the value is returned UNCHANGED, so the already-correct text/timestamp + * comparison is preserved and Postgres is never given an epoch integer. + * + * When the hook is absent (legacy wiring, non-SQL drivers) the strategy binds + * the value as-is — exactly today's behaviour — so it is purely additive. + * + * @param objectName Logical object / table backing the cube. + * @param fieldName Bare column name the filter targets. + * @param value The stringified comparand from the normalized filter. + */ + coerceTemporalFilterValue?(objectName: string, fieldName: string, value: unknown): unknown; } /** From bcd8d8884066cc2233f3c94453594ed981069d81 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 18 Jun 2026 20:16:40 +0800 Subject: [PATCH 2/2] docs(adr-0053): record analytics raw-SQL temporal-coercion increment + follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/adr/0053-date-and-datetime-semantics.md | 70 ++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/docs/adr/0053-date-and-datetime-semantics.md b/docs/adr/0053-date-and-datetime-semantics.md index 819fbfa9e5..d5903034ec 100644 --- a/docs/adr/0053-date-and-datetime-semantics.md +++ b/docs/adr/0053-date-and-datetime-semantics.md @@ -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} $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.