Skip to content

Commit bdfa152

Browse files
committed
fix(service-analytics): every authorable filter operator reaches the query (#4128)
Closes the CAUSE behind the $between defect, not just that instance. `normalizeAnalyticsFilters` skipped any operator missing from its map, and a skipped predicate does not narrow a query — it widens it: the SQL stays valid and returns rows the author excluded. Four operators from the spec's authorable vocabulary sat in that state, and a fifth was mapped wrongly: - $startsWith / $endsWith were dropped. Both strategies now compile them — anchored LIKE on the raw-SQL path, the canonical operators (which every driver implements directly) on the ObjectQL path, so an anchored match does not depend on regex dialect. - $null was dropped. It is what the console emits for an "is empty" filter, so such a widget showed every row. - $exists was mapped value-INDEPENDENTLY to `set`, so {$exists: false} compiled to IS NOT NULL — the inverse of what it asks. It and $null are now resolved explicitly: a key→name map cannot express an operator whose meaning flips with its value, which is exactly how that inversion got in. - $notContains reached ObjectQLStrategy, which had no arm for it and fell to a `default` returning a bare value — compiling "does not contain x" as "equals x". - Unknown operators now THROW on both surfaces rather than being dropped (normalizer) or reinterpreted as an equality (ObjectQL). #3948's call for the same shape. $or / $not remain skipped — expressing them needs a recursive WHERE builder rather than the flat array the strategies consume. That gap is declared in the module doc rather than silent. filter-operator-coverage.test.ts runs the whole vocabulary against a real SQLite engine and asserts ROW IDS; six of its cases fail without this change. A dropped predicate is invisible to the SQL-string assertions the strategies' other suites use, which is how these survived. service-analytics 413 green. Closes #4128. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqqZmPS5a4gJGBoCTwipFr
1 parent d30f4e9 commit bdfa152

5 files changed

Lines changed: 326 additions & 19 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): every authorable filter operator now reaches the query (#4128)
6+
7+
Closes the cause behind the `$between` defect rather than just that instance.
8+
`normalizeAnalyticsFilters` skipped any operator missing from its map, and a
9+
skipped predicate does not narrow a query — it **widens** it: the compiled SQL
10+
stays valid and returns rows the author excluded. Four operators from the
11+
spec's authorable vocabulary sat in that state, plus one that was mapped
12+
incorrectly.
13+
14+
- **`$startsWith` / `$endsWith`** were dropped entirely. Both strategies now
15+
compile them — anchored `LIKE 'x%'` / `LIKE '%x'` on the raw-SQL path, and
16+
the canonical `$startsWith` / `$endsWith` operators (which every driver
17+
implements directly) on the ObjectQL path, so an anchored match does not
18+
depend on regex dialect.
19+
- **`$null`** was dropped. It is the shape the console emits for an "is empty"
20+
/ "is not empty" filter, so such a widget was showing every row. Now compiles
21+
to `IS NULL` / `IS NOT NULL` per its boolean.
22+
- **`$exists`** was mapped value-*independently* to `set`, so `{$exists: false}`
23+
compiled to `IS NOT NULL` — the exact inverse of what it asks for. It and
24+
`$null` are now resolved explicitly, because a key→name map cannot express an
25+
operator whose meaning flips with its value.
26+
- **`$notContains`** reached the ObjectQL strategy, which had no arm for it and
27+
fell through to a `default` returning a bare value — compiling "does not
28+
contain x" as "**equals** x".
29+
- **Unknown operators now throw** on both surfaces instead of being silently
30+
dropped (normalizer) or reinterpreted as an equality (ObjectQL strategy). An
31+
operator outside the vocabulary is a caller error, and a loud one beats a
32+
silently widened read — the call driver-memory made for the same shape in
33+
#3948.
34+
35+
Still declared as a gap, but no longer a silent one: `$or` / `$not` are skipped,
36+
since expressing them needs a recursive WHERE builder rather than the flat
37+
array the strategies consume.
38+
39+
Cover is `filter-operator-coverage.test.ts`, which runs the whole vocabulary
40+
against a real SQLite engine and asserts **row ids** — six of its cases fail
41+
without this change. A dropped predicate is invisible to the SQL-string
42+
assertions the strategies' other suites use, which is how these survived.
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Every authorable filter operator reaches the query — the closure of #4128.
5+
*
6+
* `normalizeAnalyticsFilters` maps the spec's `FilterCondition` vocabulary onto
7+
* the internal pipeline form. Whatever it fails to map used to be `continue`d,
8+
* and that is not "unsupported": the predicate DISAPPEARS, the compiled SQL
9+
* stays valid, and the query returns rows the author excluded. It reads as a
10+
* chart drawn over the whole dataset (#3650's symptom) and is invisible to a
11+
* test that asserts the emitted SQL string — which is what every other suite
12+
* for these strategies does, and why four operators sat broken behind one that
13+
* had already been found (`$between`, ADR-0053 D-A3.1).
14+
*
15+
* So this file asserts ROW IDS, against a real SQLite (`sql.js`, the pure-WASM
16+
* engine `driver-sql` itself falls back to — see the note in
17+
* `read-scope-sql-conformance.test.ts` for why not `better-sqlite3`). A dropped
18+
* predicate cannot hide from it: the row set is simply wrong.
19+
*
20+
* The table below is the whole authorable vocabulary of `filter.zod.ts`, so a
21+
* new operator added to the spec without a home in the pipeline fails here
22+
* rather than silently widening a customer's dashboard.
23+
*/
24+
25+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
26+
import type { Cube } from '@objectstack/spec/data';
27+
import type { AnalyticsQuery, FilterCondition } from '@objectstack/spec/data';
28+
import type { StrategyContext } from '@objectstack/spec/contracts';
29+
30+
import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js';
31+
import { normalizeAnalyticsFilters } from '../strategies/filter-normalizer.js';
32+
33+
interface Row {
34+
id: string;
35+
name: string | null;
36+
score: number;
37+
}
38+
39+
/** `n_null` carries a NULL name — the row the null predicates turn on. */
40+
const ROWS: Row[] = [
41+
{ id: 'a_alpha', name: 'alpha-one', score: 10 },
42+
{ id: 'b_alphex', name: 'alphex-two', score: 20 },
43+
{ id: 'c_beta', name: 'beta-one', score: 30 },
44+
{ id: 'd_null', name: null, score: 40 },
45+
];
46+
47+
const CUBE: Cube = {
48+
name: 'ops',
49+
title: 'Ops',
50+
sql: 'ops',
51+
measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } },
52+
dimensions: {
53+
id: { name: 'id', label: 'Id', type: 'string', sql: 'id' },
54+
name: { name: 'name', label: 'Name', type: 'string', sql: 'name' },
55+
score: { name: 'score', label: 'Score', type: 'number', sql: 'score' },
56+
},
57+
public: false,
58+
} as unknown as Cube;
59+
60+
/**
61+
* One case per operator in `filter.zod.ts`'s authorable vocabulary.
62+
* `expected` is the row ids the filter MUST return — the full row set means
63+
* the predicate went missing.
64+
*/
65+
const CASES: Array<{ op: string; filter: FilterCondition; expected: string[]; note?: string }> = [
66+
{ op: '$eq', filter: { name: { $eq: 'alpha-one' } }, expected: ['a_alpha'] },
67+
{ op: '$ne', filter: { name: { $ne: 'alpha-one' } }, expected: ['b_alphex', 'c_beta'] },
68+
{ op: '$gt', filter: { score: { $gt: 20 } }, expected: ['c_beta', 'd_null'] },
69+
{ op: '$gte', filter: { score: { $gte: 20 } }, expected: ['b_alphex', 'c_beta', 'd_null'] },
70+
{ op: '$lt', filter: { score: { $lt: 20 } }, expected: ['a_alpha'] },
71+
{ op: '$lte', filter: { score: { $lte: 20 } }, expected: ['a_alpha', 'b_alphex'] },
72+
{ op: '$in', filter: { name: { $in: ['alpha-one', 'beta-one'] } }, expected: ['a_alpha', 'c_beta'] },
73+
{ op: '$nin', filter: { name: { $nin: ['alpha-one'] } }, expected: ['b_alphex', 'c_beta'] },
74+
{ op: '$between', filter: { score: { $between: [20, 30] } }, expected: ['b_alphex', 'c_beta'], note: '#4128: was dropped → every row.' },
75+
{ op: '$contains', filter: { name: { $contains: 'one' } }, expected: ['a_alpha', 'c_beta'] },
76+
{
77+
op: '$notContains',
78+
filter: { name: { $notContains: 'one' } },
79+
expected: ['b_alphex'],
80+
note: 'On the ObjectQL path this had no arm and fell to the default, compiling "does not contain" as an EQUALITY.',
81+
},
82+
{
83+
op: '$startsWith',
84+
filter: { name: { $startsWith: 'alpha' } },
85+
expected: ['a_alpha'],
86+
note: '#4128: was dropped → every row. b_alphex proves the anchor is a prefix, not a substring.',
87+
},
88+
{
89+
op: '$endsWith',
90+
filter: { name: { $endsWith: '-one' } },
91+
expected: ['a_alpha', 'c_beta'],
92+
note: '#4128: was dropped → every row.',
93+
},
94+
{
95+
op: '$null: true',
96+
filter: { name: { $null: true } },
97+
expected: ['d_null'],
98+
note: '#4128: was dropped → every row. This is the shape the console emits for an "is empty" filter.',
99+
},
100+
{ op: '$null: false', filter: { name: { $null: false } }, expected: ['a_alpha', 'b_alphex', 'c_beta'] },
101+
{ op: '$exists: true', filter: { name: { $exists: true } }, expected: ['a_alpha', 'b_alphex', 'c_beta'] },
102+
{
103+
op: '$exists: false',
104+
filter: { name: { $exists: false } },
105+
expected: ['d_null'],
106+
note: 'Was mapped value-INDEPENDENTLY to `set`, so it compiled to IS NOT NULL — the exact inverse of what it asks.',
107+
},
108+
{ op: 'implicit equality', filter: { name: 'beta-one' }, expected: ['c_beta'] },
109+
{ op: 'bare null', filter: { name: null }, expected: ['d_null'] },
110+
{
111+
op: '$and',
112+
filter: { $and: [{ score: { $gte: 20 } }, { name: { $contains: 'one' } }] },
113+
expected: ['c_beta'],
114+
},
115+
];
116+
117+
/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */
118+
async function locateWasm(): Promise<((file: string) => string) | undefined> {
119+
try {
120+
const { createRequire } = await import('node:module');
121+
const require = createRequire(import.meta.url);
122+
const pkgJsonPath = require.resolve('sql.js/package.json');
123+
const { dirname, join } = await import('node:path');
124+
const dir = dirname(pkgJsonPath);
125+
return (file: string) => join(dir, 'dist', file);
126+
} catch {
127+
return undefined;
128+
}
129+
}
130+
131+
describe('analytics filters — every authorable operator reaches the query (#4128)', () => {
132+
let db: any;
133+
let ctx: StrategyContext;
134+
135+
beforeAll(async () => {
136+
const mod: any = await import('sql.js');
137+
const initSqlJs = mod.default ?? mod;
138+
const locateFile = await locateWasm();
139+
const SQL = await initSqlJs(locateFile ? { locateFile } : undefined);
140+
141+
db = new SQL.Database();
142+
db.run(`CREATE TABLE "ops" ("id" TEXT PRIMARY KEY, "name" TEXT, "score" INTEGER);`);
143+
const insert = db.prepare(`INSERT INTO "ops" ("id","name","score") VALUES (?,?,?)`);
144+
for (const r of ROWS) insert.run([r.id, r.name, r.score]);
145+
insert.free();
146+
147+
ctx = {
148+
getCube: (name: string) => (name === 'ops' ? CUBE : undefined),
149+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
150+
executeRawSql: async (_object: string, sql: string, params: unknown[]) => {
151+
const stmt = db.prepare(sql.replace(/\$\d+/g, '?'));
152+
stmt.bind(params as any[]);
153+
const out: Record<string, unknown>[] = [];
154+
while (stmt.step()) out.push(stmt.getAsObject());
155+
stmt.free();
156+
return out;
157+
},
158+
} as StrategyContext;
159+
});
160+
161+
afterAll(() => {
162+
db?.close();
163+
});
164+
165+
for (const c of CASES) {
166+
it(`${c.op} narrows to the rows it names`, async () => {
167+
const result = await new NativeSQLStrategy().execute(
168+
{ cube: 'ops', measures: ['total'], dimensions: ['id'], where: c.filter } as AnalyticsQuery,
169+
ctx,
170+
);
171+
const got = result.rows.map((r) => String(r.id)).sort();
172+
expect(got, c.note).toEqual(c.expected);
173+
// Belt and braces: a predicate that silently vanished returns the whole
174+
// fixture, which is the one wrong answer that looks like a working query.
175+
expect(got.length, `${c.op} matched every row — the predicate was dropped`).toBeLessThan(ROWS.length);
176+
});
177+
}
178+
179+
it('an operator outside the vocabulary throws instead of widening the query', () => {
180+
// The failure mode this whole file exists to prevent: silently returning
181+
// rows the filter excludes. A typo'd or non-spec operator is a caller
182+
// error, and a loud one — the same call driver-memory made in #3948.
183+
expect(() =>
184+
normalizeAnalyticsFilters({ where: { name: { $sortOf: 'alpha' } } }),
185+
).toThrow(/Unsupported filter operator "\$sortOf"/);
186+
});
187+
188+
it('a malformed $between throws rather than binding a half-open guess', () => {
189+
expect(() => normalizeAnalyticsFilters({ where: { score: { $between: [10] } } })).toThrow(
190+
/two-element/,
191+
);
192+
});
193+
});

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

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,31 @@
1717
* spec is honoured: dashboard metadata is authored once in the
1818
* canonical MongoDB form and the server normalizes at the boundary.
1919
*
20-
* # Coverage, stated honestly — an unmapped operator WIDENS the query
20+
* # Coverage — a dropped predicate WIDENS the query, so nothing is dropped
2121
*
22-
* Dropping a predicate is not "not supporting" it: the compiled SQL stays
23-
* valid and simply returns more rows, which reads as a chart drawn over the
24-
* whole dataset (#3650's symptom) and is invisible to any test that asserts
25-
* the emitted SQL string. So what this maps is a capability claim:
22+
* Failing to map an operator is not "not supporting" it: the predicate simply
23+
* disappears, the compiled SQL stays valid, and the query returns rows the
24+
* author excluded. It reads as a chart drawn over the whole dataset (#3650's
25+
* symptom) and is invisible to any test that asserts the emitted SQL string.
26+
* `$between`, `$startsWith`, `$endsWith` and `$null` each sat broken that way
27+
* (#4128), so what this maps is now a complete capability claim over
28+
* `filter.zod.ts`'s authorable vocabulary:
2629
*
2730
* - mapped 1:1 — `$eq` `$ne` `$gt` `$gte` `$lt` `$lte` `$in` `$nin`
28-
* `$contains` `$notContains` `$exists`, plus `null` → `notSet`;
29-
* - lowered — `$and` (flattened in place), and `$between`, which becomes
30-
* its two bounds so each strategy's existing upper-bound handling
31-
* applies the calendar-day whole-day rule (see the note at the lowering);
32-
* - NOT covered, and silently dropped today — `$startsWith` `$endsWith`
33-
* `$null` `$regex`, and the `$or` / `$not` combinators (the latter two
34-
* deliberately, pending recursive WHERE building). Tracked in #4128,
35-
* which also carries the case for turning the fallback into a throw the
36-
* way driver-memory did in #3948.
31+
* `$contains` `$notContains` `$startsWith` `$endsWith`;
32+
* - value-DEPENDENT, so resolved explicitly rather than through the map —
33+
* `$null` and `$exists`, whose meaning flips with their boolean;
34+
* - lowered — `$and` (flattened in place), and `$between`, which becomes its
35+
* two bounds so each strategy's existing upper-bound handling applies the
36+
* calendar-day whole-day rule (see the note at the lowering);
37+
* - anything else THROWS. An operator outside the vocabulary is a caller
38+
* error, and a loud one beats a silently widened read — the call
39+
* driver-memory made for the same shape in #3948.
40+
*
41+
* The one remaining gap is declared, not silent: the `$or` / `$not`
42+
* combinators are still skipped, because expressing them needs a recursive
43+
* WHERE builder rather than this flat array. Row-result cover for everything
44+
* above lives in `filter-operator-coverage.test.ts`.
3745
*/
3846

3947
export interface NormalizedAnalyticsFilter {
@@ -42,6 +50,14 @@ export interface NormalizedAnalyticsFilter {
4250
values: string[];
4351
}
4452

53+
/**
54+
* The value-INDEPENDENT operators: the pipeline name depends only on the key.
55+
*
56+
* `$null` and `$exists` are deliberately absent — their meaning flips with
57+
* their boolean value, which a key→name map cannot express. Putting `$exists`
58+
* here anyway is what made `{$exists: false}` compile to `IS NOT NULL`, the
59+
* exact inverse of what it asks for; both are handled explicitly below.
60+
*/
4561
const MONGO_TO_CUBE_OP: Record<string, string> = {
4662
$eq: 'equals',
4763
$ne: 'notEquals',
@@ -53,7 +69,8 @@ const MONGO_TO_CUBE_OP: Record<string, string> = {
5369
$nin: 'notIn',
5470
$contains: 'contains',
5571
$notContains: 'notContains',
56-
$exists: 'set',
72+
$startsWith: 'startsWith',
73+
$endsWith: 'endsWith',
5774
};
5875

5976
/**
@@ -133,8 +150,36 @@ function flattenCondition(cond: Record<string, unknown>, out: NormalizedAnalytic
133150
out.push({ member: key, operator: 'lte', values: [stringifyForCube(v[1])] });
134151
continue;
135152
}
153+
154+
// The two null predicates read their BOOLEAN, not just their key —
155+
// which is why neither can live in MONGO_TO_CUBE_OP. `$null: true`
156+
// asks for IS NULL (`notSet`), `$null: false` for IS NOT NULL
157+
// (`set`); `$exists` is the mirror image. `$null` is the shape the
158+
// console emits for an "is empty" / "is not empty" filter
159+
// (`is_null`/`is_not_null` normalise to it in `filter.zod.ts`), so
160+
// dropping it silently meant such a widget showed every row.
161+
if (opKey === '$null' || opKey === '$exists') {
162+
const isNull = opKey === '$null' ? wrapper[opKey] === true : wrapper[opKey] === false;
163+
out.push({ member: key, operator: isNull ? 'notSet' : 'set', values: [] });
164+
continue;
165+
}
166+
136167
const cubeOp = MONGO_TO_CUBE_OP[opKey];
137-
if (!cubeOp) continue;
168+
if (!cubeOp) {
169+
// NEVER drop: a missing predicate does not narrow the query, it
170+
// WIDENS it — the compiled SQL stays valid and simply returns rows
171+
// the author excluded, which is indistinguishable from a
172+
// legitimately broad query and invisible to any test that asserts
173+
// the emitted SQL. That failure mode is #3650's, and skipping
174+
// unmapped operators is how `$between` reproduced it (#4128).
175+
// driver-memory made the same call for the same reason in #3948.
176+
throw new Error(
177+
`[analytics] Unsupported filter operator "${opKey}" on "${key}". ` +
178+
`Supported: ${Object.keys(MONGO_TO_CUBE_OP).join(', ')}, $between, $null, $exists ` +
179+
`(and $and; $or/$not are not yet compiled by the analytics strategies). ` +
180+
`Dropping it would silently widen the query to rows the filter excludes.`,
181+
);
182+
}
138183
const v = wrapper[opKey];
139184
const values = Array.isArray(v)
140185
? v.map(stringifyForCube)

‎packages/services/service-analytics/src/strategies/native-sql-strategy.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,14 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
484484
const opMap: Record<string, string> = {
485485
equals: '=', notEquals: '!=', gt: '>', gte: '>=', lt: '<', lte: '<=',
486486
contains: 'LIKE', notContains: 'NOT LIKE',
487+
startsWith: 'LIKE', endsWith: 'LIKE',
488+
};
489+
/** The LIKE pattern each string operator wraps its comparand in. */
490+
const likePattern: Record<string, (v: string) => string> = {
491+
contains: (v) => `%${v}%`,
492+
notContains: (v) => `%${v}%`,
493+
startsWith: (v) => `${v}%`,
494+
endsWith: (v) => `%${v}`,
487495
};
488496

489497
// Null predicates and the LIKE family read the column as stored — the former
@@ -504,8 +512,11 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
504512
const sqlOp = opMap[operator];
505513
if (!sqlOp || !values || values.length === 0) return null;
506514

507-
if (operator === 'contains' || operator === 'notContains') {
508-
params.push(`%${values[0]}%`);
515+
// The LIKE family reads the column as stored — a substring/prefix/suffix
516+
// match is on the raw text — so it keeps the un-normalised reference.
517+
const pattern = likePattern[operator];
518+
if (pattern) {
519+
params.push(pattern(values[0]));
509520
return `${rawCol} ${sqlOp} $${params.length}`;
510521
}
511522

0 commit comments

Comments
 (0)