diff --git a/.changeset/20010-analytics-where-face-arms.md b/.changeset/20010-analytics-where-face-arms.md new file mode 100644 index 00000000000..c0b684a3e9a --- /dev/null +++ b/.changeset/20010-analytics-where-face-arms.md @@ -0,0 +1,28 @@ +--- +"@objectstack/service-analytics": minor +--- + +fix(service-analytics)!: the analytics `where` door runs every arm of the shared comparand-shape face on the object spelling, not only the equality arm (#20010) + +Clause-②: no (narrowing) + + + +**BREAKING**: this narrows what the analytics faces of `@objectstack/service-analytics` accept. A caller `where`, a dataset `filter` or a measure `filter` that carries one of the shapes below compiled before this change, at any depth under `$and` / `$or` / `$not` or inside a nested relation. It is now refused with `INVALID_FILTER` / 400, carrying the shared face's own message, path and prescription. It ships as `minor` under the launch-window convention for accept-set narrowings. + +| you wrote | what it did before | write instead | +|:--|:--|:--| +| `{ stage: { $in: ['won', null] } }`, meaning "one of these, or empty" | `stage IN ('won', NULL)`: the empty rows were never matched | `{ $or: [{ stage: { $in: ['won'] } }, { stage: { $null: true } }] }` | +| `{ stage: { $nin: ['won', null] } }`, meaning "has a value, and not one of these" | `stage IS NULL OR stage NOT IN ('won', NULL)`: only the empty rows | `{ $and: [{ stage: { $nin: ['won'] } }, { stage: { $null: false } }] }` | +| `{ amount: { $gt: null } }` (or `$gte` / `$lt` / `$lte`) | `amount > NULL`: no row, while for `$lt` / `$lte` the draft preview charted every non-empty row | `{ amount: { $eq: null } }` for "has no value", `{ amount: { $ne: null } }` for "has a value" | +| `{ amount: { $between: [null, 100] } }` | `amount >= NULL AND amount <= 100`: no row | `{ amount: { $lte: 100 } }` for a one-sided range; `$or` with `{ amount: { $null: true } }` to include the empty rows | +| `{ amount: { $between: ['', 100] } }` | `amount >= '' AND amount <= 100`: the blank compared as a value, and the ObjectQL engine path accepted it | the bound you meant, or `{ amount: { $lte: 100 } }` for a one-sided range | +| `{ stage: { $in: 'won' } }` or `{ stage: { $nin: 'won' } }` | laundered into a one-member list | `{ stage: 'won' }` / `{ stage: { $ne: 'won' } }`, or `{ stage: { $in: ['won'] } }` | + +The shared comparand-shape face in `@objectstack/spec` (`assertListComparandShapes`) is the one place that decides whether `$in` / `$nin` / `$between` received a list at all, for every driver. Three rulings put the null and blank positions on that same door: a null list member and a null `$between` endpoint (2026-08-31), a null ordering comparand (2026-09-01), and a blank `$between` endpoint (2026-09-20). The analytics `where` door met that face only for the `FilterArray` spelling (`['stage', 'in', ['won', null]]`), which was already refused. PR #20008 carried the face's equality arm to the object spelling. Every other arm now follows it: each field entry of the object-form `where` is handed to the face after the equality pass, so both spellings of one condition get the same refusal, byte for byte. This holds on the native SQL execute path, the `/analytics/sql` echo and the ObjectQL engine path. The draft-data preview runs the same gate, so a drafted chart refuses what the published chart refuses. Before, the preview charted rows for several of these shapes: `{ amount: { $lt: null } }` charted every non-empty row. + +Three shapes this door already refused now carry the face's wording instead of this package's own, the same wording the `FilterArray` spelling gets: a `$between` that is not a two-element list, a `$between` endpoint that is a `{ $field }` reference, and an `undefined` `$between` endpoint. Their verdict, code and status are unchanged. + +Who is affected: nothing in this repository's examples, seeds, docs or package sources authors any of the shapes. This was measured by a text scan over 4013 non-test files with positive controls. Stored datasets, dashboard widget filters and report runtime filters in a deployment were NOT measured. The authoring schema still admits the shapes, so such a document still publishes, and it is refused when it is charted. + +Not changed: `$ne` with a list is not judged by the face yet, so it compiles as before. `$in: []` / `$nin: []`, falsy list members (`0`, `''`, `false`), every non-null ordering comparand, a `{ $field }` in an ordering slot, and `null` in the equality slot (the has-no-value predicate) all compile as before. The comparand-TYPE face is not run on this door. An `undefined` comparand outside a `$between` endpoint keeps this package's own refusal. diff --git a/packages/rest/src/analytics-filter-refusal-envelope.test.ts b/packages/rest/src/analytics-filter-refusal-envelope.test.ts index b75092c4128..8cec51ea3cd 100644 --- a/packages/rest/src/analytics-filter-refusal-envelope.test.ts +++ b/packages/rest/src/analytics-filter-refusal-envelope.test.ts @@ -194,9 +194,17 @@ describe('[#5352] POST /analytics/dataset/query — a filter refusal reaches the message: /carries a field constraint with zero operators/, }, { + // [#20010] RE-WORDED, same verdict: still a real normalizer refusal, + // still 400 INVALID_FILTER through this seam. PR #20032 hands every field + // entry of an object-form `where` to the shared comparand-shape face + // (`assertListComparandShapes`) before any node is built, and the face's + // own rule answers first — a list operator takes a list, `$between` a + // two-element [min, max] (#5869, moved to the face by #9228). So the + // message is the face's, the same bytes the FilterArray spelling gets; + // the analytics "needs a two-element" sentence is no longer reached. name: 'a $between with one bound', runtimeFilter: { amount: { $between: [10] } }, - message: /needs a two-element \[min, max\] array/, + message: /Operator "\$between" on field "amount" requires a \[min, max\] value array/, }, { name: 'an unsupported top-level operator', diff --git a/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts b/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts index e7cd7fd4fc6..12c0d8aa188 100644 --- a/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts +++ b/packages/services/service-analytics/src/__tests__/comparand-door-single-source.test.ts @@ -124,9 +124,16 @@ const MATRIX: readonly Row[] = [ { label: 'boolean', value: true, bindable: true, renderable: true, whereLike: OK, whereIn: OK, whereEq: OK, scopeLike: OK, scopeIn: OK, scopeEq: OK }, + // [#20010] One cell of this row moved AFTER the #8186 measurement, on + // purpose: the `where` door now hands every field entry to the shared + // comparand-shape face, whose 2026-08-31 ruling (#13357) refuses "a `null` + // member of `$in` / `$nin`" at that door, for every driver. The object + // spelling used to compile it to `status IN (NULL)`; the FilterArray + // spelling was already refused (`where-face-arms-refusal.test.ts`). The + // read-scope cell is that door's own and is not this card's. { label: 'null', value: null, bindable: true, renderable: true, - whereLike: OK, whereIn: OK, whereEq: OK, scopeLike: OK, scopeIn: OK, scopeEq: OK }, + whereLike: OK, whereIn: REFUSED_WHERE, whereEq: OK, scopeLike: OK, scopeIn: OK, scopeEq: OK }, { label: 'Date', value: new Date('2026-01-01T00:00:00.000Z'), bindable: true, renderable: true, whereLike: OK, whereIn: OK, whereEq: OK, scopeLike: OK, scopeIn: OK, scopeEq: OK }, @@ -266,7 +273,13 @@ describe('[#8186] the comparand matrix is unchanged by the door reconciliation', 'string', 'number', 'bigint', 'boolean', 'null', 'Date', ]); for (const row of doorTypes) { - for (const cell of [row.whereLike, row.whereIn, row.whereEq, + // [#20010] ONE exception, and it is not a TYPE verdict: `null` is an + // accepted comparand type, but not as an `$in` MEMBER — the SHAPE face's + // 2026-08-31 carve-out, which this door now runs. Every other position + // of every accepted type still accepts. + const whereIn = row.label === 'null' ? REFUSED_WHERE : OK; + expect(row.whereIn, `${row.label} $in`).toBe(whereIn); + for (const cell of [row.whereLike, row.whereEq, row.scopeLike, row.scopeIn, row.scopeEq]) { expect(cell, row.label).toBe(OK); } diff --git a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts index b6134e3ab46..fc9d0455ad0 100644 --- a/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/comparand-shape-refusal.test.ts @@ -169,9 +169,21 @@ describe('[#5234] the analytics `where` door refuses an uncompilable comparand', expect(tree({ status: { $in: ['a', 'b'] } })).toEqual({ kind: 'leaf', member: 'status', operator: 'in', values: ['a', 'b'], }); - expect(tree({ status: { $in: ['a', null, 5, true] } })).toEqual({ - kind: 'leaf', member: 'status', operator: 'in', values: ['a', null, 5, true], + expect(tree({ status: { $in: ['a', 5, true] } })).toEqual({ + kind: 'leaf', member: 'status', operator: 'in', values: ['a', 5, true], }); + // [#20010] RE-JUDGED. This list carried a `null` member, pinned as a + // legitimate `$in` member TYPE for this guard. It still is one for this + // guard, which asks only "can it bind". But the shared comparand-shape + // face refuses a null list member by its 2026-08-31 ruling (#13357: "A + // `null` member of `$in` / `$nin` … refused at this door"), and this door + // now runs that face first. So the list is refused whole, in the face's + // words and not this guard's. + const err = refusalOf(() => tree({ status: { $in: ['a', null, 5, true] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message.startsWith('Operator "$in" on field "status" does not accept null as a list member')).toBe(true); + expect(err.message).not.toContain('cannot be bound as a SQL parameter'); }); it('keeps every primitive LIKE comparand, including the two #5526 pinned', () => { diff --git a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts index d4d7705998d..3ee142f9ab3 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts @@ -286,23 +286,38 @@ describe("[#7598] the #5222 corpus's REFUSAL arm — routed, or refused at this .toContain('StringOperatorSchema'); expect(refusalOf(() => tree({ amount: { $in: [{ $field: 'budget' }, 1] } })).message) .toContain('cannot be bound as a SQL parameter'); + // [#20010] RE-JUDGED: the `$between` endpoint is now answered one step + // earlier, by the shared comparand-shape face this door hands every field + // entry to, in the face's words (its #7596 endpoint arm, enforced on the + // face itself). Still a third gate with its own wording, which is this case's + // point; the FilterArray spelling gets the same bytes. expect(refusalOf(() => tree({ amount: { $between: [{ $field: 'budget' }, 100] } })).message) - .toContain('may not be a field reference on any backend'); + .toContain('does not accept a { "$field": … } reference as an endpoint'); // …and the scalar position is not refused at all here any more. expect(tree({ amount: { $gt: { $field: 'budget' } } })).toEqual({ kind: 'leaf', member: 'amount', operator: 'gt', values: [{ $field: 'budget' }], }); }); - it('the `$between` refusal names the laundering it prevents, not a bind failure', () => { + it('the `$between` refusal names the endpoint and the two-bound spelling, not a bind failure', () => { // The repair a `$between` author needs is different from the one a // read-scope author needs, so the two sentences are different (#5240 in the // direction that separates rather than merges). + // + // [#20010] RE-JUDGED. This pinned this door's own sentence, which named the + // laundering (`index 1`, `#7596`). The door now hands every field entry to + // the shared comparand-shape face before any leaf is built, and the face's + // endpoint arm answers first — the 2026-08-11 ruling (#7596), as the face + // records it: "The reference stays legal in the four ORDERING slots … + // the alternative this refusal prescribes". Same verdict, the face's words, + // which are the words the FilterArray spelling already got on this door. + // What the case protects is unchanged: the side is named, and the message + // points at the spelling that IS served. const err = refusalOf(() => tree({ amount: { $between: [0, { $field: 'budget' }] } })); expect(err.code).toBe('INVALID_FILTER'); expect(err.status).toBe(400); - expect(err.message).toContain('index 1'); - expect(err.message).toContain('#7596'); + expect(err.message).toContain('at where.amount.$between[1], the MAX bound'); + expect(err.message).not.toContain('cannot be bound'); // It points at the spelling that IS served, rather than at "use a literal" // alone — the capability exists one operator away. expect(err.message).toContain('"$gte"'); diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts index e6f0f9c2865..8037af79343 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts @@ -178,14 +178,18 @@ const MEASURED: Array<{ name: string; where: unknown; path: string; wasReadAs: s * refused as a whole before this gate runs, as the twin always did, and the row * moved to `where-equality-slot-list-refusal.test.ts` ("a list is diagnosed as * the list, not by one of its members"). + * + * [#20010] And a third: a `$between` bound, `{d: {$between: [undefined, 5]}}` + * → `"d".$between[0]`. The door now hands every field entry to the shared + * comparand-shape face before this gate runs, and the face's 2026-09-20 + * endpoint ruling (#19071) names this exact spelling: "`''` and `undefined` + * are refused, naming the blank side (MIN / MAX and the index)". So the bound + * is still refused, INVALID_FILTER / 400, but in the face's words, which are + * the words the FilterArray spelling already got; the row moved to + * `where-face-arms-refusal.test.ts`. Every other position below keeps this + * gate's sentence: the shape face judges `undefined` only as a range endpoint. */ const BEYOND_THE_TABLE: Array<{ name: string; where: unknown; path: string; wasReadAs: string }> = [ - { - name: "a $between bound — lowered to the leaf's comparand", - where: { d: { $between: [undefined, 5] } }, - path: '"d".$between[0]', - wasReadAs: 'd gte [null] AND d lte [5]', - }, { name: 'a member of a $nin list', where: { d: { $nin: [undefined] } }, @@ -238,38 +242,20 @@ const NULL_CONTROL: Array<{ name: string; where: unknown; tree: unknown }> = [ where: { d: { $ne: null } }, tree: { kind: 'leaf', member: 'd', operator: 'set', values: [] }, }, - { - name: '{$gt: null} → a real comparison binding NULL (#5526)', - where: { d: { $gt: null } }, - tree: { kind: 'leaf', member: 'd', operator: 'gt', values: [null] }, - }, - { - name: '{$in: [null]} → a list member, not a predicate', - where: { d: { $in: [null] } }, - tree: { kind: 'leaf', member: 'd', operator: 'in', values: [null] }, - }, - { - name: '{$nin: [null]} → null-safe guarded (#5298)', - where: { d: { $nin: [null] } }, - tree: { - kind: 'or', - children: [ - { kind: 'leaf', member: 'd', operator: 'notSet', values: [] }, - { kind: 'leaf', member: 'd', operator: 'notIn', values: [null] }, - ], - }, - }, - { - name: '{$between: [null, 5]} → lowered to two bounds', - where: { d: { $between: [null, 5] } }, - tree: { - kind: 'and', - children: [ - { kind: 'leaf', member: 'd', operator: 'gte', values: [null] }, - { kind: 'leaf', member: 'd', operator: 'lte', values: [5] }, - ], - }, - }, + // [#20010] RE-JUDGED. Four rows stood here: `{$gt: null}` → `gt [null]` + // (#5526's by-construction reading), `{$in: [null]}` → `in [null]`, + // `{$nin: [null]}` → the #5298-guarded `notIn [null]`, and + // `{$between: [null, 5]}` → `gte [null] AND lte [5]`. Each is a position the + // shared comparand-shape face refuses by ruling, for every driver: the null + // list member and the null `$between` endpoint by the 2026-08-31 ruling + // (#13357), the null ordering comparand by the 2026-09-01 ruling (#14080) — + // a position the face's docblock records #5332's landing had called one + // "no ruling covers", so #5526's reading of it was a consequence of deleting + // an encoder, never a ruling of this door's. The door now runs that + // face, so each shape is refused whole (INVALID_FILTER / 400, in the face's + // words, `where-face-arms-refusal.test.ts`) and is no longer an accepted + // position for this group to hold still. What the group protects is + // unchanged: none of the four is refused as an UNDEFINED comparand. { name: "{$contains: null} → LIKE '%null%' (#5526)", where: { d: { $contains: null } }, diff --git a/packages/services/service-analytics/src/__tests__/filter-operator-coverage.test.ts b/packages/services/service-analytics/src/__tests__/filter-operator-coverage.test.ts index 9a14e8cb597..5dbf0744b00 100644 --- a/packages/services/service-analytics/src/__tests__/filter-operator-coverage.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-operator-coverage.test.ts @@ -196,8 +196,11 @@ describe('analytics filters — every authorable operator reaches the query (#41 }); it('a malformed $between throws rather than binding a half-open guess', () => { + // [#20010] The shared comparand-shape face answers this one first now, in + // its own words (the words the FilterArray spelling gets); this door's + // "two-element" sentence is no longer reached from `normalizeAnalyticsFilterTree`. expect(() => normalizeAnalyticsFilterTree({ where: { score: { $between: [10] } } })).toThrow( - /two-element/, + /requires a \[min, max\] value array/, ); }); }); diff --git a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts index 9983999bc79..8eb2db97f84 100644 --- a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts @@ -94,9 +94,16 @@ const REFUSALS: Array<{ issueBullet: true, }, { + // [#20010] RE-WORDED, same input, same verdict, same envelope. The door now + // hands every field entry to the shared comparand-shape face before any + // leaf is built, and the face's arity arm answers first, in the words the + // FilterArray spelling already got. `fieldLeaves`' own "needs a + // two-element" check stays as that function's invariant and is no longer + // reached from this door. The row keeps #5352's bullet: the authoring + // mistake is the one #5352 named. name: '$between without exactly two bounds', where: { amount: { $between: [10] } }, - message: /needs a two-element \[min, max\] array/, + message: /Operator "\$between" on field "amount" requires a \[min, max\] value array/, issueBullet: true, }, // FLIPPED with the #5322 ruling: these two entries were "$and/$or with an diff --git a/packages/services/service-analytics/src/__tests__/filter-value-type-fidelity.test.ts b/packages/services/service-analytics/src/__tests__/filter-value-type-fidelity.test.ts index 822d3b27292..4fd6e27412b 100644 --- a/packages/services/service-analytics/src/__tests__/filter-value-type-fidelity.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-value-type-fidelity.test.ts @@ -461,17 +461,33 @@ describe("[#5526] analytics SQL path — a text column's own spelling is what ge expect(await bindsFor({ code: { $eq: false } as never })).toEqual([0]); }); - it('a null comparand in an ORDERING position binds NULL, so the widget draws nothing', async () => { - // Uncovered by any ruling before this (#5332 said so explicitly): the encoder - // wrote `''`, i.e. `code > ''`, a real comparison that on a text column - // returned rows. NULL is UNKNOWN for every row — no rows, no accident. + it('a null comparand in an ORDERING position is REFUSED before anything binds', async () => { + // Uncovered by any ruling when #5526 landed (#5332 said so explicitly): the + // encoder wrote `''`, i.e. `code > ''`, a real comparison that on a text + // column returned rows, and #5526 left it binding NULL (UNKNOWN for every + // row: no rows, no accident). + // + // [#20010] RE-JUDGED. The position has a ruling now: 2026-09-01 (#14080), + // quoted from the shared comparand-shape face — "A `null` comparand of + // `$gt` / `$gte` / `$lt` / `$lte` … refused at this door, same envelope, so + // the divergent cells are constructively unreachable". This door runs that + // face on the object spelling, so the filter is refused INVALID_FILTER / + // 400 and no statement binds anything. The concern this case was written + // for still holds, more strongly: no `''` is ever bound. bound.length = 0; - const result = await new NativeSQLStrategy().execute( - { cube: 'orders', measures: ['total'], dimensions: ['id'], where: { code: { $gt: null } } } as AnalyticsQuery, - ctx, - ); - expect(bound[0]).toEqual([null]); - expect(result.rows).toEqual([]); + let err: { code?: unknown; status?: unknown; message?: unknown } | undefined; + try { + await new NativeSQLStrategy().execute( + { cube: 'orders', measures: ['total'], dimensions: ['id'], where: { code: { $gt: null } } } as AnalyticsQuery, + ctx, + ); + } catch (e) { + err = e as typeof err; + } + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + expect(String(err?.message).startsWith('Operator "$gt" on field "code" does not accept a null comparand')).toBe(true); + expect(bound).toEqual([]); }); }); diff --git a/packages/services/service-analytics/src/__tests__/where-face-arms-refusal.test.ts b/packages/services/service-analytics/src/__tests__/where-face-arms-refusal.test.ts new file mode 100644 index 00000000000..d8efce7b217 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/where-face-arms-refusal.test.ts @@ -0,0 +1,489 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20010] The analytics `where` door runs EVERY arm of the shared + * comparand-shape face on the object spelling, not only the equality arm. + * + * `assertListComparandShapes` (`@objectstack/spec/data`) is "the one place that + * decides whether `$in` / `$nin` / `$between` received a list at all, for every + * driver", and it carries the carve-outs ruled onto that door: + * + * - 2026-08-31 (#13357): "A `null` member of `$in` / `$nin`, and a `null` + * `$between` endpoint (#13495's shape), are refused at this door"; + * - 2026-09-01 (#14080): "A `null` comparand of `$gt` / `$gte` / `$lt` / + * `$lte` … refused at this door, same envelope, so the divergent cells are + * constructively unreachable"; + * - 2026-09-20 (#19071): "`''` and `undefined` are refused, naming the blank + * side (MIN / MAX and the index)". + * + * This door met the face for the `FilterArray` spelling only (inside + * `parseFilterAST`), and PR #20008 carried the equality arm alone to the object + * spelling. Measured over a real engine before this change (recorded on the + * branch, `a508dddc2c`), the object spelling of each arm compiled while its + * `FilterArray` spelling was refused `INVALID_FILTER` / 400: + * + * | object `where` | before | + * |---|---| + * | `{ stage: { $in: ['won', null] } }` | `stage IN ('won', NULL)`: d1; the draft preview served d1, d4 | + * | `{ stage: { $nin: ['won', null] } }` | `stage IS NULL OR stage NOT IN (…)`: d4 only | + * | `{ amt: { $lt: null } }` | `amt < NULL`: no row; the draft preview served d1, d2, d3, d5 | + * | `{ amt: { $between: [null, 5] } }` | `amt >= NULL AND amt <= 5`; the engine path refused it as `$gte` | + * | `{ amt: { $between: ['', 5] } }` | `amt >= '' AND amt <= 5`; the engine path ACCEPTED it | + * | `{ stage: { $in: 'won' } }` | laundered to `stage IN ('won')`: d1 | + * | `{ stage: { $nin: 'won' } }` | laundered to a list: d2, d3, d4, d5 | + * + * Seven blocks: every arm refused at every depth; one condition, one wording + * (object spelling = `FilterArray` spelling = the face, byte for byte); what + * is diagnosed first; the arm this card does NOT move (`$ne`, waiting on + * #19886) held to the face's own answer; the neighbouring shapes that compile + * exactly as before; the four faces over a real engine; a stored dataset + * through the service doors. + * + * Every refusal asserts the ADR-0112 envelope (`code` + `status`); a bare + * `toThrow()` would be satisfied by any uncoded error. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { assertListComparandShapes, type Cube } from '@objectstack/spec/data'; +import { DatasetSchema, type Dataset } from '@objectstack/spec/ui'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; + +import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; +import { evaluateAnalyticsQueryOverRows } from '../preview-evaluator.js'; +import { AnalyticsService } from '../analytics-service.js'; + +interface Refusal extends Error { + code?: unknown; + status?: unknown; +} + +const tree = (where: unknown) => normalizeAnalyticsFilterTree({ where } as never); + +function refusalOf(run: () => unknown): Refusal { + let out: unknown; + try { + out = run(); + } catch (e) { + return e as Refusal; + } + throw new Error(`expected a refusal, but it answered ${JSON.stringify(out)}`); +} + +function expectEnvelope(err: Refusal): void { + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); +} + +/** The face's own refusal of `node`, read straight from `@objectstack/spec`. */ +function faceSays(node: unknown): string { + return refusalOf(() => assertListComparandShapes(node)).message; +} + +/** The face's opening sentence for each arm. */ +const NULL_MEMBER = (op: string, f: string) => `Operator "${op}" on field "${f}" does not accept null as a list member`; +const NULL_ORDERING = (op: string, f: string) => `Operator "${op}" on field "${f}" does not accept a null comparand`; +const NULL_BOUND = (f: string) => `Operator "$between" on field "${f}" requires two non-null bounds`; +const BLANK_BOUND = (f: string) => `Operator "$between" on field "${f}" requires two non-blank bounds`; +const NON_LIST = (op: string, f: string) => `Operator "${op}" on field "${f}" requires an ARRAY of values`; +const MALFORMED_RANGE = (f: string) => `Operator "$between" on field "${f}" requires a [min, max] value array`; +const FIELD_BOUND = (f: string) => `Operator "$between" on field "${f}" does not accept a { "$field": … } reference as an endpoint`; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#20010] every arm of the shared comparand-shape face refuses the object spelling', () => { + const CASES: Array<[string, unknown, string, string]> = [ + // 2026-08-31 — a null list member. + ['a null $in member', { stage: { $in: ['won', null] } }, NULL_MEMBER('$in', 'stage'), 'where.stage.$in[1]'], + ['$in: [null]', { stage: { $in: [null] } }, NULL_MEMBER('$in', 'stage'), 'where.stage.$in[0]'], + ['a null $nin member', { stage: { $nin: ['won', null] } }, NULL_MEMBER('$nin', 'stage'), 'where.stage.$nin[1]'], + // 2026-09-01 — a null ordering comparand, all four slots. + ['$gt: null', { amt: { $gt: null } }, NULL_ORDERING('$gt', 'amt'), 'where.amt.$gt'], + ['$gte: null', { amt: { $gte: null } }, NULL_ORDERING('$gte', 'amt'), 'where.amt.$gte'], + ['$lt: null', { amt: { $lt: null } }, NULL_ORDERING('$lt', 'amt'), 'where.amt.$lt'], + ['$lte: null', { amt: { $lte: null } }, NULL_ORDERING('$lte', 'amt'), 'where.amt.$lte'], + // 2026-08-31 — a null $between endpoint, either side. + ['a null $between MIN', { amt: { $between: [null, 5] } }, NULL_BOUND('amt'), 'where.amt.$between[0]'], + ['a null $between MAX', { amt: { $between: [1, null] } }, NULL_BOUND('amt'), 'where.amt.$between[1]'], + // 2026-09-20 — a blank $between endpoint: the empty string and undefined. + ['an empty-string $between MIN', { amt: { $between: ['', 5] } }, BLANK_BOUND('amt'), 'where.amt.$between[0]'], + ['an empty-string $between MAX', { amt: { $between: [1, ''] } }, BLANK_BOUND('amt'), 'where.amt.$between[1]'], + ['an undefined $between MIN', { amt: { $between: [undefined, 5] } }, BLANK_BOUND('amt'), 'where.amt.$between[0]'], + // #5869's rule, on the face since #9228 — a list operator given no list. + ['a scalar $in', { stage: { $in: 'won' } }, NON_LIST('$in', 'stage'), 'where.stage.$in'], + ['a scalar $nin', { stage: { $nin: 'won' } }, NON_LIST('$nin', 'stage'), 'where.stage.$nin'], + ['$in: null', { stage: { $in: null } }, NON_LIST('$in', 'stage'), 'where.stage.$in'], + ['a one-bound $between', { amt: { $between: [1] } }, MALFORMED_RANGE('amt'), 'where.amt.$between'], + ['a scalar $between', { amt: { $between: 5 } }, MALFORMED_RANGE('amt'), 'where.amt.$between'], + // 2026-08-11 (#7596), enforced on the face itself — a { $field } endpoint. + ['a { $field } $between endpoint', { amt: { $between: [{ $field: 'id' }, 5] } }, FIELD_BOUND('amt'), 'where.amt.$between[0]'], + // Depth: the face's own traversal. + ['under $and', { $and: [{ id: 'd3' }, { amt: { $gt: null } }] }, NULL_ORDERING('$gt', 'amt'), 'where.$and[1].amt.$gt'], + ['under $or', { $or: [{ id: 'd3' }, { stage: { $in: ['won', null] } }] }, NULL_MEMBER('$in', 'stage'), 'where.$or[1].stage.$in[1]'], + ['under $not', { $not: { amt: { $lt: null } } }, NULL_ORDERING('$lt', 'amt'), 'where.$not.amt.$lt'], + ['under $not over $or', { $not: { $or: [{ id: 'd3' }, { stage: { $nin: 'won' } }] } }, NON_LIST('$nin', 'stage'), 'where.$not.$or[1].stage.$nin'], + ['beside a legal operator', { amt: { $gte: 1, $lte: null } }, NULL_ORDERING('$lte', 'amt'), 'where.amt.$lte'], + // One step past the face: a nested relation compiles to its dotted member. + ['inside a nested relation', { acct: { amt: { $gt: null } } }, NULL_ORDERING('$gt', 'amt'), 'where.acct.amt.$gt'], + ['a dotted member', { 'acct.amt': { $between: ['', 5] } }, BLANK_BOUND('acct.amt'), 'where.acct.amt.$between[0]'], + ]; + + for (const [name, where, opening, path] of CASES) { + it(`${name}: INVALID_FILTER / 400, the face's refusal at ${path}`, () => { + const err = refusalOf(() => tree(where)); + expectEnvelope(err); + expect(err.message.startsWith(opening)).toBe(true); + expect(err.message).toContain(`${path}`); + // The face's closing sentence: the query did not run. + expect(err.message).toContain('The filter was NOT applied'); + }); + } +}); + +describe('[#20010] one condition, one wording — the object spelling, the FilterArray spelling and the face agree byte for byte', () => { + const PAIRS: Array<[string, unknown, unknown]> = [ + ['a null $in member', { stage: { $in: ['won', null] } }, ['stage', 'in', ['won', null]]], + ['a null $nin member', { stage: { $nin: ['won', null] } }, ['stage', 'not_in', ['won', null]]], + ['$gt: null', { amt: { $gt: null } }, ['amt', '>', null]], + ['$gte: null', { amt: { $gte: null } }, ['amt', '>=', null]], + ['$lt: null', { amt: { $lt: null } }, ['amt', 'before', null]], + ['$lte: null', { amt: { $lte: null } }, ['amt', '<=', null]], + ['a null $between endpoint', { amt: { $between: [null, 5] } }, ['amt', 'between', [null, 5]]], + ['an empty-string $between endpoint', { amt: { $between: ['', 5] } }, ['amt', 'between', ['', 5]]], + ['an undefined $between endpoint', { amt: { $between: [undefined, 5] } }, ['amt', 'between', [undefined, 5]]], + ['a { $field } $between endpoint', { amt: { $between: [1, { $field: 'id' }] } }, ['amt', 'between', [1, { $field: 'id' }]]], + ['a one-bound $between', { amt: { $between: [1] } }, ['amt', 'between', [1]]], + ['a scalar $in', { stage: { $in: 'won' } }, ['stage', 'in', 'won']], + ['a scalar $nin', { stage: { $nin: 'won' } }, ['stage', 'nin', 'won']], + ['under $and', { $and: [{ id: 'd3' }, { amt: { $gt: null } }] }, ['and', ['id', '=', 'd3'], ['amt', '>', null]]], + ['under $or', { $or: [{ id: 'd3' }, { stage: { $in: ['won', null] } }] }, ['or', ['id', '=', 'd3'], ['stage', 'in', ['won', null]]]], + ['a dotted member', { 'acct.amt': { $gt: null } }, ['acct.amt', '>', null]], + ]; + + for (const [name, object, array] of PAIRS) { + it(name, () => { + const face = faceSays(object); + const objectErr = refusalOf(() => tree(object)); + const arrayErr = refusalOf(() => tree(array)); + expectEnvelope(objectErr); + expectEnvelope(arrayErr); + expect(objectErr.message).toBe(face); + expect(arrayErr.message).toBe(face); + }); + } + + it('the explicit `$not` spelling reads as the face reads it', () => { + const where = { $not: { amt: { $between: [null, 5] } } }; + expect(refusalOf(() => tree(where)).message).toBe(faceSays(where)); + }); +}); + +describe('[#20010] what is diagnosed first', () => { + const IMPLICIT = (f: string) => `The implicit-equality comparand on field "${f}" requires a single comparable value`; + + it('the equality-slot list first, as #19888 ordered it — even when another refused entry comes earlier', () => { + const err = refusalOf(() => tree({ stage: { $in: ['won', null] }, id: ['d1', 'd2'] })); + expectEnvelope(err); + expect(err.message.startsWith(IMPLICIT('id'))).toBe(true); + }); + + it('then the face, before this door\'s own member gates (#6386 undefined, #6444 mixed wrapper, #5240 empty wrapper)', () => { + // A null member beside an undefined one: the face's sentence, not #6386's. + const a = refusalOf(() => tree({ stage: { $in: [null, undefined] } })); + expect(a.message.startsWith(NULL_MEMBER('$in', 'stage'))).toBe(true); + expect(a.message).not.toContain('is undefined'); + // A null ordering comparand beside a non-$ sibling: the face's, not #6444's. + const b = refusalOf(() => tree({ amt: { $gt: null, nested: 'x' } })); + expect(b.message.startsWith(NULL_ORDERING('$gt', 'amt'))).toBe(true); + expect(b.message).not.toContain('mixes $-operator keys'); + }); + + it('CONTROL: an `undefined` outside a `$between` endpoint keeps #6386\'s own sentence — the TYPE face is not run here', () => { + for (const [where, path] of [ + [{ amt: { $gt: undefined } }, '"amt".$gt'], + [{ stage: { $in: [undefined] } }, '"stage".$in[0]'], + [{ stage: undefined }, '"stage"'], + ] as const) { + const err = refusalOf(() => tree(where)); + expectEnvelope(err); + expect(err.message).toContain(`comparand at ${path} is undefined`); + } + }); +}); + +describe('[#20010] `$ne` with a list is the face\'s to judge, and this door answers what the face answers', () => { + // #19886 stage 2 puts the `$ne` refusal on the shared face. Until then the + // face accepts it and this card does not move it; the day the face refuses + // it, this door refuses it too, with no change here. Held as PARITY with the + // face so the test does not pin the #19886 defect in either direction. + for (const where of [ + { stage: { $ne: ['won', 'lost'] } }, + { $not: { stage: { $ne: ['won'] } } }, + ]) { + it(JSON.stringify(where), () => { + let face: string | undefined; + try { + assertListComparandShapes(where); + } catch (e) { + face = (e as Error).message; + } + let door: string | undefined; + try { + tree(where); + } catch (e) { + door = (e as Error).message; + } + expect(door).toBe(face); + }); + } +}); + +describe('[#20010] the neighbouring shapes compile exactly as before', () => { + const leaf = (member: string, operator: string, values: unknown[]) => ({ kind: 'leaf', member, operator, values }); + const ACCEPTED: Array<[string, unknown, unknown]> = [ + ['a list under $in', { stage: { $in: ['won', 'lost'] } }, leaf('stage', 'in', ['won', 'lost'])], + ['falsy $in members are values, not blanks', { stage: { $in: [0, '', false] } }, leaf('stage', 'in', [0, '', false])], + ['the empty $in — the FALSE constant', { stage: { $in: [] } }, { kind: 'const', value: false }], + ['the empty $nin — the TRUE constant', { stage: { $nin: [] } }, { kind: 'const', value: true }], + ['a scalar ordering comparand', { amt: { $gt: 5 } }, leaf('amt', 'gt', [5])], + ['a { $field } in an ordering slot (served on the engine path)', { amt: { $gt: { $field: 'id' } } }, leaf('amt', 'gt', [{ $field: 'id' }])], + ['the implicit null — the has-no-value predicate', { stage: null }, leaf('stage', 'notSet', [])], + ['$eq: null — the same predicate', { stage: { $eq: null } }, leaf('stage', 'notSet', [])], + ['$ne: null — has a value', { stage: { $ne: null } }, leaf('stage', 'set', [])], + ["$contains: null — LIKE '%null%' (#5526)", { stage: { $contains: null } }, leaf('stage', 'contains', [null])], + ['a nested-relation scalar', { acct: { amt: 5 } }, leaf('acct.amt', 'equals', [5])], + ]; + + for (const [name, where, expected] of ACCEPTED) { + it(name, () => { + expect(tree(where)).toEqual(expected); + }); + } + + const between = (lo: unknown, hi: unknown) => ({ + kind: 'and', + children: [leaf('amt', 'gte', [lo]), leaf('amt', 'lte', [hi])], + }); + + for (const [name, bounds] of [ + ['numbers', [1, 5]], + ['zero to zero — falsy endpoints are endpoints', [0, 0]], + ["ISO strings ('0' / '9' are values)", ['0', '9']], + // The face does not trim: a whitespace-only endpoint passes the schema door + // too (the 2026-09-20 ruling's own scope). + ['a whitespace-only endpoint', [' ', 'M']], + ] as const) { + it(`$between keeps its two bounds: ${name}`, () => { + expect(tree({ amt: { $between: bounds } })).toEqual(between(bounds[0], bounds[1])); + }); + } +}); + +// ───────────────────────────────────────────────────────────────────────────── + +const OBJECT = 'deal'; +const ROWS = [ + { id: 'd1', amt: 1, stage: 'won' }, + { id: 'd2', amt: 5, stage: 'lost' }, + { id: 'd3', amt: 10, stage: 'open' }, + { id: 'd4', amt: null, stage: null }, + { id: 'd5', amt: 3, stage: '' }, +]; +const CUBE: Cube = { + name: 'deals', + sql: OBJECT, + measures: { n: { sql: '*', type: 'count', title: 'n' } }, + dimensions: Object.fromEntries( + [['id', 'string'], ['amt', 'number'], ['stage', 'string']].map(([n, t]) => [n, { name: n, label: n, type: t, sql: n }]), + ), + public: false, +} as unknown as Cube; + +describe('[#20010] every analytics face refuses before anything runs (real engine)', () => { + let driver: SqliteWasmDriver; + let statements = 0; + let aggregates = 0; + + const runRawSql = async (sql: string, params: unknown[]): Promise[]> => { + statements++; + const result = await driver.execute(sql.replace(/\$\d+/g, '?'), params); + if (Array.isArray(result)) return result as Record[]; + if (result && typeof result === 'object' && 'rows' in (result as Record)) { + return (result as { rows: Record[] }).rows; + } + return []; + }; + + const ctxFor = (nativeSql: boolean): StrategyContext => + ({ + getCube: (name: string) => (name === 'deals' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql, objectqlAggregate: !nativeSql, inMemory: false }), + getReadScope: () => undefined, + executeRawSql: (_object: string, sql: string, params: unknown[]) => runRawSql(sql, params), + // The engine path: a probe `engine.aggregate` that serves the rows its + // `$in` filter names, so the CONTROL below is a real answer. + executeAggregate: async (_object: string, opts: { filter?: Record }) => { + aggregates++; + const f = opts.filter ?? {}; + const inList = (f.stage as { $in?: unknown[] } | undefined)?.$in; + return ROWS.filter((r) => (inList ? inList.includes(r.stage) : true)).map((r) => ({ id: r.id, n: 1 })); + }, + sqlDialect: () => 'sqlite', + }) as unknown as StrategyContext; + + const q = (where: unknown) => ({ cube: 'deals', dimensions: ['id'], measures: ['n'], where }) as unknown as AnalyticsQuery; + const ids = (rows: Array>) => rows.map((r) => String(r.id)).sort(); + + const FACES: Array<[string, (where: unknown) => Promise]> = [ + ['native execute', async (where) => ids((await new NativeSQLStrategy().execute(q(where), ctxFor(true))).rows)], + ['/analytics/sql echo', async (where) => { + const { sql, params } = await new ObjectQLStrategy().generateSql(q(where), ctxFor(false)); + return ids(await runRawSql(sql, params)); + }], + ['ObjectQL engine path', async (where) => ids((await new ObjectQLStrategy().execute(q(where), ctxFor(false))).rows)], + ['draft preview', async (where) => ids(evaluateAnalyticsQueryOverRows(q(where), CUBE, ROWS.map((r) => ({ ...r }))).rows)], + ]; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { + name: OBJECT, + fields: { + id: { type: 'text', name: 'id' }, + amt: { type: 'number', name: 'amt' }, + stage: { type: 'text', name: 'stage' }, + }, + } as never, + ]); + for (const row of ROWS) await driver.create(OBJECT, { ...row }); + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + it('CONTROL: no where serves every row, and a legal $in serves exactly the named rows', async () => { + // Without this, the refusals below could pass on a harness that serves nothing. + for (const [face, run] of FACES) { + expect(await run(undefined), `${face}: no where`).toEqual(['d1', 'd2', 'd3', 'd4', 'd5']); + expect(await run({ stage: { $in: ['won', 'lost'] } }), `${face}: $in`).toEqual(['d1', 'd2']); + } + }); + + for (const [name, where, opening] of [ + ['a null $in member', { stage: { $in: ['won', null] } }, NULL_MEMBER('$in', 'stage')], + ['a null $nin member', { stage: { $nin: ['won', null] } }, NULL_MEMBER('$nin', 'stage')], + ['$gt: null', { amt: { $gt: null } }, NULL_ORDERING('$gt', 'amt')], + ['$lt: null', { amt: { $lt: null } }, NULL_ORDERING('$lt', 'amt')], + ['a null $between endpoint', { amt: { $between: [null, 5] } }, NULL_BOUND('amt')], + ['an empty-string $between endpoint', { amt: { $between: ['', 5] } }, BLANK_BOUND('amt')], + ['a scalar $in', { stage: { $in: 'won' } }, NON_LIST('$in', 'stage')], + ['a scalar $nin', { stage: { $nin: 'won' } }, NON_LIST('$nin', 'stage')], + ] as const) { + for (const [face, run] of FACES) { + it(`${face}: ${name} is refused, and nothing reaches the engine`, async () => { + statements = 0; + aggregates = 0; + let err: Refusal | undefined; + let served: string[] | undefined; + try { + served = await run(where); + } catch (e) { + err = e as Refusal; + } + expect(served, `${face}: expected a refusal, got rows`).toBeUndefined(); + expectEnvelope(err as Refusal); + expect((err as Refusal).message.startsWith(opening)).toBe(true); + expect(statements).toBe(0); + expect(aggregates).toBe(0); + }); + } + } +}); + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#20010] a STORED dataset carrying one of the shapes is refused on every service door', () => { + // Built without the shape and then given it, so this file pins the RUNTIME + // answer and not whether the authoring schema admits the shape. + const BASE = DatasetSchema.parse({ + name: 'deal_metrics', + label: 'Deal Metrics', + object: OBJECT, + dimensions: [{ name: 'stage', label: 'Stage', field: 'stage', type: 'string' }], + measures: [{ name: 'deal_count', label: 'Deals', aggregate: 'count' }], + }) as Dataset; + const withScope = (filter: unknown): Dataset => ({ ...BASE, filter } as Dataset); + const withMeasureFilter = (filter: unknown): Dataset => + ({ ...BASE, measures: [{ ...BASE.measures[0], filter }] } as Dataset); + + function service(preview: boolean) { + const calls = { aggregate: 0, raw: 0 }; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async () => { + calls.aggregate++; + return [{ stage: 'won', deal_count: 1 }]; + }, + executeRawSql: async () => { + calls.raw++; + return []; + }, + ...(preview ? { draftRowsResolver: async () => ROWS.map((r) => ({ ...r })) } : {}), + }); + return { svc, calls }; + } + + const DOORS: Array<[string, (ds: Dataset) => Promise]> = [ + ['queryDataset (the dashboard door)', (ds) => service(false).svc.queryDataset!(ds, { measures: ['deal_count'], dimensions: ['stage'] })], + ['queryDataset over a pending seed draft (the draft preview)', (ds) => service(true).svc.queryDataset!(ds, { measures: ['deal_count'], dimensions: ['stage'] }, undefined, { previewDrafts: true })], + ]; + + const STORED: Array<[string, (f: unknown) => Dataset, unknown, string]> = [ + ['the dataset scope filter, a null $in member', withScope, { stage: { $in: ['won', null] } }, NULL_MEMBER('$in', 'stage')], + ['a measure filter, a scalar $nin', withMeasureFilter, { stage: { $nin: 'lost' } }, NON_LIST('$nin', 'stage')], + ]; + + for (const [label, filterOf, where, opening] of STORED) { + for (const [door, run] of DOORS) { + it(`${door}: ${label} → INVALID_FILTER / 400`, async () => { + let err: Refusal | undefined; + try { + await run(filterOf(where)); + } catch (e) { + err = e as Refusal; + } + expectEnvelope(err as Refusal); + expect((err as Refusal).message.startsWith(opening)).toBe(true); + }); + } + } + + it('the registered cube on the ObjectQL door refuses before engine.aggregate runs', async () => { + const { svc, calls } = service(false); + svc.registerDataset(withScope({ stage: { $in: ['won', null] } })); + let err: Refusal | undefined; + try { + await svc.query({ cube: 'deal_metrics', measures: ['deal_count'] } as AnalyticsQuery); + } catch (e) { + err = e as Refusal; + } + expectEnvelope(err as Refusal); + expect(calls.aggregate).toBe(0); + }); + + it('CONTROL: the same dataset with the prescribed spelling answers on the dashboard door', async () => { + // The face's own prescription for "one of […] OR has no value". + const { svc, calls } = service(false); + const result = (await svc.queryDataset!( + withScope({ $or: [{ stage: { $in: ['won'] } }, { stage: { $null: true } }] }), + { measures: ['deal_count'], dimensions: ['stage'] }, + )) as { rows: unknown[] }; + expect(result.rows.length).toBeGreaterThan(0); + expect(calls.aggregate).toBeGreaterThan(0); + }); +}); diff --git a/packages/services/service-analytics/src/preview-evaluator.ts b/packages/services/service-analytics/src/preview-evaluator.ts index 711d7c256d6..42209acbb5e 100644 --- a/packages/services/service-analytics/src/preview-evaluator.ts +++ b/packages/services/service-analytics/src/preview-evaluator.ts @@ -26,6 +26,7 @@ // operators (`{ name: {} }`), at any depth — see isEmptyFieldConstraint. // [#19888] And a list in the equality slot (`{ stage: ['won', 'lost'] }`, // `{ stage: { $eq: [...] } }`), through the published door's own gate. +// [#20010] Which now carries every arm of the shared comparand-shape face. import { calendarPartsInTzOrUtc, @@ -39,8 +40,9 @@ import { explicitDateRangeWindow } from './date-range-array-arm.js'; // invent a second spelling of it. A draft-preview filter is a `where`-door // refusal in every respect that matters: the caller authored the predicate and // the repair is theirs. [#19888] So is the equality-slot list gate, for the -// same reason: one rule, one spelling, on both faces. -import { assertNoListInEqualitySlot, invalidFilterError } from './strategies/filter-normalizer.js'; +// same reason: one rule, one spelling, on both faces. [#20010] And, through +// the same gate, every other arm of the shared comparand-shape face. +import { assertWhereComparandShapes, invalidFilterError } from './strategies/filter-normalizer.js'; import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts'; import { emptyGroupValueFor, type Cube } from '@objectstack/spec/data'; @@ -636,7 +638,12 @@ export function evaluateAnalyticsQueryOverRows( // `where` door runs: {@link matchesWhere} would otherwise compare each row // against the list's STRING form (`'won,lost'`) and chart that, for a filter // publish refuses `INVALID_FILTER` / 400 (ruling 乙, #19757). - assertNoListInEqualitySlot(query.where); + // [#20010] Then the shared face's other arms, through the same gate: a null + // `$in` member, a null ordering comparand, a null or blank `$between` bound, + // a scalar `$in` / `$nin`. Measured before, {@link matchesWhere} answered + // them (`{ amt: { $lt: null } }` charted every non-NULL row) for filters + // publish now refuses. + assertWhereComparandShapes(query.where); assertPreviewCanEvaluate(query.where); let filtered = rows.filter((r) => matchesWhere(r, query.where)); const timeDims = query.timeDimensions ?? []; diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index f99cba1d42c..37fe0eda40e 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -196,11 +196,13 @@ * non-NULL row — because `stringifyForCube(null)` was `''`. It is now * `LIKE '%null%'`, which is what `driver-sql` has always compiled it to. * - `{amount: {$gt: null}}` compiled to `amount > ''`, a real comparison - * against the empty string. It now binds NULL, so the predicate is UNKNOWN - * and the widget draws nothing — the honest answer for an unordered - * comparand, and the one `driver-memory` / `formula` give. (#5332 named this + * against the empty string. It then bound NULL, so the predicate was UNKNOWN + * and the widget drew nothing — the honest answer for an unordered + * comparand, and the one `driver-memory` / `formula` gave. (#5332 named this * comparand position as covered by no ruling and left the `''` placeholder - * alone; deleting the encoder decides it by construction.) + * alone; deleting the encoder decided it by construction.) [#20010] The + * position has a ruling now (2026-09-01, #14080) and it is a refusal, at the + * shared comparand-shape face this door runs — see the #20010 section below. * * # A `where` ARRAY is lowered here, not dropped (#5334) * @@ -346,6 +348,23 @@ * `{ f: { $in: [...] } }` for "one of these values". `$ne` with a list is not * this ruling's. * + * # The same face's OTHER arms reach the object spelling too (#20010) + * + * The same face carries the null and blank carve-outs ruled onto it — a `null` + * `$in` / `$nin` member and a `null` `$between` endpoint (2026-08-31), a `null` + * ordering comparand (2026-09-01), a blank `$between` endpoint (2026-09-20) — + * and its original rule, that `$in` / `$nin` / `$between` receive a list at + * all. The `FilterArray` spelling met every one of them inside + * `parseFilterAST`; the object spelling met none, so `{ s: { $in: ['won', null] } }` + * compiled to `s IN ('won', NULL)` and `{ s: { $in: 'won' } }` was laundered to + * a one-member list, while the array spelling of each was refused + * `INVALID_FILTER` / 400. {@link assertWhereComparandShapes} now hands every + * field entry to the face after the #19888 equality pass, so both spellings + * get the face's own refusal byte for byte, on every face of this door and on + * the draft preview. The comparand-TYPE face is not run here; `undefined` + * keeps {@link assertDefinedComparands}' refusal except as a `$between` + * endpoint, which the face now answers. + * * Row-result cover: `filter-operator-coverage.test.ts` for the operator * vocabulary, `native-sql-filter-logic-conformance.test.ts`, which runs the * SHARED combinator table (`FILTER_LOGIC_CASES`, #3774) that the SQL compiler, @@ -360,7 +379,9 @@ * `filter-normalizer-mixed-wrapper.test.ts` for the mixed `$`/non-`$` wrapper * refusal and its pure-shape control groups (#6444), and * `where-equality-slot-list-refusal.test.ts` for the equality-slot list refusal - * on every analytics face and its neighbouring shapes (#19888). + * on every analytics face and its neighbouring shapes (#19888), and + * `where-face-arms-refusal.test.ts` for the face's other arms, both spellings, + * every face (#20010). */ import { @@ -674,6 +695,13 @@ function assertCompilableComparand(opKey: string, field: string, value: unknown) * Asserted under the `$between` name, not under the `gte` / `lte` the bounds * lower to, because the author wrote `$between` and that is the key they have * to repair. + * + * [#20010] The `where` door now hands every field entry to the shared + * comparand-shape face before any leaf is built, and the face refuses this + * endpoint by the 2026-08-11 ruling (#7596), which the face now enforces — so the + * door answers in the face's words, the ones the `FilterArray` spelling always + * got, and this check is {@link fieldLeaves}' own invariant rather than the + * door's refusal. The laundering argument above is why the invariant stays. */ function assertNoFieldReferenceComparand(opKey: string, field: string, value: unknown): void { if (opKey !== '$between' || !Array.isArray(value)) return; @@ -758,7 +786,11 @@ function undefinedComparandError(field: string, path: string): Error { * - an OPERATOR's comparand — `{d: {$gt: undefined}}`, `$eq`, `$ne`, the LIKE * family, every other single-value operator; * - a MEMBER of a list operator's array — `{d: {$in: [undefined]}}`, `$nin`, - * and `$between`'s two bounds. + * and `$between`'s two bounds. [#20010] A `$between` bound is answered + * first by the shared comparand-shape face, whose 2026-09-20 ruling + * refuses an `undefined` endpoint as BLANK, naming the side; the door runs + * that face before any leaf is built, so this gate's sentence is reached + * only in the other positions. * * `$null` / `$exists` are deliberately NOT swept, exactly as on the twin: their * comparand is a declared BOOLEAN — a flag, not a value to compare against — so @@ -970,6 +1002,10 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { // (`native-sql-temporal-conformance.test.ts`). if (opKey === '$between') { const v = wrapper[opKey]; + // [#20010] From `lowerAnalyticsWhere`, the shared comparand-shape face + // has already judged this range — arity, a null or blank endpoint, a + // `{ $field }` endpoint — in its own words, so the two checks below + // are this function's invariants and are not reached from the door. if (!Array.isArray(v) || v.length !== 2) { // Never drop it: an unbounded read is the failure mode this whole // branch exists to prevent, and it is indistinguishable from a @@ -1507,7 +1543,55 @@ function filterArrayNotLowerableError(where: unknown[]): Error { ); } -// ── [#19888] A LIST in the equality slot ───────────────────────────────────── +// ── [#19888 / #20010] The shared comparand-shape face, on the object spelling ── + +/** + * [#20010] Visit every FIELD ENTRY of an object-form `where` — `{ key: spec }` + * with the `path` of the node that holds it — the way the shared + * comparand-shape face walks a condition, plus one step the face does not take. + * + * The face's traversal: `$and` / `$or` arrays, `$not`, and field entries; any + * other `$` key at node level is a logical operator the face does not judge + * (an unknown one is refused by {@link buildNode} by name), and a non-array + * `$and` / `$or` operand is {@link buildNode}'s to refuse with its own message. + * + * The extra step: a NESTED-RELATION object (`{ acct: { region: … } }`, no `$` + * key) is descended, where the face leaves one alone because a driver reads it + * as a deep-equality comparand or another object's condition. This compiler + * reads it as neither: {@link fieldLeaves} flattens it to the dotted member + * `acct.region`, so its entries are comparisons in their own right. The entry + * is visited as `{ region: … }` at path `where.acct`, which is how the #19888 + * gate has named that position since it landed. + * + * One traversal for both passes of {@link assertWhereComparandShapes}, so the + * two cannot disagree about which positions a `where` has. + */ +function forEachWhereFieldEntry( + node: unknown, + path: string, + visit: (key: string, spec: unknown, path: string) => void, +): void { + if (!isFilterObject(node)) return; + for (const [key, spec] of Object.entries(node)) { + const here = `${path}.${key}`; + if (key === '$and' || key === '$or') { + if (Array.isArray(spec)) { + spec.forEach((child, index) => forEachWhereFieldEntry(child, `${here}[${index}]`, visit)); + } + continue; + } + if (key === '$not') { + forEachWhereFieldEntry(spec, here, visit); + continue; + } + if (key.startsWith('$')) continue; + if (isFilterObject(spec) && !Object.keys(spec).some((k) => k.startsWith('$'))) { + forEachWhereFieldEntry(spec, here, visit); + continue; + } + visit(key, spec, path); + } +} /** * [#19888] Refuse every LIST in the EQUALITY slot of an object-form `where` — @@ -1555,42 +1639,107 @@ function filterArrayNotLowerableError(where: unknown[]): Error { * * It runs before every gate {@link buildNode} reaches, so a list is diagnosed * as the list and not by one of its members (`{ f: [1, undefined] }`), the - * order the face and `read-scope-sql.ts`'s twin use. + * order the face and `read-scope-sql.ts`'s twin use. [#20010] It also runs + * before the face's OTHER arms, as the first pass of + * {@link assertWhereComparandShapes}: a `where` carrying both an equality-slot + * list and another refused shape is answered with the list. * * ⛔ `$ne` is not judged: the ruling names equality, and `$ne` with a list is * #19886's ruling A, carried on that card. The list operators keep their * lists, `$in: []` / `$nin: []` included, and every scalar — `null` above all — * passes exactly as before. * + * [#20010] Module-private since the draft-data preview calls + * {@link assertWhereComparandShapes}, which runs this first. + */ +function assertNoListInEqualitySlot(node: unknown, path = 'where'): void { + forEachWhereFieldEntry(node, path, (key, spec, at) => { + if (Array.isArray(spec)) { + assertListComparandShapes({ [key]: spec }, undefined, at); + } else if (isFilterObject(spec) && Array.isArray(spec.$eq)) { + assertListComparandShapes({ [key]: { $eq: spec.$eq } }, undefined, at); + } + }); +} + +/** + * [#20010] Hand every field entry of an object-form `where` to the shared + * comparand-shape face — ALL of its arms, not only the equality one — before + * any node is built. + * + * ## Why + * + * `assertListComparandShapes` (`@objectstack/spec/data`) declares itself "the + * one place that decides whether `$in` / `$nin` / `$between` received a list at + * all, for every driver", and carries the rulings that carved the null and + * blank positions onto that same door. Quoted from its docblock: + * + * - 2026-08-31 (#13357): "A `null` member of `$in` / `$nin`, and a `null` + * `$between` endpoint (#13495's shape), are refused at this door"; + * - 2026-09-01 (#14080): "A `null` comparand of `$gt` / `$gte` / `$lt` / + * `$lte` … refused at this door, same envelope, so the divergent cells are + * constructively unreachable"; + * - 2026-09-20 (#19071): "`''` and `undefined` are refused, naming the blank + * side (MIN / MAX and the index)" — the runtime twin of the schema door's + * non-blank endpoint rule. + * + * This door met that face for the `FilterArray` spelling only, inside + * `parseFilterAST`. {@link assertNoListInEqualitySlot} (#19888) carried the + * equality arm to the object spelling and, by design, none of the others. So + * one condition got two answers on one door, measured over a real engine + * before this gate: + * + * | object `where` | before | + * |---|---| + * | `{ s: { $in: ['won', null] } }` | `s IN ('won', NULL)`, served; the draft preview also served the NULL row | + * | `{ a: { $lt: null } }` | `a < NULL`: no row; the draft preview served every non-NULL row | + * | `{ a: { $between: [null, 5] } }` | `a >= NULL AND a <= 5`; the ENGINE path received `$gte: null` and refused it under an operator the author never wrote | + * | `{ a: { $between: ['', 5] } }` | `a >= '' AND a <= 5`, and the engine path ACCEPTED it | + * | `{ s: { $in: 'won' } }` | laundered to `s IN ('won')`: served natively, and ACCEPTED by the engine path as a list | + * + * while the `FilterArray` spelling of each row was refused `INVALID_FILTER` / + * 400 by the face. Every row now answers like the `FilterArray` spelling, with + * the face's own envelope, wording, path and prescription. + * + * ## How + * + * Two passes over {@link forEachWhereFieldEntry}'s one traversal. The first is + * {@link assertNoListInEqualitySlot}, unchanged, so the equality arm keeps the + * precedence #19888 gave it. The second hands each whole entry to the face as a + * one-entry node with the same path seed, `where`: the face then reports + * exactly the path and field it reports on the whole condition, so the object + * spelling gets the `FilterArray` spelling's refusal byte for byte. A nested + * relation's entries are handed over too, since this compiler flattens them to + * the dotted member. + * + * Refusals this door already gave in its own words now give the face's, the + * same words the `FilterArray` spelling gets: a `$between` that is not a + * two-element list, a `{ $field }` endpoint, an `undefined` endpoint. The + * local checks for those in {@link fieldLeaves} stay as that function's own + * invariants and are no longer reached from this door. + * + * ⛔ Not moved: + * + * - `$ne` with a list. The face does not judge it yet; #19886's stage 2 puts + * that refusal on the face, and this gate carries it the day it does, with no + * change here. + * - The comparand-TYPE face (`normalizeFilterComparandTypes`). An `undefined` + * comparand outside a `$between` endpoint is still refused by + * {@link assertDefinedComparands}, in this door's own words. + * - `$in: []` / `$nin: []`, every scalar ordering comparand, a `{ $field }` in + * an ordering slot, and `null` in the equality slot (`{ f: null }`, + * `$eq: null`, `$ne: null`, the null predicate) all compile as before. + * * EXPORTED for the one other face in this package that evaluates a `where` * without this door: the draft-data preview (`preview-evaluator.ts`), which * calls it so a drafted chart refuses what the published one refuses, rather * than spelling the rule a second time. */ -export function assertNoListInEqualitySlot(node: unknown, path = 'where'): void { - if (!isFilterObject(node)) return; - for (const [key, spec] of Object.entries(node)) { - const here = `${path}.${key}`; - if (key === '$and' || key === '$or') { - // A non-array operand is refused by `buildNode` with its own message. - if (Array.isArray(spec)) { - spec.forEach((child, index) => assertNoListInEqualitySlot(child, `${here}[${index}]`)); - } - continue; - } - if (key === '$not') { - assertNoListInEqualitySlot(spec, here); - continue; - } - if (key.startsWith('$')) continue; - if (Array.isArray(spec)) assertListComparandShapes({ [key]: spec }, undefined, path); - if (!isFilterObject(spec)) continue; - if (!Object.keys(spec).some((k) => k.startsWith('$'))) { - assertNoListInEqualitySlot(spec, here); - continue; - } - if (Array.isArray(spec.$eq)) assertListComparandShapes({ [key]: { $eq: spec.$eq } }, undefined, path); - } +export function assertWhereComparandShapes(node: unknown, path = 'where'): void { + assertNoListInEqualitySlot(node, path); + forEachWhereFieldEntry(node, path, (key, spec, at) => { + assertListComparandShapes({ [key]: spec }, undefined, at); + }); } /** @@ -1638,9 +1787,10 @@ export function lowerAnalyticsWhere( return condition as Record; } - // [#19888] The object spelling meets the shared face's equality arm here, the - // way the array spelling met it inside `parseFilterAST` just above. - assertNoListInEqualitySlot(where); + // [#19888, #20010] The object spelling meets the shared comparand-shape face + // here — the equality arm first, then every other arm — the way the array + // spelling met it inside `parseFilterAST` just above. + assertWhereComparandShapes(where); return where as Record; } @@ -1780,6 +1930,8 @@ export function collectFilterLeaves( * the honest answer for an unordered comparand and the one the JS backends give; * the `''` this used to bind was a real comparison against the empty string, * which on a text column silently matched rows (see the module header). + * [#20010] From the `where` door, a `null` no longer reaches an ordering slot + * at all: the shared comparand-shape face refuses it first (2026-09-01 ruling). */ export function toSqlBindValue(v: unknown): unknown { if (typeof v === 'boolean') return v ? 1 : 0;