Skip to content

Commit 944bf7a

Browse files
committed
fix(service-analytics): the matrix's sixth consumer, and the dropped $between it found (ADR-0053 D-A3.1)
NativeSQLStrategy is the surface #3650 was actually about, and it was listed in the conformance matrix's own backend table with no consumer — because every existing suite for it asserts the emitted SQL string, and a DROPPED predicate is invisible to a string assertion: the SQL stays valid, just wider. Executing the shared cases against a real engine (sql.js, the same pure-WASM engine driver-sql falls back to) and asserting row ids is what D-A3 asked for, and it immediately showed `$between` returning the entire table. The cause sat one layer below the strategy, in the shared filter-normalizer: `$between` was absent from MONGO_TO_CUBE_OP, and an unmapped operator is `continue`d, so the predicate vanished from the compiled WHERE clause. Both strategies read that normalizer, so the ObjectQL aggregate path was affected too. User-visible symptom: a widget with a range filter charts the whole dataset, with nothing in the SQL to suggest a filter was ever requested. `$between [min, max]` now LOWERS to its two bounds (gte + lte) rather than gaining an operator of its own, so a range's max inherits the calendar-day whole-day rule (#3777) from each strategy's existing upper-bound handling — NativeSQLStrategy compiles a bare-day upper bound half-open itself, the ObjectQL path gets the same rule from the driver — instead of needing a second implementation to keep in step. That is how #4098 closed the same defect on the preview evaluator. A malformed `$between` throws rather than being dropped, the stance driver-memory took for the same shape in #3948. The consumer runs the literal, relative-token and timeDimensions.dateRange spellings of every shared case. Rows are seeded canonical, where the driver's temporal hooks are identities, so the context omits them and exercises the same "absent = identity" path Postgres takes (D-A2); the un-backfilled mixed column stays covered where the driver truth lives. The cause itself is NOT fully closed: `$startsWith`, `$endsWith`, `$null` and `$regex` are still silently dropped by the same `continue`. Filed as #4128 rather than expanded into this change, with the #3948 precedent for turning the fallback into a throw. service-analytics 391 green (was 360; +31 from the new consumer). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqqZmPS5a4gJGBoCTwipFr
1 parent 848b0db commit 944bf7a

4 files changed

Lines changed: 253 additions & 0 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): a `$between` analytics filter no longer vanishes from the query (ADR-0053 D-A3.1)
6+
7+
A dashboard widget or dataset whose filter used `$between` was querying **every
8+
row**. `normalizeAnalyticsFilters` maps Mongo-style operators onto the internal
9+
pipeline form, `$between` was missing from that map, and an unmapped operator is
10+
skipped — so the predicate was silently dropped from the compiled WHERE clause.
11+
Both strategies read that normalizer, so both the raw-SQL and the ObjectQL
12+
aggregate paths were affected. The symptom is #3650's: a chart that draws the
13+
whole dataset instead of the requested window, with nothing in the SQL to
14+
suggest a filter was ever asked for.
15+
16+
`$between [min, max]` now lowers to its two bounds (`gte` + `lte`) instead of
17+
gaining an operator of its own, so a range's max inherits the calendar-day
18+
whole-day rule (#3777) from each strategy's existing upper-bound handling —
19+
`NativeSQLStrategy` compiles a bare-day upper bound half-open itself, and the
20+
ObjectQL path gets the same rule from the driver — rather than needing a second
21+
implementation to keep in step. A malformed `$between` (not a two-element
22+
array) now throws instead of being dropped, matching the stance driver-memory
23+
took for the same shape in #3948: an unbounded read is exactly the failure this
24+
prevents, and it is indistinguishable from a legitimately wide query.
25+
26+
Found by giving the temporal conformance matrix its missing sixth consumer
27+
(`native-sql-temporal-conformance.test.ts`), which executes the shared cases
28+
against a real SQLite engine and asserts row ids — a dropped predicate is
29+
invisible to the SQL-string assertions the strategy's other suites use.

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,25 @@ Two things, which is the argument for having built it:
857857
exists to provide. Fixed in the same change, sharing the `$lte` bound helper
858858
so the two cannot drift again.
859859

860+
**The same defect had a twin one layer over, found the same way when the
861+
sixth consumer was added (#4081 follow-up).** `NativeSQLStrategy` — the
862+
surface #3650 was actually about — was listed in the matrix's own backend
863+
table but had no consumer, because every existing suite for it asserts the
864+
emitted SQL string, and a *dropped* predicate is invisible to a string
865+
assertion: the SQL stays valid, just wider. Executing the matrix against a
866+
real engine showed `$between` returning the **entire table**. The cause was
867+
one layer below the strategy, in the shared `filter-normalizer`: `$between`
868+
was absent from `MONGO_TO_CUBE_OP`, and an unmapped operator is `continue`d,
869+
so the predicate vanished from the WHERE clause. Both raw-SQL and ObjectQL
870+
strategies read that normalizer, so both were affected. `$between` now
871+
**lowers to its two bounds** (`gte` + `lte`) rather than gaining an operator
872+
of its own: each strategy's upper-bound arm already carries the whole-day
873+
calendar rule (`NativeSQLStrategy` compiles half-open directly, the ObjectQL
874+
path inherits it from the driver), so a range's max gets that rule by
875+
construction instead of via a second implementation. A malformed `$between`
876+
now throws, the anti-silent-widening stance driver-memory took for the same
877+
shape in #3948.
878+
860879
2. **A measured, irreducible limit.** `$gt` with a bare-day comparand on a
861880
`datetime` column cannot agree across backends. A typed backend anchors the
862881
bound to midnight and excludes a value stored at exactly 00:00; a type-blind
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Temporal conformance for the analytics raw-SQL strategy (ADR-0053 D-A3),
5+
* executed against a real SQLite engine (`sql.js`, pure WASM).
6+
*
7+
* The cases come from `@objectstack/spec/data` so this backend, the three
8+
* drivers, the draft preview and `formula`'s write-side `check` evaluator are
9+
* all held to one standard — see `temporal-conformance.ts` for the four
10+
* divergences that standard exists to prevent.
11+
*
12+
* ## Why this surface needed its own consumer
13+
*
14+
* `NativeSQLStrategy` is the surface #3650 broke — the dashboard window was
15+
* dropped and the chart drew all history — and it is the one listed in the
16+
* matrix's own backend table that had no consumer. It is also the only backend
17+
* that hand-compiles SQL text rather than delegating to a driver's compiler, so
18+
* "the other backends are green" says nothing about it.
19+
*
20+
* Every other suite for this strategy (`native-sql-datetime-filter.test.ts`,
21+
* `native-sql-datetime-filter-column.test.ts`) asserts the emitted SQL string.
22+
* That is a different question, with a lower ceiling: a string assertion checks
23+
* the compiler emits what its author wrote down, and a dropped predicate is
24+
* invisible to it — the SQL is still valid, just wider. This file asserts ROW
25+
* IDS, which is what D-A3 demanded and what catches a predicate that silently
26+
* went missing. (It found one: see the `$between` note in
27+
* `filter-normalizer.ts`.)
28+
*
29+
* ## Storage form
30+
*
31+
* Rows are seeded in the canonical post-#3912 form — UTC ISO text for the
32+
* `datetime` column, bare `YYYY-MM-DD` for the `date` one — which is the state
33+
* a backfilled deployment is in, and the state in which the driver's
34+
* `temporalFilterValue` / `temporalFilterColumnSql` hooks are identities. The
35+
* context therefore omits them, exercising the same "absent = identity" path a
36+
* Postgres deployment takes (ADR-0053 D-A2). The un-backfilled mixed column is
37+
* a different axis, covered where the driver truth lives:
38+
* `native-sql-datetime-filter-column.test.ts` for the emitted normalisation and
39+
* `driver-sql`'s own legacy sweep for row results.
40+
*
41+
* ## Why `sql.js` and not `better-sqlite3`
42+
*
43+
* Same reason as `read-scope-sql-conformance.test.ts`: the native binding is
44+
* loadable only by the exact Node ABI it was built for and aborts the vitest
45+
* worker on CI's Node, taking the whole file's cases silently with it. `sql.js`
46+
* is the pure-WASM engine `driver-sql` itself falls back to.
47+
*/
48+
49+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
50+
import { TEMPORAL_CASES, TEMPORAL_NOW, TEMPORAL_ROWS } from '@objectstack/spec/data';
51+
import type { Cube } from '@objectstack/spec/data';
52+
import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts';
53+
import { resolveFilterTokens } from '@objectstack/core';
54+
55+
import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js';
56+
57+
/**
58+
* Dimension ids match the fixture's property names (`at` / `on`) so the shared
59+
* cases apply unchanged, while the COLUMNS are deliberately different
60+
* (`happened_at` / `happened_on`) — that is what proves the strategy resolved
61+
* the real column rather than echoing the member name.
62+
*/
63+
const CUBE: Cube = {
64+
name: 'conformance',
65+
title: 'Conformance',
66+
sql: 'conformance',
67+
measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } },
68+
dimensions: {
69+
id: { name: 'id', label: 'Id', type: 'string', sql: 'id' },
70+
at: { name: 'at', label: 'At', type: 'time', sql: 'happened_at' },
71+
on: { name: 'on', label: 'On', type: 'time', sql: 'happened_on' },
72+
},
73+
public: false,
74+
} as unknown as Cube;
75+
76+
const resolveTokens = <T,>(filter: T): T =>
77+
resolveFilterTokens(filter, { now: new Date(TEMPORAL_NOW) });
78+
79+
/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */
80+
async function locateWasm(): Promise<((file: string) => string) | undefined> {
81+
try {
82+
const { createRequire } = await import('node:module');
83+
const require = createRequire(import.meta.url);
84+
const pkgJsonPath = require.resolve('sql.js/package.json');
85+
const { dirname, join } = await import('node:path');
86+
const dir = dirname(pkgJsonPath);
87+
return (file: string) => join(dir, 'dist', file);
88+
} catch {
89+
return undefined;
90+
}
91+
}
92+
93+
describe('NativeSQLStrategy — temporal conformance', () => {
94+
let db: any;
95+
let ctx: StrategyContext;
96+
97+
beforeAll(async () => {
98+
const mod: any = await import('sql.js');
99+
const initSqlJs = mod.default ?? mod;
100+
const locateFile = await locateWasm();
101+
const SQL = await initSqlJs(locateFile ? { locateFile } : undefined);
102+
103+
db = new SQL.Database();
104+
db.run(`
105+
CREATE TABLE "conformance" (
106+
"id" TEXT PRIMARY KEY,
107+
"happened_at" TEXT,
108+
"happened_on" TEXT,
109+
"why" TEXT
110+
);
111+
`);
112+
const insert = db.prepare(
113+
`INSERT INTO "conformance" ("id","happened_at","happened_on","why") VALUES (?,?,?,?)`,
114+
);
115+
for (const r of TEMPORAL_ROWS) insert.run([r.id, r.at, r.on, r.why]);
116+
insert.free();
117+
118+
ctx = {
119+
getCube: (name: string) => (name === 'conformance' ? CUBE : undefined),
120+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
121+
// The strategy binds `$1`-style placeholders in ascending order, each
122+
// pushed immediately before it is referenced, so a positional rewrite to
123+
// SQLite's `?` preserves the pairing.
124+
executeRawSql: async (_object: string, sql: string, params: unknown[]) => {
125+
const stmt = db.prepare(sql.replace(/\$\d+/g, '?'));
126+
stmt.bind(params as any[]);
127+
const out: Record<string, unknown>[] = [];
128+
while (stmt.step()) out.push(stmt.getAsObject());
129+
stmt.free();
130+
return out;
131+
},
132+
} as StrategyContext;
133+
});
134+
135+
afterAll(() => {
136+
db?.close();
137+
});
138+
139+
/** Group by `id` so the result rows ARE the matched row ids. */
140+
const idsFor = async (query: Omit<AnalyticsQuery, 'cube' | 'measures' | 'dimensions'>) => {
141+
const result = await new NativeSQLStrategy().execute(
142+
{ cube: 'conformance', measures: ['total'], dimensions: ['id'], ...query } as AnalyticsQuery,
143+
ctx,
144+
);
145+
return result.rows.map((r) => String(r.id)).sort();
146+
};
147+
148+
for (const c of TEMPORAL_CASES) {
149+
it(c.name, async () => {
150+
expect(await idsFor({ where: c.filter }), c.note).toEqual([...c.expected].sort());
151+
});
152+
153+
// The D-A3 token axis (#4081): the same case spelled in relative tokens,
154+
// resolved at the pinned instant, must reach the same rows.
155+
if (c.tokenFilter) {
156+
it(`${c.name} — via relative tokens`, async () => {
157+
expect(await idsFor({ where: resolveTokens(c.tokenFilter) }), c.note).toEqual(
158+
[...c.expected].sort(),
159+
);
160+
});
161+
}
162+
163+
// The dashboard-window path — the shape #3650 dropped entirely. No
164+
// granularity, or `canHandle` correctly declines to the ObjectQL strategy.
165+
if (c.dateRange) {
166+
it(`${c.name} — via timeDimensions.dateRange`, async () => {
167+
expect(
168+
await idsFor({ timeDimensions: [{ dimension: c.field, dateRange: resolveTokens(c.dateRange) }] }),
169+
c.note,
170+
).toEqual([...c.expected].sort());
171+
});
172+
}
173+
}
174+
});

packages/services/service-analytics/src/strategies/filter-normalizer.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,37 @@ function flattenCondition(cond: Record<string, unknown>, out: NormalizedAnalytic
8484
const opKeys = Object.keys(wrapper).filter(k => k.startsWith('$'));
8585
if (opKeys.length > 0) {
8686
for (const opKey of opKeys) {
87+
// `$between [min, max]` LOWERS to its two bounds rather than getting a
88+
// `between` operator of its own. Both strategies already carry the
89+
// calendar-day whole-day rule on their upper bound — NativeSQLStrategy
90+
// compiles a bare-day `lte` half-open (#3777), ObjectQLStrategy hands
91+
// `$lte` to the driver, which does the same — so a range's max
92+
// inherits that rule by construction instead of needing a second
93+
// implementation to keep in step. (The preview evaluator's `$between`
94+
// gap was closed the same way, sharing its `$lte` helper.)
95+
//
96+
// Before this, `$between` was simply absent from the operator map and
97+
// fell to the `continue` below: the predicate VANISHED from the WHERE
98+
// clause, so a dashboard widget carrying a range filter charted the
99+
// entire dataset — #3650's symptom, on the surface #3650 was about.
100+
// The temporal conformance matrix caught it as row results
101+
// (`native-sql-temporal-conformance.test.ts`).
102+
if (opKey === '$between') {
103+
const v = wrapper[opKey];
104+
if (!Array.isArray(v) || v.length !== 2) {
105+
// Never drop it: an unbounded read is the failure mode this whole
106+
// branch exists to prevent, and it is indistinguishable from a
107+
// legitimately wide query. Same stance driver-memory took for the
108+
// same shape (#3948).
109+
throw new Error(
110+
`[analytics] "$between" on "${key}" needs a two-element [min, max] array, got ` +
111+
`${JSON.stringify(v)}. Dropping the predicate would silently widen the query to every row.`,
112+
);
113+
}
114+
out.push({ member: key, operator: 'gte', values: [stringifyForCube(v[0])] });
115+
out.push({ member: key, operator: 'lte', values: [stringifyForCube(v[1])] });
116+
continue;
117+
}
87118
const cubeOp = MONGO_TO_CUBE_OP[opKey];
88119
if (!cubeOp) continue;
89120
const v = wrapper[opKey];

0 commit comments

Comments
 (0)