From a17c7c1ff1c6efb05ea09eb8bb4fb15e2c7b4734 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:08:29 +0000 Subject: [PATCH] =?UTF-8?q?fix(objectql):=20HAVING=20=E7=9A=84=20`$nin`=20?= =?UTF-8?q?/=20`$notContains`=20=E5=AF=B9=E6=97=A0=E5=80=BC=E8=A1=8C=20NUL?= =?UTF-8?q?L-safe=20(#5905)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5298 的方案 A 裁决(「列没有值」满足「不是这个值」的测试)由 PR #5962 落到了 driver-sql / formula / service-analytics 与 `FILTER_LOGIC_*` 一致性表,但那次 清点里没有 `packages/objectql/src/having-filter.ts` —— HAVING 是同一套算子词表 的第五个求值面,于是成为唯一仍与已生效裁决相反的一面,而且是唯一没有任何 一致性表覆盖的一面(`FILTER_LOGIC_CASES` 不驱动 HAVING 路径,已实测: packages/objectql 里零处引用)。 本 PR 只对齐被裁决的两格,不重开语义取舍: - 早退守卫的豁免名单补上 `$nin` / `$notContains`,并抽成具名常量 `NO_VALUE_ANSWERED_BY_OPERATOR` 把「哪些算子自己回答无值」写在一处。 此前守卫先于算子分支返回 false,`$nin` 分支本来会答 true 却从未被走到。 - `$notContains` 改为 formula 的读法(`matches-filter.ts`: `!(typeof actual === 'string' && …)`),即它是 `$contains` 的镜像而不是 「取反的副本」—— 非字符串/无值的列不可能包含子串,故成立。driver-sql 的 极性表对同一算子早已如此(`case '$notContains': return true`)。 ⛔ 未动的格子:`$exists` / `$null` / `$eq` 及 `$notContains` 的比较数类型(formula 额外要求 `typeof v === 'string'`,本 PR 不引入 —— 那一格未被裁决)。 真值表(两格 × 无值两形):`$nin` × NULLED 改前就已成立(守卫只拦 `undefined`), 其余三格 false → true;有值行逐条不变。 driver-memory / driver-mongodb 仍是旧答案,因为 #5499 冻结了它们 —— 本文件的 分叉是相对一个被冻结的面,不是相对裁决。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --- ...ing-filter-null-safe-negative-operators.md | 5 + packages/objectql/src/having-filter.test.ts | 120 +++++++++++++++++- packages/objectql/src/having-filter.ts | 60 +++++++-- 3 files changed, 173 insertions(+), 12 deletions(-) create mode 100644 .changeset/having-filter-null-safe-negative-operators.md diff --git a/.changeset/having-filter-null-safe-negative-operators.md b/.changeset/having-filter-null-safe-negative-operators.md new file mode 100644 index 0000000000..904accb00e --- /dev/null +++ b/.changeset/having-filter-null-safe-negative-operators.md @@ -0,0 +1,5 @@ +--- +'@objectstack/objectql': patch +--- + +HAVING 求值对齐 #5298 的 NULL-safe 裁决:聚合行上没有值的列现在满足 `$nin` 与 `$notContains`,与 driver-sql / formula / service-analytics 一致(此前 HAVING 是唯一仍判否的求值面)。 diff --git a/packages/objectql/src/having-filter.test.ts b/packages/objectql/src/having-filter.test.ts index 4e81cdd699..1abad272dd 100644 --- a/packages/objectql/src/having-filter.test.ts +++ b/packages/objectql/src/having-filter.test.ts @@ -4,10 +4,12 @@ * HAVING evaluator (#4286 step 3) — semantics over AGGREGATED rows. * * The namespace is the aggregated row's own columns (aggregation aliases + - * groupBy projections); operator semantics mirror the Filter Protocol's - * memory evaluation, EXCEPT that an unknown operator throws — ignoring one - * would silently return unfiltered aggregates, the exact silently-inert - * failure (#4286, ADR-0078) enforcement exists to end. + * groupBy projections); operator semantics follow the Filter Protocol, with two + * deliberate divergences from driver-memory's matcher: an unknown operator + * throws — ignoring one would silently return unfiltered aggregates, the exact + * silently-inert failure (#4286, ADR-0078) enforcement exists to end — and the + * negation-carrying operators are NULL-safe per #5298 (see the grid at the + * bottom of this file, #5905). */ import { describe, it, expect } from 'vitest'; @@ -78,3 +80,113 @@ describe('matchesHaving — the unknown-operator refusal', () => { expect(matchesHaving({ k: 'Alpha' }, { k: { $regex: '^alp', $options: 'i' } })).toBe(true); }); }); + +/** + * [#5905] The no-value grid for the negation-carrying operators. + * + * #5298 ruled (option A, 2026-08-06) that "the column has no value" SATISFIES a + * test for "not this value", and PR #5962 landed it on driver-sql, formula, + * service-analytics and the `FILTER_LOGIC_*` conformance table. HAVING is the + * fifth evaluation face of the same vocabulary and was not in that PR's + * inventory, so it stayed the lone holdout — and no conformance table would + * have caught it, because `FILTER_LOGIC_CASES` does not drive the HAVING path + * (verified: `packages/objectql` imports it nowhere). This grid IS that + * coverage. + * + * Two no-value shapes, deliberately separated, because on this face they did + * NOT arrive at the old answer by the same route: + * + * - NULLED — the key is present with `null`. The early-exit guard tests + * `=== undefined`, so it never fired here; `$nin` was already NULL-safe and + * `$notContains` was not (`typeof null !== 'string'` ⇒ judged false). + * - MISSING — the key is absent, so the aggregated row reads `undefined`. The + * early-exit guard fired first and answered false for BOTH operators, before + * either arm was reached. + * + * The positive-operator rows are the control: `$in` / `$contains` must keep + * REJECTING both no-value shapes. Widening the exemption list too far would + * turn them green, which is the failure this pair is here to catch. + */ +describe('no-value rows and the negation-carrying operators (#5905 / #5298 option A)', () => { + const NULLED = { customer_id: 'nulled', tag: null, total: 100 }; + const MISSING = { customer_id: 'missing', total: 100 }; + const VALUED_OUT = { customer_id: 'valued_out', tag: 'gamma', total: 100 }; + const VALUED_IN = { customer_id: 'valued_in', tag: 'alpha', total: 100 }; + const GRID = [NULLED, MISSING, VALUED_OUT, VALUED_IN]; + + describe('$nin', () => { + it('a NULLED column satisfies $nin', () => { + expect(matchesHaving(NULLED, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true); + }); + + it('a MISSING column satisfies $nin', () => { + expect(matchesHaving(MISSING, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true); + }); + + it('a present value OUTSIDE the list still satisfies $nin (unchanged)', () => { + expect(matchesHaving(VALUED_OUT, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true); + }); + + it('a present value INSIDE the list still fails $nin (unchanged)', () => { + expect(matchesHaving(VALUED_IN, { tag: { $nin: ['alpha', 'beta'] } })).toBe(false); + }); + + it('applyHaving keeps both no-value rows and drops only the listed value', () => { + expect(applyHaving(GRID, { tag: { $nin: ['alpha', 'beta'] } }).map((r) => r.customer_id)) + .toEqual(['nulled', 'missing', 'valued_out']); + }); + }); + + describe('$notContains', () => { + it('a NULLED column satisfies $notContains', () => { + expect(matchesHaving(NULLED, { tag: { $notContains: 'lph' } })).toBe(true); + }); + + it('a MISSING column satisfies $notContains', () => { + expect(matchesHaving(MISSING, { tag: { $notContains: 'lph' } })).toBe(true); + }); + + it('a present value WITHOUT the substring still satisfies $notContains (unchanged)', () => { + expect(matchesHaving(VALUED_OUT, { tag: { $notContains: 'lph' } })).toBe(true); + }); + + it('a present value WITH the substring still fails $notContains (unchanged)', () => { + expect(matchesHaving(VALUED_IN, { tag: { $notContains: 'lph' } })).toBe(false); + }); + + it('applyHaving keeps both no-value rows and drops only the containing value', () => { + expect(applyHaving(GRID, { tag: { $notContains: 'lph' } }).map((r) => r.customer_id)) + .toEqual(['nulled', 'missing', 'valued_out']); + }); + }); + + describe('the control: positive operators still reject a no-value column', () => { + it('$in rejects NULLED and MISSING', () => { + expect(matchesHaving(NULLED, { tag: { $in: ['alpha', 'beta'] } })).toBe(false); + expect(matchesHaving(MISSING, { tag: { $in: ['alpha', 'beta'] } })).toBe(false); + }); + + it('$contains rejects NULLED and MISSING', () => { + expect(matchesHaving(NULLED, { tag: { $contains: 'lph' } })).toBe(false); + expect(matchesHaving(MISSING, { tag: { $contains: 'lph' } })).toBe(false); + }); + + it('$ne — already exempt before #5905 — is unchanged for both shapes', () => { + expect(matchesHaving(NULLED, { tag: { $ne: 'alpha' } })).toBe(true); + expect(matchesHaving(MISSING, { tag: { $ne: 'alpha' } })).toBe(true); + expect(matchesHaving(VALUED_IN, { tag: { $ne: 'alpha' } })).toBe(false); + }); + }); + + /** + * The NULL-safety lives at the LEAF, so `$not` inverts it rather than + * inheriting it — the same design driver-sql writes down for its own + * `$not` rewrite (a nested negation totalises its own operand). A no-value + * row satisfies `$nin`, therefore it does NOT satisfy `$not: { $nin }`. + */ + it('$not inverts the leaf answer instead of re-applying the guard', () => { + expect(matchesHaving(MISSING, { $not: { tag: { $nin: ['alpha'] } } })).toBe(false); + expect(matchesHaving(NULLED, { $not: { tag: { $notContains: 'lph' } } })).toBe(false); + expect(matchesHaving(VALUED_IN, { $not: { tag: { $nin: ['alpha'] } } })).toBe(true); + }); +}); diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts index 11da0ca228..9b48372a9d 100644 --- a/packages/objectql/src/having-filter.ts +++ b/packages/objectql/src/having-filter.ts @@ -19,13 +19,27 @@ // item's `alias` for structured entries — after date bucketing). It is an // ordinary FilterCondition over those columns: implicit equality, the // comparison / set / null / existence / string operators, and `$and` / `$or` / -// `$not` composition. Operator semantics mirror the Filter Protocol as -// driver-memory's matcher implements it, with ONE deliberate divergence: +// `$not` composition. Operator semantics follow the Filter Protocol, with TWO +// deliberate divergences from driver-memory's matcher — the face this module +// was originally written against: // -// AN UNKNOWN OPERATOR THROWS. The memory matcher ignores operators it does not -// know; here an ignored operator would silently return UNFILTERED aggregates — -// the precise failure mode (#4286, ADR-0078) this module exists to end. The -// rejection names the operator and the supported set. +// 1. AN UNKNOWN OPERATOR THROWS. The memory matcher ignores operators it does +// not know; here an ignored operator would silently return UNFILTERED +// aggregates — the precise failure mode (#4286, ADR-0078) this module exists +// to end. The rejection names the operator and the supported set. +// +// 2. [#5905] THE NEGATION-CARRYING OPERATORS ARE NULL-SAFE. `$ne` / `$nin` / +// `$notContains` are satisfied by a row whose column HAS NO VALUE — "the +// column has no value" satisfies a test for "not this value". That is the +// ruling the maintainer took on #5298 (option A, 2026-08-06), landed by +// PR #5962 across driver-sql, formula, service-analytics and the +// `FILTER_LOGIC_*` conformance table. HAVING is the FIFTH evaluation face of +// the same vocabulary and was not in that PR's inventory, which left this +// file as the lone holdout (#5905) — and the only face no conformance table +// covers, since `FILTER_LOGIC_CASES` does not drive the HAVING path. +// driver-memory / driver-mongodb still answer the old way only because +// #5499 freezes them; the divergence is against a frozen face, not against +// the ruling. import type { FilterCondition } from '@objectstack/spec/data'; @@ -49,6 +63,27 @@ function unknownOperator(op: string, where: 'logical' | 'condition'): Error { ); } +/** + * [#5905] Operators whose answer for a column with NO VALUE is decided by the + * operator's own arm below, not by the early exit in {@link checkCondition}. + * + * That exit exists so a POSITIVE test (`$gt`, `$in`, `$contains`, …) can never + * be accidentally satisfied by a column the aggregated row does not carry. The + * operators listed here are the ones for which "no value" is a real answer + * rather than an accident: + * + * - `$exists` / `$null` — answering about absence IS their whole job; + * - `$ne` / `$nin` / `$notContains` — they carry their own negation, and #5298 + * ruled (option A) that a value-less column satisfies them, on every backend. + * + * `$nin` and `$notContains` were missing from this list, which is the defect + * #5905 records: the exit fired first and answered FALSE for them, so the arms + * below — which would have answered TRUE — were never reached. + */ +const NO_VALUE_ANSWERED_BY_OPERATOR: ReadonlySet = new Set([ + '$exists', '$ne', '$null', '$nin', '$notContains', +]); + /** * Filter aggregated rows by the query's `having` condition. An absent or empty * condition returns the rows unchanged (same vacuous-filter convention as @@ -109,7 +144,7 @@ function checkCondition(value: any, condition: any): boolean { for (const op of keys) { if (op === '$options') continue; // consumed by $regex below const target = (condition as Record)[op]; - if (value === undefined && op !== '$exists' && op !== '$ne' && op !== '$null') return false; + if (value === undefined && !NO_VALUE_ANSWERED_BY_OPERATOR.has(op)) return false; switch (op) { // eslint-disable-next-line eqeqeq case '$eq': if (value != target) return false; break; @@ -134,7 +169,16 @@ function checkCondition(value: any, condition: any): boolean { if (target === false && value == null) return false; break; case '$contains': if (typeof value !== 'string' || !value.includes(target)) return false; break; - case '$notContains': if (typeof value !== 'string' || value.includes(target)) return false; break; + // [#5905] The mirror of `$contains`, NOT its copy-with-a-negated-test. + // `$contains` fails a non-string value because "contains" cannot hold for + // something that is not text; `$notContains` SUCCEEDS for the same value + // for the same reason — it cannot contain the substring. Reusing the + // `typeof value !== 'string' ⇒ false` guard here (what this line used to + // do) made a value-less column fail BOTH an operator and its negation, + // the two-valued reading #5298 ruled out. This is `formula`'s shape + // (`matches-filter.ts`: `!(typeof actual === 'string' && …)`), which + // driver-sql's polarity table already follows for the same operator. + case '$notContains': if (typeof value === 'string' && value.includes(target)) return false; break; case '$startsWith': if (typeof value !== 'string' || !value.startsWith(target)) return false; break; case '$endsWith': if (typeof value !== 'string' || !value.endsWith(target)) return false; break; case '$regex': {