Skip to content

Commit 226e00c

Browse files
fix(service-analytics)!: the analytics where door refuses a non-boolean $null / $exists flag, which it read as IS NOT NULL (#20040) (#20115)
Fixes #20040 Clause-②: no (narrowing) ## What this changes The analytics `where` door (`lowerAnalyticsWhere` in `packages/services/service-analytics/src/strategies/filter-normalizer.ts`) now refuses a `$null` or `$exists` flag whose value is not a boolean. It refuses before any lowering, in the door's `INVALID_FILTER` / 400 envelope, with the message kept. - `FieldOperatorsSchema` declares both flags `z.boolean()`. The #5347 / #5369 rulings refuse a non-boolean flag in every position and on every backend, because the backends read one in opposite directions. `driver-sql` refuses it (`nonBooleanNullComparandError` / `nonBooleanExistsComparandError`), and so does the read-scope compiler (#6387, `assertBooleanFlagComparands`). - This door read the flag by identity (`=== true` for `$null`, `=== false` for `$exists`), so every other value lowered to `set`, IS NOT NULL. How: - `normalizeWhereComparands` gains a third pass, `assertBooleanNullFlags`, after the shared comparand-shape face and the shared comparand-type face. - It walks their traversal (`forEachWhereFieldEntry`), so it finds a flag in every position they judge: `$and` / `$or` members, `$not`, field entries, and a nested relation's entries. - It reports a flag at the path those faces give it (`where.acct.stage.$null`). - The message names the operator, the field and the path: `[analytics] Operator "$null" on field "stage" requires a boolean comparand (true or false). Received a string ("x") at where.stage.$null. …` - Its main clause is `driver-sql`'s, word for word through "(true or false)". - The rest says what this door used to do and how to repair it. It carries no tracker number. - **Reuse.** `@objectstack/spec`, `core` and `types` publish no predicate or sentence for this refusal (searched; none exists). - So the one check lives in this file, and its docblock cites #5347 / #5369 and the sibling faces. - It imports no driver and adds no spec export. - The received value is rendered with this package's own `shapePreview` (`comparand-shape.ts`). - `true` / `false` lower exactly as before. - The `FilterArray` spelling needs no pass (measured, below): - `parseFilterAST` writes a hard-coded boolean for `is_null` / `is_not_null`, and refuses `$null` / `exists` as array operators; - `isFilterAST` refuses an embedded object. ### The order chosen, and why - **After the shape and type faces.** A flag the type face refuses keeps the face's sentence: `undefined`, a plain object, a `Map`, a bigint beyond 2^53. That is the sentence the `FilterArray` spelling and the engine seam give. A shape or type defect elsewhere in the same `where` is still answered first. Ablation C pins this order. - **Before the `undefined` gate (#6386) and everything else `buildNode` reaches.** Two readers see the flag before the identity read in `fieldLeaves`: - the #5146 `$not` rewrite, which classifies every field spec through `nullValueSatisfiesOperator` / `operatorIsNullTotal` (the flag readers at the old `filter-normalizer.ts:1411` / `:1464`); - the draft preview, which evaluates what `normalizeWhereComparands` returns and never reaches `fieldLeaves`. The `undefined` gate skips both flags by name, so the two gates never judge one value. - **One precedence change, same envelope.** A `where` carrying a non-boolean flag and a defect only the lowering diagnoses is now answered with the flag. Those defects are a mixed `$` / non-`$` wrapper (#6444), an operator outside the vocabulary, and a zero-operator constraint. The code and status stay `INVALID_FILTER` / 400. #20035 took the same precedence for the type face over #6444. It is pinned, with boolean controls that keep the old sentences. - **The preview.** Before the fix it refused every flag ("not evaluated", 400) while the published faces served the cell, so the two answered it differently. It reaches the new pass through `normalizeWhereComparands` and now refuses this cell in the door's words, byte for byte. `preview-evaluator.ts` is not edited. A boolean flag still gets the preview's own "not evaluated" refusal. ## Measured first (recorded on this branch as `12e3081867`, test-only, before any source change) Base `origin/main` `8d76c2d38c`. - **Data.** A real sql.js engine (`driver-sqlite-wasm`) over rows r1 'won', r2 NULL and r3 'lost' (field `stage`), with a real `ObjectQL` behind the ObjectQL face. The echo's SQL was run on the same database. - **HTTP legs.** Read and measured only, through throwaway harnesses (not committed) over a real `AnalyticsService`: - `packages/runtime`'s dispatcher route `POST /api/v1/analytics/query`, native and ObjectQL-only; - `packages/rest`'s `POST /analytics/dataset/query`, with the flag in `selection.runtimeFilter` and in an inline `dataset.filter`. | `{ stage: { FLAG: V } }` | native execute | echo (run) | ObjectQL engine path | draft preview | `AnalyticsService.query` (native / ObjectQL) | HTTP, both routes | |:--|:--|:--|:--|:--|:--|:--| | `$null` or `$exists`, V = `'x'`, `'false'`, `'true'`, `0`, `1`, `null`, `[true]`, `{ $field: 'id' }`, a `Date`, `2n` | r1 r3 (IS NOT NULL) | r1 r3 | r1 r3; the engine received `{ stage: { $ne: null } }` | 400, "not evaluated" | r1 r3 | **200**, r1 r3, 1 statement | | the same under `$not` | r2 | r2 | r2 | 400, "not evaluated" | r2 | 200, r2 | | V = `{ a: 1 }` or `undefined` | 400, type face | = | = | = | = | 400 (the plain object) | | `$null: true` / `$exists: false` (control) | r2 | r2 | r2, `{ stage: null }` | 400, "not evaluated" | r2 | 200, r2 | | `$null: false` / `$exists: true` (control) | r1 r3 | r1 r3 | r1 r3, `{ stage: { $ne: null } }` | 400, "not evaluated" | r1 r3 | 200, r1 r3 | - **Positions.** The top level, `$and`, `$or`, `$not`, and `$not` beside `$ne`; every coerced cell answered the same in each. `{ $not: { stage: { $null: 'true' } } }` served the NULL row the author excluded. - **`$null: null` / `$exists: null` are reachable**, in process and over HTTP (JSON `null`). Both read as IS NOT NULL and answered 200: - the `undefined` gate (#6386) skips both flags by name; - the type face accepts `null`, which is in its accepted set; - the shape face's null carve-outs cover `$in` / `$nin` members, `$between` endpoints and ordering comparands, not the flags. - **The card's HTTP 200 is confirmed on `main`.** `FilterConditionSchema` types a field entry as `z.unknown()`, so the body parse admitted every non-boolean flag on both routes. - **The `FilterArray` spelling carries no flag value.** - `['stage', 'is_null', 'x']` gives `notSet` and `['stage', 'isnotnull', 0]` gives `set`; the third member is filler. - `['stage', '$null', 'x']`, `'null'`, `'exists'` and `'$exists'` are refused as not a filter. - `isFilterAST` answers false for every array embedding an object node (5 shapes probed). - **Other analytics paths through the same lowering** (the card's check): - dataset scope filters and measure filters reach it through the strategies' `normalizeAnalyticsFilterTree` calls; - the preview reaches it through `normalizeWhereComparands`; - the read-scope door does not use it. ## After (head `66266c2a3b`) - **In process.** Every non-boolean row above is refused `INVALID_FILTER` / 400 on all seven in-process columns, the preview included, in the door's sentence (44 of 44 rows). Nothing reaches the database or the engine: 0 statements and 0 `engine.aggregate` calls. - **HTTP re-measure.** Both routes answer **400 `INVALID_FILTER`** for every non-boolean cell, with the message kept and 0 statements. The controls still answer 200 with the same rows. - **Controls, byte for byte.** For `$null` / `$exists` × `true` / `false`, at the top level, under `$not`, and under `$not` beside `$ne`, I captured: - the compiled tree; - the native SQL and its params; - the echo SQL and its params; - the engine `where`. The capture taken on `8d76c2d38c` and the one taken after the fix are byte-identical (10,328 bytes, the same sha256). The test pins them as literals. - **The read scope keeps its own answer**, untouched: - on native, the echo and `AnalyticsService.query` (native): `READ_SCOPE_COMPILE_FAILED` / 500, withheld, in `read-scope-sql`'s sentence; - on the ObjectQL face, `driver-sql` answers it: `INVALID_FILTER` / 400, with the policy content withheld. ## Tests New file `packages/services/service-analytics/src/__tests__/where-boolean-flag-refusal.test.ts`, 63 cases. Every refusal asserts `code` + `status`. - the door rows: 2 flags × 10 non-boolean values; - every position, the nested-relation path, and the received value named in the message; - the `FilterArray` control; - existing refusals keeping their order and sentence; - the measured precedence against the lowering's own refusals, with boolean controls; - every face over a real engine, with statement and engine-call counters; - the preview, whose message is byte-equal to the door's; - the byte-for-byte controls; - the read-scope face on every sub-face, plus `compileScopedFilterToSql`; - stored dataset and measure filters through `queryDataset`. **Pins re-judged, none deleted.** - The census was taken before the change, over `service-analytics`, `packages/rest` and `packages/runtime`, plus a repo-wide test scan. - `rest` and `runtime` had 0 hits asserting this door's admission. `rest-4xx-message-truncation.test.ts` copies a `driver-sql` message and is not this door. - One pin in `filter-normalizer-undefined-comparand.test.ts` asserted the admission: the case "$null / $exists: an undefined flag is refused by the type face; an accepted flag value reads as before". - It asserted `{ $null: 'false' }` and `{ $exists: 'yes' }` lower to `set`. - **Flipped to refusals, with a note.** The `undefined` half is unchanged, and the case title now says what it asserts. **Runs.** - Test-only commit `12e3081867`, before the fix: **53 failed | 10 passed (63)**. - Every door row was red. - Green: the four controls, the type-face and shape-face order pins, the `FilterArray` control and the three read-scope pins. - Head `66266c2a3b`: - `pnpm --filter @objectstack/service-analytics` suite: **125 files, 2949 tests passed**; - `typecheck`: exit 0, and `tsc --listFiles` includes both touched test files; - consumer sanity run against the rebuilt `dist`, not a pin: `packages/rest`'s 10 analytics route suites (148 passed) and `packages/runtime`'s 3 analytics suites (36 passed). ### Ablations Each ran through `scripts/ablation-replace.mjs` from committed state, with the anchor hit ×1 and the restore proven (blob == HEAD, `git diff HEAD` empty). `service-analytics` tests import the subject by relative `src` paths, so no `dist` is in the path, except for leg F. | leg | mutation | red / green | restore | |:--|:--|:--|:--| | A | the gate's one call site removed | **54 red / 43 green** over the new file and the re-judged one: the 53 door rows from the before-fix run plus the re-judged pin. Controls and read-scope pins stayed green, as predicted | blob `a7bdec971782` == HEAD | | B | `$exists` dropped from the gate's set | **27 red / 70 green**. Every red case carries an `$exists` assertion (21 name it, 6 are mixed); every `$null`-only case stayed green | == HEAD | | C | the gate moved ahead of the shape and type faces | **11 red / 192 green** over the new file, the re-judged file and `where-type-face-refusal.test.ts`: the order pins (2 + 2) and 7 existing type-face pins. The first attempt was refused by the tool itself, because the replacement contained the anchor, so the count could not drop. Nothing ran, the restore was proven, and it was re-run with an equivalent spelling | == HEAD | | D | the gate widened to refuse `false` | **2 red / 61 green**: the two `false` controls | == HEAD | | E | `read-scope-sql.ts`'s `assertBooleanFlagComparands` call removed (a transient ablation; the file is not in this diff) | **2 red / 61 green**: the read-scope pin and `compileScopedFilterToSql`. The ObjectQL-face pin stayed green, as predicted, because `driver-sql` answers there | blob `403440964f36` == HEAD | | F | `driver-sql`'s `$null` gate removed and `driver-sql` rebuilt; `ablation-dist-preflight --absent` passed (the marker is absent from all 6 built files) | **1 red / 62 green**: the ObjectQL-face read-scope pin | blob `a89054ee3aab` == HEAD; rebuilt, and preflight shows the marker present in 2 built files with the tree clean | ## Gates (head `66266c2a3b`) - **Derivation.** `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived **60 families** from the real diff. That is 6 more than the dispatch-time list: `check:engine-double-contract`, `check:objectql-double-limit`, `check:query-options-erasure`, `check:type-check-coverage`, `check:type-check-debt` and `check:where-matcher`. - **60 of 60 exit 0.** Among them: - `check-adr-0087-registration --base origin/main`, with the disposition `not-required (no-migration-prescription)`; - `check-changeset-no-major`, `check-empty-changeset` and `check:nul-bytes`; - `check:issue-citations`, which runs only its self-test. The real run (`GITHUB_TOKEN="$GH_TOKEN" node scripts/check-issue-citations.mjs`) judged 29 citations, and all resolve. - **The two built-output gates.** `check:dual-build-cjs-loads` and `check:type-check-debt` read built output. On the first head, before any full build, both answered PREREQUISITE NOT MET, which is not a failure. They ran green after the lint.yml build command (`turbo run build --filter='./packages/*' --filter='./packages/*/*'`). - **Reconciliation.** `--ran`: 60 derived, 60 run, 0 NOT MEASURED, 0 UNRUN. Every entry carries its exit code. - **Merges.** `main` moved twice while this ran: `aa04ea2964` (objectql) and `fa00ebf447` (runtime), both disjoint from this diff. Both were merged, never rebased, and the whole union was re-run on the final head. - **Lint, narrowed and proven.** - `eslint --no-inline-config --format json` over the 3 changed `.ts` files: 3 files, 0 errors, 0 warnings. - `isPathIgnored` is false for all three. The changeset `.md` is ignored and has no config, so it is outside the population. - `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project` and no `projectService`, as its line 327 states). So this diff cannot move the verdict of any untouched file. ## Changeset `.changeset/20040-analytics-null-flag.md`: `minor` with `!` and a `Clause-②: no (narrowing)` line, following the #20035 / #20068 precedents. - **ADR-0087 marker.** It is an HTML-comment line, described here in words so this body keeps it. It declares `not-required (no-migration-prescription)`, and the gate accepts it. - No ledger entry names the flags' boolean domain: the #5347 / #5369 refusals in `driver-sql` and the #6387 refusal were never registered. - A non-boolean flag has no boolean it can be mechanically rewritten to. - **Who is affected.** A text scan over 4598 tracked non-test files found 22 matches for a non-boolean flag. 21 are code comments; the 22nd is an operator-name lookup table in `driver-memory`, not a filter. Stored datasets, widget filters and report runtime filters in a deployment were NOT measured. ## Acceptance notes - **The preview still refuses a BOOLEAN flag ("not evaluated").** It evaluates no `$null` / `$exists` at all (the #19810 table), and that is unchanged and not this card's to change. Widening that table is ruled to land together with the conformance kits. A drafted chart carrying `{ $null: 'x' }` now gets the flag refusal first. Once repaired to `true`, it gets the preview's own refusal, while publish serves it. ## Out-of-scope findings (reported to the seat, not filed here) 1. **Class (c).** The save door admits a non-boolean flag that every query face refuses. - Measured at `66266c2a3b`: - `FilterConditionSchema.safeParse({ stage: { $null: 'x' } })` succeeds; - `DatasetSchema.safeParse` with `filter: { stage: { $null: 'x' } }` (also with `$exists: 'false'` and with `$null: null`) succeeds; - the same save door refuses `$icontains: ''` (#19514). - So a stored dataset or widget filter carrying the shape saves, and then answers 400 on every query face: `driver-sql` since #5347, the analytics door since this PR. - The named producer is whoever authors a dataset `filter`: `defineStack` metadata or the Studio. - `Seam: spec:FilterConditionSchema (checkFilterConditionComparands) → runtime:driver-sql reduceFilterKey | service-analytics normalizeWhereComparands` - Dedupe words: `FilterConditionSchema $null non-boolean admitted at save` · `dataset filter $exists string saves then 400` · `checkFilterConditionComparands boolean flag` --- _Generated by [Claude Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 55daf89 commit 226e00c

4 files changed

Lines changed: 718 additions & 9 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
fix(service-analytics)!: the analytics `where` door refuses a non-boolean `$null` / `$exists` flag, which it used to read as IS NOT NULL, the way every backend and the read-scope compiler already refuse it (#20040)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) no ADR-0087 ledger entry names the $null / $exists boolean domain: the refusals driver-sql makes (#5347, #5369) and the read-scope compiler makes (#6387) were never registered, and this change adds no new transition. It brings the analytics object-form `where`, its dataset and measure filters and the draft-data preview under the refusal every other face already makes. No ledger entry could carry it: a non-boolean flag has no boolean it can be mechanically rewritten to, because the backends disagreed on which one it meant and the string "false" is truthy. Which boolean the author meant is an authoring decision. The table below records the verdict each cell had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what the analytics faces of `@objectstack/service-analytics` accept. A `$null` or `$exists` flag whose value is not a boolean used to compile, in a caller `where`, a dataset `filter` or a measure `filter`, at any depth under `$and` / `$or` / `$not` or inside a nested relation. That covers a string such as `"false"`, a number, `null`, an array, a `Date` and a `{ $field }` reference. It is now refused with `INVALID_FILTER` / 400, in a message that names the operator, the field and the path, before any SQL statement runs or any `engine.aggregate` call is made. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
| flag in an object-form `where` | before | now |
14+
|:--|:--|:--|
15+
| `$null` or `$exists` with a string, a number, `null`, an array, a `Date` or a `{ $field }` reference | read as IS NOT NULL on the native SQL path, the `/analytics/sql` echo and the ObjectQL engine path (the engine received `{ "$ne": null }`); `POST /api/v1/analytics/query` and `/analytics/dataset/query` answered 200 | refused 400: `Operator "$null" on field "stage" requires a boolean comparand (true or false). Received …` |
16+
| the same, under `$not` | the negation of IS NOT NULL: the rows with no value | refused 400 |
17+
| the same, in the draft-data preview | refused 400 as an operator the preview does not evaluate | refused 400, in the published door's words |
18+
| `true` or `false` | IS NULL or IS NOT NULL, per the contract | unchanged: the compiled tree, the SQL and the engine `where` are byte-identical |
19+
| `undefined`, or a plain object | refused 400 by the shared comparand-type face | unchanged, in that face's words |
20+
21+
`FieldOperatorsSchema` in `@objectstack/spec` declares both flags as booleans. The #5347 and #5369 rulings refuse a non-boolean one in every position and on every backend, because the backends read one in opposite directions: `driver-sql` compiled IS NULL for anything but `false`, and the JavaScript drivers compiled IS NOT NULL for anything but `true`. `driver-sql` refuses it, and so does this package's read-scope compiler. This door read every non-boolean as IS NOT NULL, so `"true"` and `"false"` asked for the same rows, and `{ "$null": "true" }` asked for the rows it excludes.
22+
23+
The repair is to write the boolean the filter means. `"$null": true` matches rows with no value and `"$null": false` rows with one; `$exists` is the exact inverse.
24+
25+
Over HTTP, both routes type the `where`, the `runtimeFilter` and the inline `dataset.filter` as `FilterConditionSchema`, whose field entries are open, so the body parse admitted the flag and the service served it. Both routes now answer 400 `INVALID_FILTER` from the service, with no statement run.
26+
27+
A `where` that carries a non-boolean flag and also a defect this compiler finds only while lowering (a field constraint mixing `$` and non-`$` keys, an operator outside the vocabulary, a zero-operator constraint) is now answered with the flag refusal. The code and status are the same 400 `INVALID_FILTER`. A shape or type defect elsewhere in the same `where` is still answered first.
28+
29+
Who is affected: nothing in this repository's examples, seeds, docs or package sources authors a non-boolean flag. A text scan over 4598 tracked non-test files found 22 matches: 21 are code comments, and one is an operator-name lookup table in `driver-memory`, not a filter. Stored datasets, dashboard widget filters and report runtime filters in a deployment were NOT measured.
30+
31+
Not changed: a `true` or `false` flag; the `FilterArray` spelling, whose `is_null` and `is_not_null` take their boolean from the operator name; the read-scope door, which already refused a non-boolean flag in its own withheld envelope.

‎packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@
112112
* split, reconciled against `FieldOperatorsSchema` by the face's own test),
113113
* so `{$null: undefined}` is refused rather than read as `set`. The boolean
114114
* DOMAIN question for an ACCEPTED value (#5347 / #5369 / #6387) is not
115-
* touched: `{$null: 'false'}` still lowers as before.
115+
* touched: `{$null: 'false'}` still lowers as before. [#20040] It does not
116+
* any more: the door refuses every non-boolean flag before any node is
117+
* built, and the case below that pinned the old reading is re-judged.
116118
*
117119
* `assertDefinedComparands` (#6386) stays as `fieldLeaves`' invariant and still
118120
* answers the positions the face does not judge — an `undefined` inside an
@@ -480,7 +482,7 @@ describe('[#6386] the #5146 rewrite cannot swallow the leaf — the gate side is
480482
});
481483

482484
describe('[#6386] what the sweep deliberately leaves alone', () => {
483-
it('$null / $exists: an undefined flag is refused by the type face; an accepted flag value reads as before', () => {
485+
it('$null / $exists: an undefined flag is refused by the type face; a non-boolean accepted type is refused by the door (#20040)', () => {
484486
// Same call as `read-scope-sql`'s twin. ⚠️ The two modules read the flag
485487
// DIFFERENTLY — identity here, truthiness there. Which reading is right for
486488
// a non-boolean value is the boolean-DOMAIN question (#5347 / #5369,
@@ -496,14 +498,29 @@ describe('[#6386] what the sweep deliberately leaves alone', () => {
496498
// runs the same face. The domain question is still not decided here: the
497499
// face accepts every value of the six types, so an accepted non-boolean flag
498500
// keeps this module's identity reading, unchanged.
501+
//
502+
// [#20040] RE-JUDGED, the second half. The two lines that closed this case
503+
// pinned `{$null: 'false'}` and `{$exists: 'yes'}` → `set` (IS NOT NULL):
504+
// the identity reading of an ACCEPTED non-boolean flag, which this file
505+
// kept deliberately undecided. #20040 decided it the way #5347 / #5369
506+
// decided it for every backend, and #6387 for the sibling door: a
507+
// non-boolean flag is REFUSED, because the backends read one in opposite
508+
// directions and `'false'` is truthy. The door now refuses it before any
509+
// node is built, in its `INVALID_FILTER` / 400 envelope, so the two cells
510+
// are asserted as refusals below. The `undefined` half is unchanged: it is
511+
// still the type face's refusal, in the face's sentence.
499512
for (const op of ['$null', '$exists'] as const) {
500513
const err = refusalFor({ d: { [op]: undefined } });
501514
expect(err?.code, op).toBe('INVALID_FILTER');
502515
expect(err?.status, op).toBe(400);
503516
expect(String(err?.message).startsWith(`Filter comparand at where.d.${op} is undefined.`), op).toBe(true);
504517
}
505-
expect(treeFor({ d: { $null: 'false' } })).toEqual({ kind: 'leaf', member: 'd', operator: 'set', values: [] });
506-
expect(treeFor({ d: { $exists: 'yes' } })).toEqual({ kind: 'leaf', member: 'd', operator: 'set', values: [] });
518+
for (const [op, value] of [['$null', 'false'], ['$exists', 'yes']] as const) {
519+
const err = refusalFor({ d: { [op]: value } });
520+
expect(err?.code, op).toBe('INVALID_FILTER');
521+
expect(err?.status, op).toBe(400);
522+
expect(String(err?.message), op).toContain(`Operator "${op}" on field "d" requires a boolean comparand`);
523+
}
507524
});
508525

509526
it('a combinator with an undefined value gets its own, truer refusal', () => {

0 commit comments

Comments
 (0)