From ee5b9a06fd449957573568ac28a3dd9f892fab7b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 14:14:36 +0000 Subject: [PATCH] fix(analytics): $ne / $nin / $notContains keep the no-value rows on the Cube face (#5977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second batch of the #5298 ruling; PR #5962 landed the first on driver-sql, read-scope-sql and formula. Measured before the change on this package's own fixture, for `{stage: {$ne: 'won'}}` with rows 3-4 carrying a NULL `stage`: the raw-SQL strategy answered `2`, the display-SQL echo answered `2`, and the ObjectQL engine condition answered `2,3,4`. Three compilers of ONE normalized tree, two answers — and the engine column was right only because driver-sql guards for itself since #5962, not because the analytics layer did. The echo was the worst of the three: it described a NARROWER query than the one that ran. `fieldLeaves` now emits the guard as tree STRUCTURE — an `or` of the `notSet` leaf with the comparison — rather than as a SQL trick inside one strategy, so all three compilers produce one predicate and none of them needs to know the rule. Same trade the #5146 `$not` rewrite in this file already took, including its cost: the engine path guards twice, which is idempotent. Which operators are guarded is NOT a new list. It is the existing polarity pair `nullValueSatisfiesOperator` / `operatorIsNullTotal` — the guard applies exactly on their `allowNull` verdict. Those arms read the COMPARAND, so `$ne: null` stays `IS NOT NULL` and an empty `$nin` stays the TRUE constant; a hard-coded list of three operator names would have been wrong as well as duplicated. Positive comparisons compile byte-identically, and `{$not: {stage: {$ne: 'v'}}}` still means "stage is v" rather than widening. Fixture triage, each re-judged individually rather than batch re-spelled: - `filter-operator-coverage.test.ts` — three cases gain the `d_null` row. - `objectql-daterange.test.ts` — engine-filter shape; its subject (both operands survive the merge) is unchanged and still visible. - `objectql-contains-canonical-operator.test.ts` — its stand-in engine read only `filter.stage`, which the guarded tree no longer has at top level, so it was handed an EMPTY operator object and passed every row. Replaced with a tree walk; the operator-key assertion got the same treatment. - `filter-normalizer-not-null-safe.test.ts` — the line that recorded this divergence as "real and out of #5146's scope" is now the ruling's answer. FILTER_LOGIC_CASES is unchanged: `$ne` / `$not` enrol in #5903's PR, the last blocker. The spec table's measured matrix drops the Cube row it no longer describes. Fixes #5977 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx --- ...cs-filter-normalizer-null-safe-negation.md | 40 ++++++ .../filter-normalizer-not-null-safe.test.ts | 123 +++++++++++++++++- .../filter-operator-coverage.test.ts | 18 ++- ...jectql-contains-canonical-operator.test.ts | 84 ++++++++++-- .../src/__tests__/objectql-daterange.test.ts | 8 +- .../src/strategies/filter-normalizer.ts | 49 ++++++- .../spec/src/data/filter-logic-conformance.ts | 14 +- 7 files changed, 313 insertions(+), 23 deletions(-) create mode 100644 .changeset/analytics-filter-normalizer-null-safe-negation.md diff --git a/.changeset/analytics-filter-normalizer-null-safe-negation.md b/.changeset/analytics-filter-normalizer-null-safe-negation.md new file mode 100644 index 0000000000..fa94f7149f --- /dev/null +++ b/.changeset/analytics-filter-normalizer-null-safe-negation.md @@ -0,0 +1,40 @@ +--- +'@objectstack/service-analytics': patch +'@objectstack/spec': patch +--- + +analytics: `$ne` / `$nin` / `$notContains` in a dashboard `where` keep the rows that have no value + +Second batch of the #5298 ruling, after PR #5962 landed it on `driver-sql`, +`read-scope-sql` and `formula`. An analytics filter meaning "not this" now +returns the rows whose column is empty, the same answer every other backend +gives — a `stage != 'won'` widget shows the deals with no stage set. + +The Cube face was the last surface still splitting on it, and it split three +ways for one filter. Measured on the package's own fixture before the change, +for `{stage: {$ne: 'won'}}` with rows 3-4 carrying a NULL `stage`: + +| compiler | was | now | +|---|---|---| +| `NativeSQLStrategy` raw SQL | `2` | `2,3,4` | +| `ObjectQLStrategy` display-SQL echo | `2` | `2,3,4` | +| `ObjectQLStrategy` engine condition | `2,3,4` | `2,3,4` | + +The engine column was already right — because `driver-sql` guards for itself +since #5962, not because the analytics layer did — so which rows a widget drew +depended on which compiler downstream caught the leaf, and the `/analytics/sql` +echo described a narrower query than the one that ran. + +`filter-normalizer` now emits the guard as tree STRUCTURE (an `or` of the null +predicate with the comparison) rather than as a SQL trick in one strategy, so +all three compilers of that tree produce one predicate and none of them needs +to know the rule. Which operators are guarded is decided by the polarity table +the `$not` rewrite already consults, not by a second list of operator names: +positive comparisons (`$eq`, `$in`, `$contains`, the ordering family) compile +byte-identically to before, `$ne: null` stays `IS NOT NULL`, an empty `$nin` +stays the TRUE constant, and `{$not: {stage: {$ne: 'won'}}}` still means +"stage is won" rather than widening. + +`FILTER_LOGIC_CASES` is unchanged: the `$ne` and `$not` null rows enrol in +#5903's PR, which clears the last backend (`driver-turso` remote). The spec +table's measured blocker matrix drops the Cube row it no longer describes. 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 3f2fa0dfaa..4149e12a8d 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 @@ -354,8 +354,16 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // filter excludes. `{$not: {$ne: 'won'}}` means "stage IS won". expect(await ids({ $not: { stage: { $ne: 'won' } } })).toEqual(['1']); expect(await ids({ $not: { stage: { $nin: ['won'] } } })).toEqual(['1']); + // [#5298] The guard now appears TWICE: `nullSafeNegationOperand`'s + // `allowNull` arm wraps the field spec, and `fieldLeaves` wraps the `$ne` + // leaf itself because the operator is NULL-safe everywhere now, not only + // under a `$not`. `X OR (X OR Y)` ≡ `X OR Y`, so the predicate is the one + // this case has always asserted — the two id sets above are the guarantee, + // and they are unchanged. Asserted as it is actually emitted rather than + // trimmed to the prettier form: a pin that describes SQL the compiler does + // not produce is how the next reader learns to distrust this file. const { sql } = await sqlFor({ $not: { stage: { $ne: 'won' } } }); - expect(sql).toContain('NOT ((stage IS NULL OR stage != $1))'); + expect(sql).toContain('NOT ((stage IS NULL OR (stage IS NULL OR stage != $1)))'); }); it('`$not` of an ordering comparison returns the NULL rows', async () => { @@ -438,6 +446,103 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit }); }); + // ── [#5298] The operators that carry their OWN negation ─────────────────── + + /** + * [#5298] `$ne` / `$nin` / `$notContains` OUTSIDE a `$not`. + * + * The ruling that finished what #5146 started: an operator that means "not + * this" answers TRUE for a column with no value, on every backend. Before + * this, the Cube face compiled the bare three-valued forms (`stage != $1`, + * `NOT IN`, `NOT LIKE`), each of which is UNKNOWN for a NULL column, so a + * `WHERE` dropped exactly the rows the JS backends return. + * + * The ids are the same ones `driver-sql`'s `sql-driver-not-null-safe.test.ts`, + * `formula`'s `matches-filter-not-null-safe.test.ts` and this package's + * `read-scope-not-null-safe.test.ts` assert for the same filters — moving one + * re-opens the divergence rather than adjusting a local expectation. + * + * Measured before the fix on this exact fixture (#5977), each answering `2`: + * `NativeSQLStrategy`, and the display-SQL echo. The ObjectQL ENGINE column + * already answered `2,3,4` — because `driver-sql` guards for itself since + * #5962, not because this module did — which is the whole reason the guard + * belongs here: three compilers of one tree must not need three copies of one + * rule to agree. + */ + describe('[#5298] `$ne` / `$nin` / `$notContains` include the no-value rows', () => { + const echo = async (where: unknown): Promise<{ sql: string; params: unknown[] }> => + new ObjectQLStrategy().generateSql(query(where), objectqlCtx); + + it('`$ne` returns the NULL-stage rows — was `2`', async () => { + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); + }); + + it('`$nin` returns them — was `2`', async () => { + expect(await ids({ stage: { $nin: ['won'] } })).toEqual(['2', '3', '4']); + }); + + it('`$notContains` returns them — was `2`', async () => { + expect(await ids({ stage: { $notContains: 'wo' } })).toEqual(['2', '3', '4']); + }); + + it('the guard is an OR expansion, the shape #5962 landed everywhere else', async () => { + // Not a dialect equivalent (`IS DISTINCT FROM` / `<=>`): `NOT LIKE` has no + // such form, so the family would have needed two shapes. The cost-list + // measurement (#5298 §2/§3) found the query plans identical either way. + expect((await sqlFor({ stage: { $ne: 'won' } })).sql) + .toContain('WHERE (stage IS NULL OR stage != $1)'); + expect((await sqlFor({ stage: { $nin: ['won'] } })).sql) + .toContain('WHERE (stage IS NULL OR stage NOT IN ($1))'); + expect((await sqlFor({ stage: { $notContains: 'wo' } })).sql) + .toContain('WHERE (stage IS NULL OR stage NOT LIKE $1 ESCAPE $2)'); + }); + + it('the ObjectQL path and the display SQL agree with the raw-SQL path', async () => { + // The three compilers of this tree, one predicate. The echo matters on its + // own: a statement that describes a NARROWER query than the one that ran + // sends whoever is debugging "why does this chart show empty rows" after a + // predicate the engine never applied (#5333's failure, mirrored). + expect(await engineIds({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); + expect(await engineIds({ stage: { $nin: ['won'] } })).toEqual(['2', '3', '4']); + expect(await engineIds({ stage: { $notContains: 'wo' } })).toEqual(['2', '3', '4']); + expect((await echo({ stage: { $ne: 'won' } })).sql) + .toContain('(stage IS NULL OR stage != $1)'); + }); + + it('positive comparisons take NO guard — the polarity table decides, not a name list', async () => { + // A blanket null escape would hand back the rows these filters exclude. + // `$eq` / `$in` / `$contains` are the family `nullValueSatisfiesOperator` + // answers `false` for, and they compile byte-identically to before. + expect((await sqlFor({ stage: { $eq: 'won' } })).sql).toContain('WHERE stage = $1'); + expect((await sqlFor({ stage: { $in: ['won'] } })).sql).toContain('WHERE stage IN ($1)'); + expect((await sqlFor({ stage: { $contains: 'wo' } })).sql) + .toContain('WHERE stage LIKE $1 ESCAPE $2'); + expect(await ids({ stage: { $eq: 'won' } })).toEqual(['1']); + expect(await ids({ stage: { $contains: 'wo' } })).toEqual(['1']); + }); + + it('the operators that are already TOTAL are not wrapped either', async () => { + // `$ne: null` compiles to `set` (`IS NOT NULL`), which is two-valued by + // construction — wrapping it would turn "stage has a value" into a + // tautology. `operatorIsNullTotal` is what keeps the two apart, and it + // reads the COMPARAND, which is why a hard-coded list of three operator + // NAMES would have been wrong here as well as duplicated. + expect((await sqlFor({ stage: { $ne: null } })).sql).toContain('WHERE stage IS NOT NULL'); + expect(await ids({ stage: { $ne: null } })).toEqual(['1', '2']); + // An empty `$nin` is the TRUE constant (#5134), also total. + expect((await sqlFor({ stage: { $nin: [] } })).sql).toContain('1 = 1'); + expect(await ids({ stage: { $nin: [] } })).toEqual(ALL); + }); + + it('a guarded leaf still ANDs with its siblings', async () => { + // The guard is ONE conjunct — `(stage IS NULL OR stage != 'won')` — so the + // OR binds tighter than the AND the node's keys form. Parenthesised + // wrongly, `owner = 'u1'` would have been swallowed into the disjunction + // and the filter would have widened instead of narrowing. + expect(await ids({ stage: { $ne: 'won' }, owner: 'u1' })).toEqual(['3']); + }); + }); + // ── An empty set is a constant, not an absent predicate ─────────────────── describe('an empty `$in` / bare `[]` is the FALSE constant, not a dropped clause', () => { @@ -627,9 +732,12 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit expect(sql).toContain('WHERE stage = $1'); expect(params).toEqual(['won']); expect(await ids({ stage: 'won' })).toEqual(['1']); - // Still three-valued OUTSIDE a negation: `!= 'won'` drops the NULL rows. - // That divergence from the JS backends is real and out of #5146's scope. - expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2']); + // [#5298] `$ne` outside a negation used to answer `['2']` — three-valued + // SQL dropping the NULL rows the JS backends return — and this line + // recorded that divergence as "real and out of #5146's scope". #5298 ruled + // it, so the answer moved to the JS family's; the cases for the whole + // trio are in the `$ne` / `$nin` / `$notContains` block below. + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); expect(await ids({})).toEqual(ALL); }); }); @@ -714,9 +822,12 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit expect(normalizeAnalyticsFilterTree({ where: { stage: { $eq: '' } } })).toEqual({ kind: 'leaf', member: 'stage', operator: 'equals', values: [''], }); - // And a non-null comparand of the same operators is untouched. + // And a non-null comparand of the same operators is still a VALUE + // comparison — `$eq` unchanged, `$ne` answering the JS family's row set + // since #5298 (the NULL rows satisfy "is not 'won'"; what stays out is the + // reading of `''` as a null predicate, which is what this case guards). expect(await ids({ stage: { $eq: 'won' } })).toEqual(['1']); - expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2']); + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); }); it('the ObjectQL path hands the engine a null predicate, not `\'\'`', async () => { 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 11cd0c06a8..9a14e8cb59 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 @@ -64,20 +64,30 @@ const CUBE: Cube = { */ const CASES: Array<{ op: string; filter: FilterCondition; expected: string[]; note?: string }> = [ { op: '$eq', filter: { name: { $eq: 'alpha-one' } }, expected: ['a_alpha'] }, - { op: '$ne', filter: { name: { $ne: 'alpha-one' } }, expected: ['b_alphex', 'c_beta'] }, + { + op: '$ne', + filter: { name: { $ne: 'alpha-one' } }, + expected: ['b_alphex', 'c_beta', 'd_null'], + note: '#5298: `d_null` has no `name`, and "has no value" satisfies "is not alpha-one" on every backend. It was excluded while this path emitted a bare `name != ?`, which SQL evaluates UNKNOWN for NULL.', + }, { op: '$gt', filter: { score: { $gt: 20 } }, expected: ['c_beta', 'd_null'] }, { op: '$gte', filter: { score: { $gte: 20 } }, expected: ['b_alphex', 'c_beta', 'd_null'] }, { op: '$lt', filter: { score: { $lt: 20 } }, expected: ['a_alpha'] }, { op: '$lte', filter: { score: { $lte: 20 } }, expected: ['a_alpha', 'b_alphex'] }, { op: '$in', filter: { name: { $in: ['alpha-one', 'beta-one'] } }, expected: ['a_alpha', 'c_beta'] }, - { op: '$nin', filter: { name: { $nin: ['alpha-one'] } }, expected: ['b_alphex', 'c_beta'] }, + { + op: '$nin', + filter: { name: { $nin: ['alpha-one'] } }, + expected: ['b_alphex', 'c_beta', 'd_null'], + note: '#5298: same ruling as `$ne` — `NOT IN` is UNKNOWN for a NULL column, so `d_null` used to fall out of a set it is not a member of.', + }, { op: '$between', filter: { score: { $between: [20, 30] } }, expected: ['b_alphex', 'c_beta'], note: '#4128: was dropped → every row.' }, { op: '$contains', filter: { name: { $contains: 'one' } }, expected: ['a_alpha', 'c_beta'] }, { op: '$notContains', filter: { name: { $notContains: 'one' } }, - expected: ['b_alphex'], - note: 'On the ObjectQL path this had no arm and fell to the default, compiling "does not contain" as an EQUALITY.', + expected: ['b_alphex', 'd_null'], + note: 'On the ObjectQL path this had no arm and fell to the default, compiling "does not contain" as an EQUALITY. #5298 added `d_null`: `NOT LIKE` is UNKNOWN for a NULL column, so a row with no `name` was dropped from "name does not contain one".', }, { op: '$startsWith', diff --git a/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts b/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts index 4e2b665668..8cd5cfb95a 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts @@ -143,6 +143,68 @@ function matchesLikeFamily(row: (typeof FIXTURE)[number], cond: Record): boolean { + for (const [key, value] of Object.entries(cond)) { + if (key === '$and') { + if (!(value as Record[]).every((c) => matchesCondition(row, c))) return false; + continue; + } + if (key === '$or') { + if (!(value as Record[]).some((c) => matchesCondition(row, c))) return false; + continue; + } + if (key !== 'stage') throw new Error(`[test] this face only scopes "stage", got "${key}"`); + // A bare `null` comparand is the null PREDICATE — the guard's own disjunct, + // and the spelling every driver reads as IS NULL. + if (value === null) { + if (row.stage !== null) return false; + continue; + } + if (!matchesLikeFamily(row, value as Record)) return false; + } + return true; +} + +/** + * Every operator key the condition carries for `stage`, at any depth. + * + * [#5298] Also a tree walk, for the same reason {@link matchesCondition} is: the + * NULL-safe guard moves the operator object one level down, and a lookup that + * stopped at `filter.stage` reported ZERO operators — which the "is it declared" + * assertion would have passed vacuously had it not also demanded a non-empty + * set. That non-empty demand is why this file caught the shape change instead + * of quietly asserting nothing. + */ +function stageOperators(cond: Record): string[] { + const found: string[] = []; + const walk = (node: Record): void => { + for (const [key, value] of Object.entries(node)) { + if (key === '$and' || key === '$or') { + for (const child of value as Record[]) walk(child); + continue; + } + // `{stage: null}` is the null predicate, not an operator object. + if (value === null || typeof value !== 'object') continue; + found.push(...Object.keys(value as Record)); + } + }; + walk(cond); + return found; +} + /** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */ async function locateWasm(): Promise<((file: string) => string) | undefined> { try { @@ -189,8 +251,12 @@ describe('[#5557] `contains` reaches the engine as `$contains`, comparand taken }); it('the three siblings are unchanged — the family is uniform now', async () => { + // [#5298] `$notContains` is NULL-safe, so it reaches the engine wrapped in + // the null-predicate disjunct. What THIS case asserts is unaffected and + // still exact: the operator key is the declared `$notContains` and the + // comparand is the author's literal `'a.b'`, not a `$regex` pattern. expect(await engineFilter({ stage: { $notContains: 'a.b' } })).toEqual({ - stage: { $notContains: 'a.b' }, + $and: [{ $or: [{ stage: null }, { stage: { $notContains: 'a.b' } }] }], }); expect(await engineFilter({ stage: { $startsWith: 'a.b' } })).toEqual({ stage: { $startsWith: 'a.b' }, @@ -213,7 +279,7 @@ describe('[#5557] `contains` reaches the engine as `$contains`, comparand taken ['$endsWith', 'a.b'], ] as const) { const filter = await engineFilter({ stage: { [op]: comparand } }); - const emitted = Object.keys((filter.stage ?? {}) as Record); + const emitted = stageOperators(filter); expect(emitted.length, `${op} emitted no operator`).toBeGreaterThan(0); for (const key of emitted) { expect(declared.has(key), `${op} emitted undeclared operator "${key}"`).toBe(true); @@ -237,9 +303,8 @@ describe('[#5557] `contains` reaches the engine as `$contains`, comparand taken _object: string, options: { filter?: Record }, ) => { - const cond = ((options.filter ?? {}) as Record).stage; - const pred = (cond ?? {}) as Record; - return FIXTURE.filter((r) => matchesLikeFamily(r, pred)).map((r) => ({ + const cond = (options.filter ?? {}) as Record; + return FIXTURE.filter((r) => matchesCondition(r, cond)).map((r) => ({ id: r.id, total: 1, })); @@ -271,9 +336,12 @@ describe('[#5557] `contains` reaches the engine as `$contains`, comparand taken it('the anchored siblings stay anchored on the same fixture', async () => { expect(await ids({ stage: { $startsWith: 'a.' } })).toEqual(['1']); expect(await ids({ stage: { $endsWith: '.b' } })).toEqual(['1']); - // NULL `stage` (row 5) satisfies neither direction on this face — a - // non-string value fails every LIKE arm, matcher included. - expect(await ids({ stage: { $notContains: 'a.b' } })).toEqual(['2', '3', '4']); + // [#5298] Row 5 has no `stage`, and "has no value" now satisfies "does not + // contain a.b" — the JS family's answer, reached here through the guard's + // null-predicate disjunct rather than through the matcher, whose LIKE arms + // still reject a non-string. The anchored pair above is the control: they + // are positive-polarity, take no guard, and row 5 stays out of both. + expect(await ids({ stage: { $notContains: 'a.b' } })).toEqual(['2', '3', '4', '5']); }); }); diff --git a/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts b/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts index 9e2cea2de0..73fd7e4b78 100644 --- a/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts +++ b/packages/services/service-analytics/src/__tests__/objectql-daterange.test.ts @@ -307,9 +307,15 @@ describe('ObjectQLStrategy — window ∧ where on one field (#3650)', () => { ctx, ); + // [#5298] The `$ne` operand arrives NULL-safe: `fieldLeaves` emits it as an + // `or` of the null predicate with the comparison, so "stage is not lost" + // keeps the rows that have no stage — the answer every other backend gives. + // What this case is about is unchanged and still visible: BOTH operands + // survive, the second as its own `$and` conjunct rather than overwriting the + // bare equality. expect(seen[0].filter).toEqual({ stage: 'won', - $and: [{ stage: { $ne: 'lost' } }], + $and: [{ $or: [{ stage: null }, { stage: { $ne: 'lost' } }] }], }); }); }); diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index 8251a576d7..d5550167ad 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -94,6 +94,40 @@ * `NOT (c IS NOT NULL AND (c IS NOT NULL AND c = v))` is the same predicate — * so it buys portability for one redundant conjunct. * + * # `$ne` / `$nin` / `$notContains` are NULL-safe too (#5298) + * + * Same rule, same reason, one ruling later. The operators that carry their OWN + * negation had the defect #5146 fixed for `$not`: a bare `col <> ?` is UNKNOWN + * for a NULL column and the `WHERE` drops the row, while the JS backends return + * it. Measured on this package's own fixture before the fix (#5977), for + * `{stage: {$ne: 'won'}}` over rows whose `stage` is NULL: + * + * | path | was | now (= JS family) | + * |----------------------------------------|-----------|-------------------| + * | `NativeSQLStrategy` (raw SQL) | `2` | `2,3,4` | + * | `ObjectQLStrategy` display SQL echo | `2` | `2,3,4` | + * | `ObjectQLStrategy` → engine condition | `2,3,4` | `2,3,4` | + * + * The engine column was already right, and that is the whole argument for + * fixing it HERE: it was right because `driver-sql` guards for itself (#5962), + * so the Cube face's answer depended on which compiler downstream caught the + * leaf — three emitters, two answers. `fieldLeaves` now emits the guard as + * STRUCTURE, an `or` of `notSet` with the comparison, so all three compile the + * same predicate and none of them needs to know the rule. That is the same + * trade the `$not` rewrite above took, including its cost: the engine path + * guards twice, which is idempotent (`c IS NULL OR (c IS NULL OR c <> v)`). + * + * Which operators get the guard is NOT a new list — it is + * {@link nullValueSatisfiesOperator} and {@link operatorIsNullTotal}, the same + * pair `nullGuardForFieldSpec` consults for the `$not` rewrite, asked about one + * operator instead of a whole field spec. A leaf is guarded exactly when a NULL + * value SATISFIES the operator and the compiled leaf is not already total, which + * is that pair's `allowNull` verdict. Hard-coding the three names would have put + * a second polarity table in this file, free to drift from the first — and the + * `$eq`/`$ne` arms of the existing one already turn on the COMPARAND (`$ne: + * null` compiles to `set`, which is total and must never be widened), so a name + * list would have been wrong as well as duplicated. + * * # A `null` COMPARAND is a null predicate, not a value (#5332) * * `{stage: null}` compiled to `IS NULL` while `{stage: {$eq: null}}` compiled to @@ -524,7 +558,20 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { ); } const v = wrapper[opKey]; - leaf(cubeOp, Array.isArray(v) ? v.map(comparand) : [comparand(v)]); + const values = Array.isArray(v) ? v.map(comparand) : [comparand(v)]; + // [#5298] The operators that carry their own negation are NULL-safe, + // here as everywhere else — see the module header's section on it. + if (nullValueSatisfiesOperator(opKey, v) && !operatorIsNullTotal(opKey, v)) { + out.push({ + kind: 'or', + children: [ + { kind: 'leaf', member: key, operator: 'notSet', values: [] }, + { kind: 'leaf', member: key, operator: cubeOp, values }, + ], + }); + continue; + } + leaf(cubeOp, values); } return out; } diff --git a/packages/spec/src/data/filter-logic-conformance.ts b/packages/spec/src/data/filter-logic-conformance.ts index 5c7a70e3c8..569750b40e 100644 --- a/packages/spec/src/data/filter-logic-conformance.ts +++ b/packages/spec/src/data/filter-logic-conformance.ts @@ -95,18 +95,26 @@ * * The FIXTURE half of this work is DONE (#5298): {@link FilterLogicRow.d} is * nullable, all eleven harnesses declare and seed it, and the `$null` partition - * below proves they seeded it as NULL. What remains is two backends, both - * measured against this fixture on 2026-08-06 rather than assumed: + * below proves they seeded it as NULL. What remains is ONE backend, measured + * against this fixture on 2026-08-06 rather than assumed: * * | backend | `{d: {$ne: 'v1'}}` | `{$not: {d: 'v1'}}` | blocker | * |---|---|---|---| * | `driver-turso` REMOTE | `['2']` | `['2']` | #5903 | - * | `service-analytics` `filter-normalizer` (Cube) | `['2']` | ✅ | #5298 batch 2 | * * Everything else already answers `['2','3','4']` on both: `driver-sql`, * `driver-sqlite-wasm`, `driver-turso` LOCAL, `read-scope-sql`, `driver-memory` * (both surfaces), `driver-mongodb` and `formula`. * + * `service-analytics`'s `filter-normalizer` (the Cube face) was the second row + * of that table until #5977, answering `['2']` for `$ne` while already answering + * `$not` correctly — the #5146 rewrite lived in the normalizer, the three + * self-negating operators did not. It now emits the guard as tree STRUCTURE, so + * all three compilers of that tree (raw SQL, the ObjectQL engine condition and + * the display-SQL echo) answer `['2','3','4']`. That leaves `driver-turso` + * remote as the sole blocker for BOTH rows, so `$ne` and `$not` now enrol + * together, in #5903's PR. + * * `driver-turso` remote is the interesting one and the reason the pre-#5298 * version of this note was WRONG where it said "every surface answers this * family the same way — no backend blocker remains". `TursoDriver` extends