diff --git a/.changeset/lucky-pandas-repeat.md b/.changeset/lucky-pandas-repeat.md new file mode 100644 index 0000000000..4a68cc61d4 --- /dev/null +++ b/.changeset/lucky-pandas-repeat.md @@ -0,0 +1,50 @@ +--- +'@objectstack/service-analytics': patch +--- + +fix(analytics): an `undefined` comparand in an analytics `where` is refused (400 `INVALID_FILTER`), not read seven different ways + +**Observable behaviour change.** A `where` key whose value is `undefined` used to +compile — in seven different ways, depending on where it sat. It is now refused +with `INVALID_FILTER` / 400, the envelope every other refusal at this door +already carries. + +The three that mattered WIDENED the query, which is the failure mode +`filter-normalizer.ts` forbids in its own body ("NEVER drop: a missing predicate +does not narrow the query, it WIDENS it"), while its entry line did exactly that: + +| `where` | used to normalize to | reading | +|---|---|---| +| `{d: undefined}` | `null` | the WHOLE filter dropped — the query ran **unfiltered** | +| `{stage: 'won', d: undefined}` | `stage equals 'won'` | the `d` conjunct vanished in silence | +| `{$not: {d: undefined}}` | `NOT (d set)` | `d IS NULL` — a predicate the author never wrote | +| `{d: {$eq: undefined}}` | `d equals [null]` | a value comparison, **not** `$eq: null`'s null predicate | +| `{d: {$gt: undefined}}` | `d gt [null]` | ditto | +| `{d: {$in: [undefined]}}` | `d in [null]` | ditto | +| `{d: {$ne: undefined}}` | `d notSet OR d notEquals [null]` | ditto | + +The direction is silently **wrong results** — an analytics figure, a report +total, an aggregate, wrong with nothing to read — **not** a permission bypass: +read scope is compiled by a different door (`read-scope-sql.ts`) and never passed +through here, so a caller still saw only rows it was entitled to, just more of +them than it asked for. + +**What to change if this refuses your filter.** `undefined` cannot cross JSON, so +neither REST door can carry it — this only reaches in-process callers of +`AnalyticsService.query({ where })` that spread a possibly-absent value into the +filter object (`{ owner_id: ctx.user?.id }`). Two repairs, both stated by the +error message: + +- meant the null predicate → write `{ field: null }` or `{ field: { $null: true } }`; +- the value is genuinely absent → **omit the key**, which is the same "no + constraint" without the ambiguity. + +Inside stored metadata, the platform's own answer to "scope this to the current +user" is unaffected and was already fail-closed: a `{current_user_id}` +placeholder resolves through `resolveFilterTokens`, which raises +`FILTER_TOKEN_UNRESOLVED` / 400 rather than emitting `undefined`. + +⛔ **`null` does not move.** `{d: null}`, `{$eq: null}`, `{$ne: null}`, +`{$null: …}`, `{$exists: …}` and `$contains: null` keep their exact lowering — +`null` is a declared comparand and is the null predicate. `$null` / `$exists` +carry a declared boolean flag rather than a comparand and are likewise untouched. diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts index 4149e12a8d..ff1a1a30f3 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts @@ -722,9 +722,34 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit expect(sql).toContain('"account"."region" = $1'); }); - it('an undefined value is still skipped, as it always was', async () => { - expect(await ids({ stage: undefined, owner: 'u1' })).toEqual(['1', '3']); - expect(await ids({ $not: undefined, owner: 'u1' })).toEqual(['1', '3']); + it('an undefined value is REFUSED at this seam (#6386), not skipped', async () => { + // ⚠️ REPLACED, not re-spelled. This case read "an undefined value is still + // skipped, as it always was" and asserted `ids({stage: undefined, owner: + // 'u1'}) === ['1','3']` — i.e. it PINNED the skip as a guarantee. #6386 + // measured what the skip actually bought: `{stage: undefined}` alone + // normalised to `null`, so a single-key `where` ran with NO filter and the + // chart was drawn over every row — the #3650 widening this file's own + // subject (`$not`) exists to prevent, arriving through the entry gate + // instead. The two-key spelling above hid that: `owner: 'u1'` survived, so + // the row set still looked filtered. + // + // Kept at THIS seam deliberately — `ids` executes end to end through + // `NativeSQLStrategy`, which is where a compiled-to-nothing filter turns + // into a statement with no `WHERE` (#5297's lesson, one call above the + // compiler). So this asserts the refusal reaches the executing seam, not + // merely that the normalizer throws in isolation. + await expect(ids({ stage: undefined, owner: 'u1' })).rejects.toThrowError( + /comparand at "stage" is undefined/, + ); + // `$not: undefined` is NOT the same condition and must not borrow the same + // message: it is a combinator with a missing operand, and the branch that + // already owned that shape gives the truer diagnosis. + await expect(ids({ $not: undefined, owner: 'u1' })).rejects.toThrowError( + /"\$not" requires a filter object/, + ); + // The row set the old assertion recorded is still reachable — by writing + // the filter the author meant, with the unknowable key simply omitted. + expect(await ids({ owner: 'u1' })).toEqual(['1', '3']); }); it('an ordinary filter compiles to exactly the SQL it always did', async () => { 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 new file mode 100644 index 0000000000..ef3ce6d5e0 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-undefined-comparand.test.ts @@ -0,0 +1,504 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6386, on #6050's ruling B] `undefined` in a comparand position of the + * analytics `where` is REFUSED — `INVALID_FILTER` / 400 — and `null` is not. + * + * ## What was wrong + * + * #6050 ruled (2026-08-07, ruling B) that `undefined` where a comparand belongs + * is refused, and landed it on `driver-sql` / `driver-turso`. #6125 pushed it to + * this package's OTHER door, `read-scope-sql.ts` (PR #6390). This door — the + * `where` the CALLER writes — had never been named by any of those rulings, and + * it read the one shape SEVEN ways. Measured on `origin/main` (`5faa23ca3`) by + * calling `normalizeAnalyticsFilterTree({ where })` directly: + * + * | `where` | normalized to | reading | + * |---|---|---| + * | `{d: undefined}` | `null` | the WHOLE where dropped — query ran with NO filter | + * | `{stage:'won', d: undefined}` | `stage equals 'won'` | the `d` conjunct vanished in silence | + * | `{$not: {d: undefined}}` | `NOT (d set)`, i.e. `d IS NULL` | a predicate the author never wrote | + * | `{d: {$eq: undefined}}` | `d equals [null]` | a value comparison, NOT `$eq: null`'s `notSet` | + * | `{d: {$gt: undefined}}` | `d gt [null]` | ditto | + * | `{d: {$in: [undefined]}}` | `d in [null]` | ditto | + * | `{d: {$ne: undefined}}` | `d notSet OR d notEquals [null]` | ditto | + * + * The first three WIDEN, which is the failure mode `filter-normalizer.ts` has + * forbidden inside `fieldLeaves` since #4128 — "NEVER drop: a missing predicate + * does not narrow the query, it WIDENS it … That failure mode is #3650's" — + * while its own entry line, `if (raw === undefined) continue;`, did exactly that. + * + * Row three is not merely "one conjunct fewer": #5146's null-safe rewrite splits + * the leaf into `{d: {$null: false}} AND {d: undefined}`, the entry gate dropped + * the second half, and the surviving guard was then negated — so a predicate GREW + * OUT of a discarded leaf. + * + * ⚠️ The direction is silently WRONG RESULTS, not a permission bypass. Read scope + * is compiled by the other door (`read-scope-sql.ts` → `applyReadScope`) and never + * passes through here, so a caller still saw only rows it was entitled to — just + * more of them than it asked for. What was lost is the ANSWER: an analytics + * figure, a report total, an aggregate, wrong with nothing to read. + * + * ## The four blocks, and which one is the change + * + * `the seven measured readings` is the change: run it against pre-#6386 code and + * all seven fail, because all seven COMPILE. + * + * `the null control group` is the risk. `null` is a declared comparand with + * settled semantics (#5332 for the predicate, #5526 for the type), and the way + * this change could do harm is by refusing it alongside `undefined` — the two + * live one `===` apart in every polarity table in the module. Every row passes + * both before and after, tree for tree. + * + * `the #5146 rewrite cannot swallow the leaf` is the gate-SIDE question. The gate + * sits in `fieldLeaves`, downstream of `nullSafeNegationOperand`, so whether row + * three throws or merely changes shape depends on the rewrite carrying the + * author's spec through. Measured, not assumed — PR #6390 hit the same trap on + * the sibling door, and its reasoning does not transfer (that module's polarity + * tables are uniformly `=== null`; this one mixes `=== null` for `$eq`/`$ne` + * with IDENTITY reads for `$null`/`$exists`). + * + * `what the sweep deliberately leaves alone` records the boundary of the ruling. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Ordinary direction, and with TWO independent knobs because the fix has two + * halves that each cover a different subset: + * + * - **A — restore `if (raw === undefined) continue;` only.** Predicted: the + * three FIELD-position rows go red (they compile again) plus the combinator + * rows, while the four OPERATOR/list rows stay GREEN — the `fieldLeaves` + * gate still catches those. + * - **B — remove the `assertDefinedComparands` call only.** Predicted: every + * refusal row goes red. ⚠️ B alone does NOT restore `origin/main`: with the + * entry line still deleted, `{d: undefined}` compiles to `equals [null]` + * rather than dropping to `null`. Only A+B together reproduce the old + * readings, which is the proof the two halves are independent. + * + * Both measured; the counts are in the PR body. The `null` control group stays + * green under every configuration — it never depended on either half. + * + * ## Scope, so a later reader does not "finish the job" + * + * ⛔ `comparand()`'s `undefined` → `null` normalisation (#5526) and the strict + * `wrapper[opKey] === null` branch that decides null-predicate-vs-value-comparison + * (#5332) are RULED and are not touched. The gate refuses an INPUT; it reopens + * neither. The normalisation is now unreachable from this door, and retiring it + * is #5526's decision to make on its own terms, not a cleanup rider here. + * ⛔ `@objectstack/formula` reads the same `undefined` as a THIRD semantics — "the + * key is absent from the record" — the open question in #5299, untouched. + * ⛔ `read-scope-sql.ts` is the sibling door and is not touched here; its own + * refusal landed in PR #6390 with a different envelope on purpose (500: it + * compiles a platform artifact, this door receives caller input). + */ + +import { describe, it, expect } from 'vitest'; +import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js'; + +/** The ADR-0112 fields a refusal must carry. */ +interface FilterRefusal extends Error { + code?: unknown; + status?: unknown; +} + +function refusalFor(where: unknown): FilterRefusal | undefined { + try { + normalizeAnalyticsFilterTree({ where }); + return undefined; + } catch (e) { + return e as FilterRefusal; + } +} + +function treeFor(where: unknown): unknown { + return normalizeAnalyticsFilterTree({ where }); +} + +/** + * The seven readings #6386 measured, one row each, with the `path` the refusal + * must name. `wasReadAs` is the pre-fix normalisation — recorded so the row says + * what it is protecting against, not merely that something throws. + */ +const MEASURED: Array<{ name: string; where: unknown; path: string; wasReadAs: string }> = [ + { + name: '① a single-key where — the whole filter disappeared', + where: { d: undefined }, + path: '"d"', + wasReadAs: 'null — no predicate at all, so the query ran UNFILTERED', + }, + { + name: '② one conjunct of several — that conjunct disappeared', + where: { stage: 'won', d: undefined }, + path: '"d"', + wasReadAs: "only `stage equals 'won'` survived", + }, + { + name: '③ inside a $not — a predicate GREW from the discarded leaf', + where: { $not: { d: undefined } }, + path: '"d"', + wasReadAs: 'NOT (d set) — i.e. `d IS NULL`, which the author never wrote', + }, + { + name: '④ $eq — a value comparison, not $eq: null’s null predicate', + where: { d: { $eq: undefined } }, + path: '"d".$eq', + wasReadAs: 'd equals [null]', + }, + { + name: '⑤ $gt — an ordering comparison against NULL, UNKNOWN for every row', + where: { d: { $gt: undefined } }, + path: '"d".$gt', + wasReadAs: 'd gt [null]', + }, + { + name: '⑥ a member of a $in list', + where: { d: { $in: [undefined] } }, + path: '"d".$in[0]', + wasReadAs: 'd in [null]', + }, + { + name: '⑦ $ne — the null-safe guard wrapped a comparison against NULL', + where: { d: { $ne: undefined } }, + path: '"d".$ne', + wasReadAs: 'd notSet OR d notEquals [null]', + }, +]; + +/** + * Comparand positions beyond the issue's seven, measured in the same round. + * + * Two of them are where this gate deliberately diverges from `read-scope-sql`'s + * twin, and in both cases because THIS module accepts the enclosing shape that + * one refuses outright — so a "comparand is undefined" answer is the truest + * thing to say here and would have been a mislabel there. + */ +const BEYOND_THE_TABLE: Array<{ name: string; where: unknown; path: string; wasReadAs: string }> = [ + { + name: 'a member of the BARE-ARRAY implicit $in (twin refuses the array itself)', + where: { d: [1, undefined] }, + path: '"d"[1]', + wasReadAs: 'd in [1, null]', + }, + { + 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] } }, + path: '"d".$nin[0]', + wasReadAs: 'd notSet OR d notIn [null]', + }, + { + name: 'the LIKE family, whose comparand the spec declares a string', + where: { d: { $contains: undefined } }, + path: '"d".$contains', + wasReadAs: "d contains [null] — LIKE '%null%' since #5526", + }, + { + name: 'a NESTED relation, refused on the DOTTED member (twin refuses nesting itself)', + where: { profile: { verified: undefined } }, + path: '"profile.verified"', + wasReadAs: 'profile.verified equals [null]', + }, + { + name: 'a branch of a $and', + where: { $and: [{ d: undefined }] }, + path: '"d"', + wasReadAs: 'null — the branch reduced to TRUE and the $and to nothing', + }, + { + name: 'ONE branch of a $or — TRUE absorbed the whole disjunction (#5325)', + where: { $or: [{ d: undefined }, { stage: 1 }] }, + path: '"d"', + wasReadAs: 'null — the surviving `stage` branch was absorbed too, so EVERY row', + }, +]; + +/** + * `null` — the comparand that must NOT move. One row per position the sweep + * touches, each with the tree it compiled to before this change and must still. + */ +const NULL_CONTROL: Array<{ name: string; where: unknown; tree: unknown }> = [ + { + name: '{d: null} → the null predicate (#5332)', + where: { d: null }, + tree: { kind: 'leaf', member: 'd', operator: 'notSet', values: [] }, + }, + { + name: '{$eq: null} → notSet, the SAME predicate as {d: null} (#5332)', + where: { d: { $eq: null } }, + tree: { kind: 'leaf', member: 'd', operator: 'notSet', values: [] }, + }, + { + name: '{$ne: null} → set, and it stays total (no null guard)', + 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] }, + ], + }, + }, + { + name: "{$contains: null} → LIKE '%null%' (#5526)", + where: { d: { $contains: null } }, + tree: { kind: 'leaf', member: 'd', operator: 'contains', values: [null] }, + }, + { + name: '{$null: true} → notSet', + where: { d: { $null: true } }, + tree: { kind: 'leaf', member: 'd', operator: 'notSet', values: [] }, + }, + { + name: '{$exists: false} → notSet', + where: { d: { $exists: false } }, + tree: { kind: 'leaf', member: 'd', operator: 'notSet', values: [] }, + }, + { + name: '{$not: {d: null}} → NOT(notSet), no guard (already total)', + where: { $not: { d: null } }, + tree: { kind: 'not', child: { kind: 'leaf', member: 'd', operator: 'notSet', values: [] } }, + }, + { + name: '{$not: {d: {$ne: null}}} → NOT(set), the #5332 arm of the guard table', + where: { $not: { d: { $ne: null } } }, + tree: { kind: 'not', child: { kind: 'leaf', member: 'd', operator: 'set', values: [] } }, + }, + { + name: '{d: [1, null]} → a bare-array $in carrying null', + where: { d: [1, null] }, + tree: { kind: 'leaf', member: 'd', operator: 'in', values: [1, null] }, + }, +]; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#6386] the seven measured readings are now ONE refusal', () => { + for (const c of [...MEASURED, ...BEYOND_THE_TABLE]) { + it(`refuses ${c.name} (was: ${c.wasReadAs})`, () => { + const err = refusalFor(c.where); + expect(err, 'compiled instead of refusing — the #3650 widening is back').toBeInstanceOf(Error); + expect(String(err?.message)).toContain(`comparand at ${c.path} is undefined`); + // The envelope every refusal in this module carries since #5352, and the + // one #6050 chose for this shape: caller input, so 400. + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + }); + } + + it('says ONE thing, differing only in `path` and the field it repairs (#5240)', () => { + // Erase the two things that legitimately vary — the position and the field + // name the repair hints quote — and every message must be the same string. + // Two variables rather than one: unlike `read-scope-sql`'s twin, this + // wording PRESCRIBES (`{ "d": null }`), so the field appears outside the + // path as well. + const generic = [...MEASURED, ...BEYOND_THE_TABLE].map((c) => { + const field = c.path.replace(/^"/, '').replace(/".*$/, ''); + const err = refusalFor(c.where); + // ⚠️ Load-bearing, and measured: without it this case is VACUOUSLY green + // whenever nothing throws — every row maps to the same `"undefined"` + // string, the set has one member, and a set-size assertion reads that as + // "one wording". Removing the gate turned all 14 rows silent and this + // case stayed green until the guard was added. + expect(err, `${c.name} did not refuse — the wording check would pass on nothing`).toBeInstanceOf(Error); + return String(err?.message) + .replace(`comparand at ${c.path} is`, 'comparand at is') + // `split`/`join` rather than `replaceAll` — this package's tsconfig `lib` + // predates ES2021, and a new tsc error here would widen a shrink-only + // ledger (`check:type-check-debt`). + .split(`"${field}"`).join('""'); + }); + expect(new Set(generic).size, `expected one wording, got:\n${[...new Set(generic)].join('\n\n')}`).toBe(1); + }); + + it('names the two repairs and the producer to fix, not just the refusal', () => { + const message = String(refusalFor({ d: undefined })?.message); + // The author's two legitimate spellings… + expect(message).toContain('{ "d": null }'); + expect(message).toContain('{ "d": { "$null": true } }'); + expect(message).toContain('omit the key entirely'); + // …both consequences it used to have, since this door has two… + expect(message).toContain('the key was dropped outright'); + expect(message).toContain('became a comparison against null'); + // …and where the fix belongs. Unlike the sibling read-scope door, the + // producer here IS the caller, so the message must not send them to an + // admin-authored policy. + expect(message).toContain('The producer to fix is whoever BUILT this where'); + expect(message).toContain('undefined cannot cross JSON'); + }); +}); + +describe('[#6386] the `null` control group does not move', () => { + for (const c of NULL_CONTROL) { + it(`unchanged: ${c.name}`, () => { + expect(refusalFor(c.where), 'null was refused alongside undefined — the harm direction').toBeUndefined(); + expect(treeFor(c.where)).toEqual(c.tree); + }); + } + + it('distinguishes null from undefined in the SAME position, both operators', () => { + // The whole risk in one assertion: the two are one `===` apart in every + // polarity table in the module, so a `== null` anywhere would collapse them. + expect(treeFor({ d: { $eq: null } })).toEqual({ kind: 'leaf', member: 'd', operator: 'notSet', values: [] }); + expect(refusalFor({ d: { $eq: undefined } })?.code).toBe('INVALID_FILTER'); + expect(treeFor({ d: { $ne: null } })).toEqual({ kind: 'leaf', member: 'd', operator: 'set', values: [] }); + expect(refusalFor({ d: { $ne: undefined } })?.code).toBe('INVALID_FILTER'); + }); + + it('an ABSENT `where` is still "no constraint", not a refusal', () => { + // `lowerAnalyticsWhere` answers `null` for these before `buildNode` runs. + // The refusal is about a KEY INSIDE a `where`; a missing `where` is the + // legitimate way to say "no filter" and must stay silent. + expect(treeFor(undefined)).toBeNull(); + expect(normalizeAnalyticsFilterTree({})).toBeNull(); + expect(treeFor({})).toBeNull(); + expect(treeFor([])).toBeNull(); + }); +}); + +describe('[#6386] the #5146 rewrite cannot swallow the leaf — the gate side is load-bearing', () => { + // The gate lives in `fieldLeaves`, DOWNSTREAM of `nullSafeNegationOperand`, so + // `{$not: {…}}` throws only if the rewrite carries the author's spec through. + // One case per rewrite path that can carry a swept comparand. + const REWRITE_PATHS: Array<{ name: string; where: unknown; path: string }> = [ + { + name: "`requireValue` — pushes {k: {$null: false}}, {k: spec}; spec kept by reference", + where: { $not: { d: undefined } }, + path: '"d"', + }, + { + name: '`requireValue` via an operator spec', + where: { $not: { d: { $eq: undefined } } }, + path: '"d".$eq', + }, + { + name: '`allowNull` — pushes {$or: [{k: {$null: true}}, {k: spec}]} ($ne’s polarity)', + where: { $not: { d: { $ne: undefined } } }, + path: '"d".$ne', + }, + { + name: 'the nested-relation recursion, which guards the DOTTED member', + where: { $not: { profile: { verified: undefined } } }, + path: '"profile.verified"', + }, + { + name: 'a list member inside a negation', + where: { $not: { d: { $in: [undefined] } } }, + path: '"d".$in[0]', + }, + ]; + + for (const c of REWRITE_PATHS) { + it(`throws rather than changing shape: ${c.name}`, () => { + const err = refusalFor(c.where); + expect(err, 'the rewrite swallowed the leaf and the gate blessed the new shape').toBeInstanceOf(Error); + expect(String(err?.message)).toContain(`comparand at ${c.path} is undefined`); + }); + } + + it('there is no `none`-disposition case to write, and this is why', () => { + // `nullGuardForFieldSpec` answers 'none' only when EVERY operator satisfies + // `operatorIsNullTotal`, which for an `undefined` comparand is false on every + // operator this gate sweeps ($eq/$ne compare `value === null`; $in/$nin need + // an empty array). So the only field specs reaching 'none' while holding an + // `undefined` are the `$null` / `$exists` flags — deliberately not swept, and + // asserted below to compile exactly as before. + expect(refusalFor({ $not: { d: { $null: undefined } } })).toBeUndefined(); + expect(treeFor({ $not: { d: { $null: undefined } } })).toEqual({ + kind: 'not', + child: { kind: 'leaf', member: 'd', operator: 'set', values: [] }, + }); + }); +}); + +describe('[#6386] what the sweep deliberately leaves alone', () => { + it('$null / $exists carry a declared BOOLEAN flag, not a comparand', () => { + // Same call as `read-scope-sql`'s twin. ⚠️ The two modules read the flag + // DIFFERENTLY — identity here, truthiness there — so this module answers + // `{$null: undefined}` with `set` while that one answers `IS NOT NULL` by a + // different route. Which reading is right is the boolean-DOMAIN question + // (#5347 / #5369, measured on the sibling door as #6387); refusing it here as + // an "undefined comparand" would decide it sideways and mislabel a flag. + expect(treeFor({ d: { $null: undefined } })).toEqual({ kind: 'leaf', member: 'd', operator: 'set', values: [] }); + expect(treeFor({ d: { $exists: undefined } })).toEqual({ kind: 'leaf', member: 'd', operator: 'set', values: [] }); + }); + + it('a combinator with an undefined value gets its own, truer refusal', () => { + // Deleting `buildNode`'s entry `continue` did not create four new refusals — + // it let each key kind reach the branch that already had the best thing to + // say. #5240's rule read in the direction that matters: a second wording for + // a shape refused either way only sends the author to the wrong repair. + expect(String(refusalFor({ $and: undefined })?.message)).toContain('"$and" requires an array of filter objects'); + expect(String(refusalFor({ $or: undefined })?.message)).toContain('"$or" requires an array of filter objects'); + expect(String(refusalFor({ $not: undefined })?.message)).toContain('"$not" requires a filter object'); + expect(String(refusalFor({ $nor: undefined })?.message)).toContain('Unsupported top-level filter operator'); + // …all still in the same envelope, so the REST face answers 400 for every one. + for (const where of [{ $and: undefined }, { $or: undefined }, { $not: undefined }, { $nor: undefined }]) { + expect(refusalFor(where)?.code).toBe('INVALID_FILTER'); + expect(refusalFor(where)?.status).toBe(400); + } + }); + + it('a non-$ SIBLING of an operator is still dropped — a different defect, not this one', () => { + // `{d: {$eq: 1, nested: }}` ignores `nested` whatever its value, so + // this is not an `undefined` reading and is out of scope here. Pinned so the + // measurement is on the record rather than mistaken for coverage: filed + // separately per Prime Directive #10. + expect(treeFor({ d: { $eq: 1, nested: undefined } })).toEqual({ + kind: 'leaf', member: 'd', operator: 'equals', values: [1], + }); + expect(treeFor({ d: { $eq: 1, nested: 'x' } })).toEqual({ + kind: 'leaf', member: 'd', operator: 'equals', values: [1], + }); + }); + + it('every ACCEPTED shape the module already had still compiles identically', () => { + // The other half of "the refusal set moved by exactly one condition": a gate + // that over-reached would show up here as a throw. + expect(treeFor({ stage: 'won' })).toEqual({ kind: 'leaf', member: 'stage', operator: 'equals', values: ['won'] }); + expect(treeFor({ amount: { $between: [10, 20] } })).toEqual({ + kind: 'and', + children: [ + { kind: 'leaf', member: 'amount', operator: 'gte', values: [10] }, + { kind: 'leaf', member: 'amount', operator: 'lte', values: [20] }, + ], + }); + expect(treeFor({ stage: { $in: [] } })).toEqual({ kind: 'const', value: false }); + expect(treeFor({ $and: [] })).toBeNull(); + expect(treeFor({ $or: [] })).toEqual({ kind: 'const', value: false }); + expect(treeFor({ $not: {} })).toEqual({ kind: 'const', value: false }); + expect(treeFor([['stage', '=', 'won']])).toEqual({ + kind: 'leaf', member: 'stage', operator: 'equals', values: ['won'], + }); + }); +}); 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 84a7416f06..b4e6675409 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 @@ -65,8 +65,22 @@ function refusalFor(where: unknown): FilterRefusal | undefined { * rows are the sites the bullets missed — same file, same class, same one-line * change; leaving them bare would have kept the defect alive for two spellings * of the same authoring mistake. + * + * `addedAfter5352` marks a refusal that did not EXIST when #5352 ran, so + * `issueBullet` has no meaning for it. The distinction is kept rather than + * folded in because this file's ledger case is a historical record of what + * #5352 covered — a row silently joining the `issueBullet: false` set would + * rewrite that record — while the ledger's OTHER job, "this list is every + * refusing site in the module", has to keep up with the module. A new site + * arrives here so the envelope block below covers it automatically. */ -const REFUSALS: Array<{ name: string; where: unknown; message: RegExp; issueBullet: boolean }> = [ +const REFUSALS: Array<{ + name: string; + where: unknown; + message: RegExp; + issueBullet: boolean; + addedAfter5352?: string; +}> = [ { name: 'operator outside the vocabulary (#3948)', where: { stage: { $sortOf: 'won' } }, @@ -127,6 +141,19 @@ const REFUSALS: Array<{ name: string; where: unknown; message: RegExp; issueBull message: /received a 'where' array that is not a filter/, issueBullet: true, }, + { + // [#6386] `undefined` in a comparand position. Before it, `{stage: undefined}` + // was not refused at all — `buildNode` dropped the key, so a single-key + // `where` ran with NO filter. The full seven-reading table, the `null` + // control group and the position list live in + // `filter-normalizer-undefined-comparand.test.ts`; this row exists so the + // envelope block below covers the tenth site the way it covers the nine. + name: 'an undefined comparand (#6386)', + where: { stage: undefined }, + message: /comparand at "stage" is undefined/, + issueBullet: false, + addedAfter5352: '#6386', + }, ]; /** @@ -255,10 +282,15 @@ describe('[#5352] every refusal carries the ADR-0112 envelope (INVALID_FILTER / // enumerated four bullets, and enveloping only those would have left // `{$not: 5}` and `{$nor: […]}` answering 500 while their neighbours // answered 400 — the same one-condition-two-shapes split the issue is about. - expect(REFUSALS.filter((c) => !c.issueBullet).map((c) => c.name)).toEqual([ + // Sites added AFTER #5352 are excluded from that historical set (see the + // `addedAfter5352` note on REFUSALS) and enumerated on their own below. + expect(REFUSALS.filter((c) => !c.issueBullet && !c.addedAfter5352).map((c) => c.name)).toEqual([ '$not of a non-object', 'unsupported TOP-LEVEL operator', ]); - expect(REFUSALS).toHaveLength(9); + expect(REFUSALS.filter((c) => c.addedAfter5352).map((c) => `${c.name} · ${c.addedAfter5352}`)).toEqual([ + 'an undefined comparand (#6386) · #6386', + ]); + expect(REFUSALS).toHaveLength(10); }); }); 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 c2adc5e3a5..822d3b2729 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 @@ -217,13 +217,39 @@ describe('[#5526] the SQL bind form converts only what a driver cannot bind', () expect(toSqlBindValue([1, 'x'])).toBe('[1,"x"]'); }); - it('`undefined` in a comparand position is normalised to null at the leaf', () => { - // JSON has no `undefined`, so this is not an authorable shape (#5332's - // reading). It must not reach a driver as `undefined` — that is a bind error, - // not a predicate — and `null` is the fail-closed reading. Note the operator - // is still `equals`: only `=== null` is the null PREDICATE. - const node = normalizeAnalyticsFilterTree({ where: { code: { $eq: undefined } } }); - expect(node).toEqual({ kind: 'leaf', member: 'code', operator: 'equals', values: [null] }); + it('`undefined` in a comparand position is REFUSED at this door (#6386)', () => { + // ⚠️ RE-JUDGED, and the distinction matters because two rulings sit one line + // apart here. This case used to assert the leaf `{code equals [null]}`, + // reading `comparand()`'s `undefined` → `null` normalisation (#5526) through + // the door. #6386 pushed #6050's ruling B — an `undefined` where a comparand + // belongs is REFUSED — down to this door, so the observable answer moved. + // + // ⛔ What did NOT move: `comparand()` itself, byte for byte, and with it both + // rulings it carries — #5526's normalisation and #5332's "only `=== null` is + // the null PREDICATE". #6386 refuses an INPUT; it re-decides neither. The + // consequence is that the normalisation is now unreachable FROM THIS DOOR, + // which is a fact worth recording rather than a licence to delete it: + // retiring it is #5526's call on its own terms. + // + // The `null` half of the same position is untouched and asserted below, in + // the SQL/engine consumer blocks — that is the pair this file exists to keep + // apart, and they live one `===` apart in every polarity table in the module. + const refusal = (): unknown => normalizeAnalyticsFilterTree({ where: { code: { $eq: undefined } } }); + expect(refusal).toThrowError(/comparand at "code"\.\$eq is undefined/); + // Still the module's one envelope (#5352), so the REST face answers 400. + try { + refusal(); + expect.unreachable('an undefined comparand must not compile'); + } catch (e) { + expect((e as { code?: unknown }).code).toBe('INVALID_FILTER'); + expect((e as { status?: unknown }).status).toBe(400); + } + // The neighbouring `null` comparand keeps compiling, and to the null + // PREDICATE rather than a value comparison (#5332) — the row that proves the + // refusal did not widen to `== null`. + expect(normalizeAnalyticsFilterTree({ where: { code: { $eq: null } } })).toEqual({ + kind: 'leaf', member: 'code', operator: 'notSet', values: [], + }); }); }); diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index b7f4690e4a..ce267e0d7c 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -238,15 +238,65 @@ * refused — the refusal set is pinned input-by-input in * `filter-refusal-envelope.test.ts` precisely so that stays true. * + * # An `undefined` COMPARAND is refused, not read seven different ways (#6386) + * + * #6050 ruled on 2026-08-07 (ruling B) that `undefined` sitting where a + * comparand belongs is REFUSED, and landed that on `driver-sql` / `driver-turso`. + * #6125 pushed it to this package's OTHER door, `read-scope-sql.ts` (PR #6390). + * This door — the `where` the CALLER writes — had never been named by any of + * those rulings, and it read the one shape seven ways. Measured on `origin/main` + * (`5faa23ca3`) by calling `normalizeAnalyticsFilterTree({ where })` directly: + * + * | `where` | normalized to | reading | + * |---|---|---| + * | `{d: undefined}` | `null` | the WHOLE where dropped — the query ran with NO filter | + * | `{stage:'won', d: undefined}` | `stage equals 'won'` | the `d` conjunct vanished in silence | + * | `{$not: {d: undefined}}` | `NOT (d set)`, i.e. `d IS NULL` | a predicate the author never wrote | + * | `{d: {$eq: undefined}}` | `d equals [null]` | a value comparison, NOT `$eq: null`'s `notSet` | + * | `{d: {$gt: undefined}}` | `d gt [null]` | ditto | + * | `{d: {$in: [undefined]}}` | `d in [null]` | ditto | + * | `{d: {$ne: undefined}}` | `d notSet OR d notEquals [null]` | ditto | + * + * The first three are the whole argument. They WIDEN — which is the failure mode + * the note at {@link MONGO_TO_CUBE_OP}'s miss branch has forbidden in this very + * function since #4128 ("NEVER drop: a missing predicate does not narrow the + * query, it WIDENS it … That failure mode is #3650's"). `buildNode`'s first line + * was `if (raw === undefined) continue;`: the module did, at its entry, the exact + * thing its own body refuses to do a few dozen lines further down. + * + * Row three is the strangest and is worth stating separately, because it is not + * "one conjunct fewer": the #5146 rewrite splits the leaf into `{d: {$null: + * false}} AND {d: undefined}`, the entry gate dropped the second half, and the + * surviving guard was then negated — so a predicate GREW OUT of a discarded leaf. + * + * ⚠️ The direction is silently WRONG RESULTS, not a permission bypass. Read + * scope is compiled by the other door (`read-scope-sql.ts` → `applyReadScope`) + * and never passes through here, so a caller still saw only rows it was entitled + * to — just more of them than it asked for. What was lost is the ANSWER: an + * analytics figure, a report total, an aggregate, wrong with nothing to read. + * + * The refusal is {@link undefinedComparandError}, in this module's existing + * envelope (`INVALID_FILTER` / 400) — the opposite attribution from + * `read-scope-sql`'s 500, and deliberately so: that door compiles a platform + * artifact, this one receives what the CALLER wrote. + * + * ⛔ `null` does not move, and that is the way this change could do harm: the two + * live one `===` apart in every polarity table here. `{d: null}`, `{$eq: null}`, + * `{$ne: null}`, `{$null: …}`, `{$exists: …}` and `$contains: null`'s `%null%` + * (#5526) keep their exact lowering, pinned as a control group in + * `filter-normalizer-undefined-comparand.test.ts`. + * * 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, * the in-memory matcher, `formula` and `read-scope-sql` are already held to, * `filter-normalizer-not-null-safe.test.ts` for the two squares that table * deliberately does not carry (NULL handling, boolean identities), - * `filter-array-lowering.test.ts` for the array door (#5334), and + * `filter-array-lowering.test.ts` for the array door (#5334), * `filter-value-type-fidelity.test.ts` for what each comparand TYPE binds on both - * consumers (#5526, carrying #5528's cases forward as end-to-end assertions). + * consumers (#5526, carrying #5528's cases forward as end-to-end assertions), and + * `filter-normalizer-undefined-comparand.test.ts` for the `undefined` refusal and + * its `null` control group (#6386). */ import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data'; @@ -337,6 +387,22 @@ const MONGO_TO_CUBE_OP: Record = { * leaf, not to `notSet`. Only `=== null` is the null PREDICATE (#5332's identity * test, which this module reads at the operator branch, above this function), * and widening it to `== null` here would re-decide that ruling sideways. + * + * ## Addendum (#6386): the `undefined` arm is now UNREACHABLE from this door + * + * {@link assertDefinedComparands} refuses an `undefined` before any comparand is + * read, and it covers every call site of this function — the `$between` bounds, + * the operator value and its array members, the bare-array `$in` and the implicit + * `=` — so nothing can arrive here holding `undefined` any more. The refusal + * tests enumerate exactly that set of positions, which is what makes the claim + * checkable rather than asserted. + * + * ⛔ It is left in place ON PURPOSE, code untouched. Both statements this + * function makes were RULED — `undefined` → `null` by #5526, and "only `=== null` + * is the null predicate" by #5332 — and #6386 refuses an INPUT without reopening + * either. Deleting a now-dead arm would be a semantic edit smuggled in as a + * cleanup: it is #5526's call whether the normalisation still earns its place + * once its last caller is gated, and that is a separate decision from this one. */ function comparand(v: unknown): unknown { return v === undefined ? null : v; @@ -464,6 +530,135 @@ function assertCompilableComparand(opKey: string, field: string, value: unknown) } } +/** + * [#6386, on #6050's ruling B] `undefined` in a COMPARAND position. + * + * ONE wording for every position (#5240 — one condition, one wording); only + * `path` varies, because only the position does. The wording names BOTH measured + * consequences rather than one, which is where this differs from + * `read-scope-sql`'s twin: there all four cells failed the same way (a silent + * NULL bind), here the same input is read two different ways depending on where + * it sits — the key is DROPPED in the three positions `buildNode` used to skip, + * and compiled as a comparison against `null` in the four {@link comparand} + * normalises. An author who hits either one needs to be told which of their keys + * is unreadable, and both halves of what it would otherwise have done. + * + * ## Why 400 here and 500 on the sibling door + * + * `read-scope-sql.ts` refuses the same shape as `READ_SCOPE_COMPILE_FAILED` / + * 500 because a read scope is compiled by the PLATFORM from CEL and stored + * policy — billing the caller for it would be wrong. This door is the mirror + * image: `where` is what the caller itself passed to `AnalyticsService.query`, + * so it is a 400-class mistake and takes the envelope every other refusal in + * this module already carries (#5352). + * + * ## Why the producer named is the CALLER, and what shape to look for + * + * `undefined` cannot cross JSON, so neither REST door can carry it: it can only + * come from in-process code building the object — `{ owner_id: ctx.user?.id }`, + * the shape #6050 proved reachable on `driver-sql`. Note the platform's OWN + * answer to that need is already fail-closed and is not this shape: a + * `{current_user_id}` placeholder in a dataset / widget / report filter is + * resolved by `resolveFilterTokens` (`@objectstack/core`), which throws + * `FILTER_TOKEN_UNRESOLVED` / 400 rather than emitting `undefined`. + */ +function undefinedComparandError(field: string, path: string): Error { + return invalidFilterError( + `[analytics] comparand at ${path} is undefined — refusing to compile this filter. ` + + `@objectstack/spec FieldOperatorsSchema declares no undefined comparand, and in JavaScript a key ` + + `whose value is undefined cannot be told apart from an ABSENT key — yet the two mean OPPOSITE ` + + `things (a predicate versus no constraint at all), so there is no reading of it that is not a ` + + `guess. It used to compile, two ways: in a FIELD position the key was dropped outright, so a ` + + `single-key where ran with no filter at all and the chart was drawn over every row (#3650's ` + + `widening, which this module refuses everywhere else); in an OPERATOR or list position it ` + + `became a comparison against null, which is UNKNOWN for every row and charts nothing. ` + + `Write null if the null predicate was meant ({ "${field}": null } or { "${field}": { "$null": true } }), ` + + `or omit the key entirely when the value is genuinely absent — an omitted key is the same "no ` + + `constraint" without the ambiguity. The producer to fix is whoever BUILT this where: undefined ` + + `cannot cross JSON, so it is in-process code spreading a possibly-absent value into a filter ` + + `object (#6050 ruling B, pushed down to this door by #6386).`, + ); +} + +/** + * [#6386] Refuse every `undefined` sitting in a comparand position of ONE field + * constraint. + * + * The positions are enumerated rather than swept, because "comparand" is a + * POSITION and not a type: + * + * - the DIRECT comparand — `{d: undefined}`, the implicit `=`. Reached for a + * nested relation too, because {@link fieldLeaves} recurses into one with the + * DOTTED member name, so `{profile: {verified: undefined}}` is refused as + * `"profile.verified"` — the member the leaf would have carried, not the + * relation. (`read-scope-sql`'s twin has no such case: it refuses nested + * relations outright.) + * - a MEMBER of the bare-array implicit `$in` — `{d: [1, undefined]}`. The + * array itself is a legitimate comparand here, so its elements are comparands + * in their own right. This is the deliberate divergence from that twin, which + * refuses a bare array as a whole and so must not relabel it. + * - 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. + * + * `$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 + * `undefined` there is not a comparand at all. ⚠️ This module reads that flag by + * IDENTITY (`=== true` / `=== false`, see {@link fieldLeaves}) where the twin + * reads it by truthiness, so `{$null: undefined}` lowers here to `set` + * (`IS NOT NULL`). That is the boolean-DOMAIN question #5347 / #5369 opened and + * #6387 is measuring on the sibling door; it is a different cell and is not + * decided as a rider on this one. + * + * ## Why the gate sits HERE, and what that decides for `{$not: {d: undefined}}` + * + * {@link fieldLeaves} is the only producer of leaf nodes in this module, so one + * gate covers all three consumers of the tree at once — the same argument + * {@link assertCompilableComparand} makes one function below. + * + * That places it DOWNSTREAM of {@link nullSafeNegationOperand}, and for row three + * of the header's table that choice is the whole question: a gate on the far side + * of the #5146 rewrite refuses, while a rewrite that could swallow the leaf first + * would leave a CHANGED SHAPE for the gate to bless. Measured rather than + * assumed, because the same trap cost PR #6390 a lap on the sibling door — and + * the reasoning there does NOT transfer, since the two modules' polarity tables + * are spelled differently (that one is uniformly `=== null`; this one mixes + * `=== null` for `$eq`/`$ne` with IDENTITY reads for `$null`/`$exists`). What the + * measurement shows here is that the rewrite never drops a leaf: every guard + * disposition — `requireValue` pushes `{k: {$null: false}}, {k: spec}`, + * `allowNull` pushes `{$or: [{k: {$null: true}}, {k: spec}]}`, `none` writes + * `out[k] = spec` — carries `spec` through by reference, so the author's + * `undefined` always reaches this gate and always throws. Pinned in + * `filter-normalizer-undefined-comparand.test.ts` as its own block: one case per + * rewrite path that can carry a SWEPT comparand (`requireValue`, `allowNull`, and + * the nested-relation recursion), plus the measured reason there is no third — + * `none` needs every operator to satisfy {@link operatorIsNullTotal}, which is + * false for an `undefined` comparand on every operator this gate sweeps, so the + * only field specs that reach it holding one are the `$null` / `$exists` flags it + * deliberately does not sweep. + */ +function assertDefinedComparands(field: string, spec: unknown): void { + const root = `"${field}"`; + if (spec === undefined) throw undefinedComparandError(field, root); + if (Array.isArray(spec)) { + spec.forEach((member, index) => { + if (member === undefined) throw undefinedComparandError(field, `${root}[${index}]`); + }); + return; + } + if (!isFilterObject(spec)) return; + for (const [op, opValue] of Object.entries(spec)) { + if (!op.startsWith('$') || op === '$null' || op === '$exists') continue; + const opPath = `${root}.${op}`; + if (opValue === undefined) throw undefinedComparandError(field, opPath); + if (!Array.isArray(opValue)) continue; + opValue.forEach((member, index) => { + if (member === undefined) throw undefinedComparandError(field, `${opPath}[${index}]`); + }); + } +} + /** * Compile one `field: value | { $op: … }` entry into its leaves. * @@ -472,6 +667,12 @@ function assertCompilableComparand(opKey: string, field: string, value: unknown) * `{ $gte, $lte }` depends on. */ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { + // [#6386] `undefined` in a comparand position, refused before any leaf exists. + // First statement of the only leaf producer, so no consumer of the tree can be + // handed one — see {@link assertDefinedComparands} for the position list and + // for why this side of the `$not` rewrite is the load-bearing choice. + assertDefinedComparands(key, raw); + const out: NormalizedFilterNode[] = []; const leaf = (operator: string, values: unknown[]): void => { out.push({ kind: 'leaf', member: key, operator, values }); @@ -660,8 +861,22 @@ function buildNode(cond: Record): NormalizedFilterNode | null { const children: NormalizedFilterNode[] = []; for (const [key, raw] of Object.entries(cond)) { - if (raw === undefined) continue; - + // [#6386] What used to be here — `if (raw === undefined) continue;` — was + // the entry gate doing the one thing the rest of this file forbids: a key + // dropped without trace, which does not narrow the query, it WIDENS it (see + // the module header's table and the note at MONGO_TO_CUBE_OP's miss branch). + // Removing it does NOT create four new refusals; every key kind now reaches + // the branch that already had the truest thing to say about it: + // + // {d: undefined} → `fieldLeaves` → `assertDefinedComparands` (#6386) + // {$and|$or: undefined} → "requires an array of filter objects, got undefined" + // {$not: undefined} → "requires a filter object, got undefined" + // {$other: undefined} → "Unsupported top-level filter operator" + // + // ⚠️ An absent `where` is untouched and still means "no constraint": + // `lowerAnalyticsWhere` answers `null` for `{where: undefined}` before this + // function runs. The refusal is about a KEY INSIDE a `where`, where dropping + // it silently changes which rows the author gets. if (key === '$and' || key === '$or') { if (!Array.isArray(raw)) { throw invalidFilterError(