fix(analytics): $ne / $nin / $notContains 在 Cube 面保留无值行 (#5298 第二批) (#5977)

.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.

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 () => {

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',

packages/services/service-analytics/src/__tests__/objectql-contains-canonical-operator.test.ts

@@ -143,6 +143,68 @@ function matchesLikeFamily(row: (typeof FIXTURE)[number], cond: Record<string, u
   return true;
 }
 
+/**
+ * [#5298] Evaluate the whole `FilterCondition` the strategy hands the engine,
+ * not just its `stage` entry.
+ *
+ * `$notContains` is NULL-safe now, so the strategy emits it as
+ * `{$or: [{stage: null}, {stage: {$notContains: …}}]}` and the condition no
+ * longer has `stage` at the top level. Reading `filter.stage` alone therefore
+ * found `undefined`, handed {@link matchesLikeFamily} an EMPTY operator object,
+ * and every row passed the empty loop — the whole fixture came back and the
+ * assertion below would have been green for a filter that matched nothing in
+ * particular. A stand-in engine that cannot read the shape under test measures
+ * the stand-in, so it walks the tree.
+ */
+function matchesCondition(row: (typeof FIXTURE)[number], cond: Record<string, unknown>): boolean {
+  for (const [key, value] of Object.entries(cond)) {
+    if (key === '$and') {
+      if (!(value as Record<string, unknown>[]).every((c) => matchesCondition(row, c))) return false;
+      continue;
+    }
+    if (key === '$or') {
+      if (!(value as Record<string, unknown>[]).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<string, unknown>)) 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, unknown>): string[] {
+  const found: string[] = [];
+  const walk = (node: Record<string, unknown>): void => {
+    for (const [key, value] of Object.entries(node)) {
+      if (key === '$and' || key === '$or') {
+        for (const child of value as Record<string, unknown>[]) 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<string, unknown>));
+    }
+  };
+  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<string, unknown>);
+        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<string, unknown> },
         ) => {
-          const cond = ((options.filter ?? {}) as Record<string, unknown>).stage;
-          const pred = (cond ?? {}) as Record<string, unknown>;
-          return FIXTURE.filter((r) => matchesLikeFamily(r, pred)).map((r) => ({
+          const cond = (options.filter ?? {}) as Record<string, unknown>;
+          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']);
     });
   });