Skip to content

Commit f5ab1c7

Browse files
authored
fix(service-analytics): $or / $not filters reach the query (#4128 follow-up) (#4175)
The last of the silently-dropped filter family, and the one dropped for a structural reason rather than an oversight: `normalizeAnalyticsFilters` produced a flat ARRAY, which cannot carry a disjunction. Both strategies therefore skipped `$or` and `$not`, so a widget or dataset filtering with either compiled a WHERE clause that simply did not contain it and drew every row — #3650's symptom, and unlike a rejected query it looks like a working chart. The normalizer now produces a TREE (`normalizeAnalyticsFilterTree`), the single owner of the vocabulary, and each strategy compiles it the way its own backend expresses a disjunction: - NativeSQLStrategy builds the WHERE recursively, routing every leaf through its existing clause emitter — so the storage-form coercion and the calendar-day upper-bound rule (#3777) apply at every depth, including inside an $or, where a second combinator-aware emitter would have been free to drift from the first. Parentheses are explicit rather than resting on SQL precedence: `a AND b OR c` happens to be right, and being right by construction is what stops a later edit making it wrong. - ObjectQLStrategy hands $or/$not to the engine, which speaks them natively. AND-ed leaves still merge per field exactly as before, so a query without combinators produces byte-identical engine input. - /analytics/sql renders the same tree. That block's own comment warns against echoing something other than what executes; a dropped disjunction was that lie in the other direction. - The cross-object envelope check now sees members nested in an $or. It rejects cross-object filters, so a member it could not see was a filter it could not reject. Empty $and/$or arrays throw rather than being ignored, and the tree walker's combinator handling deliberately mirrors `read-scope-sql.ts` — the compiler in this same package that has always handled the full tree. That the package held one correct implementation and one lossy one for the same filter shape, with nothing holding them to each other, is the actual defect behind this one. `native-sql-filter-logic-conformance.test.ts` runs the SHARED combinator table (FILTER_LOGIC_CASES, #3774) against a real SQLite engine and asserts row ids, so the analytics raw-SQL path now stands beside driver-sql, driver-memory, formula and read-scope-sql under one standard. 14 of its 17 cases fail without this change. Refs #4128, #3774, #3650.
1 parent d13004a commit f5ab1c7

6 files changed

Lines changed: 573 additions & 146 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): a `$or` / `$not` filter no longer vanishes from an analytics query (#4128 follow-up)
6+
7+
The last of the silently-dropped filter family. `normalizeAnalyticsFilters`
8+
produced a flat **array**, which cannot carry a disjunction, so both strategies
9+
skipped `$or` and `$not` outright — a widget or dataset whose filter used
10+
either compiled a WHERE clause that simply did not contain it, and drew every
11+
row. That is #3650's symptom, and unlike a rejected query it looks like a
12+
working chart.
13+
14+
The normalizer now produces a **tree** (`normalizeAnalyticsFilterTree`), and
15+
each strategy compiles it the way its own backend expresses a disjunction:
16+
17+
- **`NativeSQLStrategy`** builds the WHERE recursively, routing every leaf
18+
through its existing clause emitter — so the storage-form coercion and the
19+
calendar-day upper-bound rule (#3777) apply at every depth, including inside
20+
an `$or`. Parentheses are explicit rather than relying on SQL precedence.
21+
- **`ObjectQLStrategy`** hands `$or` / `$not` to the engine, which speaks them
22+
natively. AND-ed leaves still merge per field exactly as before, so a query
23+
without combinators produces byte-identical engine input.
24+
- **`/analytics/sql`** renders the same tree, so the echoed statement keeps
25+
reproducing what executes rather than showing a conjunction where the engine
26+
runs a disjunction.
27+
- The **cross-object envelope check** now sees members nested inside an `$or`.
28+
It rejects cross-object filters, so a member it could not see was a filter it
29+
could not reject.
30+
31+
Empty `$and` / `$or` arrays now throw instead of being ignored, matching the
32+
fail-closed stance of `read-scope-sql.ts` — the compiler in this same package
33+
that has always handled the full tree, and whose semantics the tree walker now
34+
mirrors deliberately.
35+
36+
Cover is `native-sql-filter-logic-conformance.test.ts`, which runs the shared
37+
combinator table (`FILTER_LOGIC_CASES`, #3774) against a real SQLite engine and
38+
asserts row ids. The analytics raw-SQL path now stands beside `driver-sql`,
39+
`driver-memory`, `formula` and `read-scope-sql` under that one standard; 14 of
40+
its 17 cases fail without this change.

packages/services/service-analytics/src/__tests__/filter-operator-coverage.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import type { AnalyticsQuery, FilterCondition } from '@objectstack/spec/data';
2828
import type { StrategyContext } from '@objectstack/spec/contracts';
2929

3030
import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js';
31-
import { normalizeAnalyticsFilters } from '../strategies/filter-normalizer.js';
31+
import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js';
3232

3333
interface Row {
3434
id: string;
@@ -181,12 +181,12 @@ describe('analytics filters — every authorable operator reaches the query (#41
181181
// rows the filter excludes. A typo'd or non-spec operator is a caller
182182
// error, and a loud one — the same call driver-memory made in #3948.
183183
expect(() =>
184-
normalizeAnalyticsFilters({ where: { name: { $sortOf: 'alpha' } } }),
184+
normalizeAnalyticsFilterTree({ where: { name: { $sortOf: 'alpha' } } }),
185185
).toThrow(/Unsupported filter operator "\$sortOf"/);
186186
});
187187

188188
it('a malformed $between throws rather than binding a half-open guess', () => {
189-
expect(() => normalizeAnalyticsFilters({ where: { score: { $between: [10] } } })).toThrow(
189+
expect(() => normalizeAnalyticsFilterTree({ where: { score: { $between: [10] } } })).toThrow(
190190
/two-element/,
191191
);
192192
});
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Filter logical-combinator conformance for the analytics raw-SQL strategy,
5+
* executed against a real SQLite engine (`sql.js`, pure WASM).
6+
*
7+
* The cases come from `@objectstack/spec/data`, so this backend now stands
8+
* beside `driver-sql`, `driver-memory`, `formula`'s `matchesFilterCondition`
9+
* and `read-scope-sql` under one standard — see `filter-logic-conformance.ts`
10+
* for why that standard exists (#3774).
11+
*
12+
* ## Why this consumer arrives late, and what it proves
13+
*
14+
* The analytics strategies could not have passed this table before: their
15+
* normalizer produced a flat ARRAY, which cannot carry a disjunction, so an
16+
* author's `{$or: […]}` was skipped outright and the compiled WHERE simply
17+
* did not contain it. That is not "unsupported" — a missing predicate WIDENS
18+
* the query, returning rows the author excluded, and it is invisible to a
19+
* test that asserts the emitted SQL string (the SQL stays valid, just
20+
* broader). Every case below whose filter carries a combinator fails against
21+
* the pre-tree normalizer, most of them by returning the entire fixture.
22+
*
23+
* The read-scope compiler in this same package (`read-scope-sql.ts`) has
24+
* always compiled the full tree, and is already a consumer of this table —
25+
* so the package contained one correct implementation and one lossy one, for
26+
* the same filter shape, with nothing holding them to each other. It does
27+
* now.
28+
*
29+
* ## Why `sql.js` and not `better-sqlite3`
30+
*
31+
* Same reason as `read-scope-sql-conformance.test.ts`: the native binding is
32+
* loadable only by the exact Node ABI it was built for and aborts the vitest
33+
* worker on CI's Node, taking the file's cases silently with it. `sql.js` is
34+
* the pure-WASM engine `driver-sql` itself falls back to.
35+
*/
36+
37+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
38+
import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data';
39+
import type { Cube } from '@objectstack/spec/data';
40+
import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts';
41+
42+
import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js';
43+
44+
/**
45+
* Dimension ids match the fixture's column names so the shared cases apply
46+
* unchanged. `id` is selected and grouped by, which makes the result rows the
47+
* matched row ids.
48+
*/
49+
const CUBE: Cube = {
50+
name: 'logic',
51+
title: 'Logic',
52+
sql: 't',
53+
measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } },
54+
dimensions: Object.fromEntries(
55+
['id', 'a', 'b', 'c', 'owner', 'status', 'parent_object', 'parent_id'].map((n) => [
56+
n,
57+
{ name: n, label: n, type: 'string', sql: n },
58+
]),
59+
),
60+
public: false,
61+
} as unknown as Cube;
62+
63+
/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */
64+
async function locateWasm(): Promise<((file: string) => string) | undefined> {
65+
try {
66+
const { createRequire } = await import('node:module');
67+
const require = createRequire(import.meta.url);
68+
const pkgJsonPath = require.resolve('sql.js/package.json');
69+
const { dirname, join } = await import('node:path');
70+
const dir = dirname(pkgJsonPath);
71+
return (file: string) => join(dir, 'dist', file);
72+
} catch {
73+
return undefined;
74+
}
75+
}
76+
77+
describe('NativeSQLStrategy — filter logic conformance', () => {
78+
let db: any;
79+
let ctx: StrategyContext;
80+
81+
beforeAll(async () => {
82+
const mod: any = await import('sql.js');
83+
const initSqlJs = mod.default ?? mod;
84+
const locateFile = await locateWasm();
85+
const SQL = await initSqlJs(locateFile ? { locateFile } : undefined);
86+
87+
db = new SQL.Database();
88+
db.run(`
89+
CREATE TABLE "t" (
90+
"id" TEXT PRIMARY KEY,
91+
"a" TEXT, "b" TEXT, "c" TEXT,
92+
"owner" TEXT, "status" TEXT,
93+
"parent_object" TEXT, "parent_id" TEXT
94+
);
95+
`);
96+
const insert = db.prepare(
97+
`INSERT INTO "t" ("id","a","b","c","owner","status","parent_object","parent_id")
98+
VALUES (?,?,?,?,?,?,?,?)`,
99+
);
100+
for (const r of FILTER_LOGIC_ROWS) {
101+
insert.run([r.id, r.a, r.b, r.c, r.owner, r.status, r.parent_object, r.parent_id]);
102+
}
103+
insert.free();
104+
105+
ctx = {
106+
getCube: (name: string) => (name === 'logic' ? CUBE : undefined),
107+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
108+
// The strategy binds `$1`-style placeholders in ascending order, each
109+
// pushed immediately before it is referenced, so a positional rewrite to
110+
// SQLite's `?` preserves the pairing.
111+
executeRawSql: async (_object: string, sql: string, params: unknown[]) => {
112+
const stmt = db.prepare(sql.replace(/\$\d+/g, '?'));
113+
stmt.bind(params as any[]);
114+
const out: Record<string, unknown>[] = [];
115+
while (stmt.step()) out.push(stmt.getAsObject());
116+
stmt.free();
117+
return out;
118+
},
119+
} as StrategyContext;
120+
});
121+
122+
afterAll(() => {
123+
db?.close();
124+
});
125+
126+
for (const c of FILTER_LOGIC_CASES) {
127+
it(c.name, async () => {
128+
const result = await new NativeSQLStrategy().execute(
129+
{
130+
cube: 'logic',
131+
measures: ['total'],
132+
dimensions: ['id'],
133+
where: c.filter,
134+
} as AnalyticsQuery,
135+
ctx,
136+
);
137+
const got = result.rows.map((r) => String(r.id)).sort((x, y) => x.localeCompare(y));
138+
expect(got, c.note).toEqual(c.expected);
139+
});
140+
}
141+
});

0 commit comments

Comments
 (0)