Skip to content

Commit f09a2e7

Browse files
baozhoutaoclaude
andauthored
fix(objectql): HAVING 的 $nin / $notContains 对无值行 NULL-safe (#5905) (#6446)
#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 冻结了它们 —— 本文件的 分叉是相对一个被冻结的面,不是相对裁决。 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent 49f208b commit f09a2e7

3 files changed

Lines changed: 173 additions & 12 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
HAVING 求值对齐 #5298 的 NULL-safe 裁决:聚合行上没有值的列现在满足 `$nin``$notContains`,与 driver-sql / formula / service-analytics 一致(此前 HAVING 是唯一仍判否的求值面)。

packages/objectql/src/having-filter.test.ts

Lines changed: 116 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
* HAVING evaluator (#4286 step 3) — semantics over AGGREGATED rows.
55
*
66
* The namespace is the aggregated row's own columns (aggregation aliases +
7-
* groupBy projections); operator semantics mirror the Filter Protocol's
8-
* memory evaluation, EXCEPT that an unknown operator throws — ignoring one
9-
* would silently return unfiltered aggregates, the exact silently-inert
10-
* failure (#4286, ADR-0078) enforcement exists to end.
7+
* groupBy projections); operator semantics follow the Filter Protocol, with two
8+
* deliberate divergences from driver-memory's matcher: an unknown operator
9+
* throws — ignoring one would silently return unfiltered aggregates, the exact
10+
* silently-inert failure (#4286, ADR-0078) enforcement exists to end — and the
11+
* negation-carrying operators are NULL-safe per #5298 (see the grid at the
12+
* bottom of this file, #5905).
1113
*/
1214

1315
import { describe, it, expect } from 'vitest';
@@ -78,3 +80,113 @@ describe('matchesHaving — the unknown-operator refusal', () => {
7880
expect(matchesHaving({ k: 'Alpha' }, { k: { $regex: '^alp', $options: 'i' } })).toBe(true);
7981
});
8082
});
83+
84+
/**
85+
* [#5905] The no-value grid for the negation-carrying operators.
86+
*
87+
* #5298 ruled (option A, 2026-08-06) that "the column has no value" SATISFIES a
88+
* test for "not this value", and PR #5962 landed it on driver-sql, formula,
89+
* service-analytics and the `FILTER_LOGIC_*` conformance table. HAVING is the
90+
* fifth evaluation face of the same vocabulary and was not in that PR's
91+
* inventory, so it stayed the lone holdout — and no conformance table would
92+
* have caught it, because `FILTER_LOGIC_CASES` does not drive the HAVING path
93+
* (verified: `packages/objectql` imports it nowhere). This grid IS that
94+
* coverage.
95+
*
96+
* Two no-value shapes, deliberately separated, because on this face they did
97+
* NOT arrive at the old answer by the same route:
98+
*
99+
* - NULLED — the key is present with `null`. The early-exit guard tests
100+
* `=== undefined`, so it never fired here; `$nin` was already NULL-safe and
101+
* `$notContains` was not (`typeof null !== 'string'` ⇒ judged false).
102+
* - MISSING — the key is absent, so the aggregated row reads `undefined`. The
103+
* early-exit guard fired first and answered false for BOTH operators, before
104+
* either arm was reached.
105+
*
106+
* The positive-operator rows are the control: `$in` / `$contains` must keep
107+
* REJECTING both no-value shapes. Widening the exemption list too far would
108+
* turn them green, which is the failure this pair is here to catch.
109+
*/
110+
describe('no-value rows and the negation-carrying operators (#5905 / #5298 option A)', () => {
111+
const NULLED = { customer_id: 'nulled', tag: null, total: 100 };
112+
const MISSING = { customer_id: 'missing', total: 100 };
113+
const VALUED_OUT = { customer_id: 'valued_out', tag: 'gamma', total: 100 };
114+
const VALUED_IN = { customer_id: 'valued_in', tag: 'alpha', total: 100 };
115+
const GRID = [NULLED, MISSING, VALUED_OUT, VALUED_IN];
116+
117+
describe('$nin', () => {
118+
it('a NULLED column satisfies $nin', () => {
119+
expect(matchesHaving(NULLED, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true);
120+
});
121+
122+
it('a MISSING column satisfies $nin', () => {
123+
expect(matchesHaving(MISSING, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true);
124+
});
125+
126+
it('a present value OUTSIDE the list still satisfies $nin (unchanged)', () => {
127+
expect(matchesHaving(VALUED_OUT, { tag: { $nin: ['alpha', 'beta'] } })).toBe(true);
128+
});
129+
130+
it('a present value INSIDE the list still fails $nin (unchanged)', () => {
131+
expect(matchesHaving(VALUED_IN, { tag: { $nin: ['alpha', 'beta'] } })).toBe(false);
132+
});
133+
134+
it('applyHaving keeps both no-value rows and drops only the listed value', () => {
135+
expect(applyHaving(GRID, { tag: { $nin: ['alpha', 'beta'] } }).map((r) => r.customer_id))
136+
.toEqual(['nulled', 'missing', 'valued_out']);
137+
});
138+
});
139+
140+
describe('$notContains', () => {
141+
it('a NULLED column satisfies $notContains', () => {
142+
expect(matchesHaving(NULLED, { tag: { $notContains: 'lph' } })).toBe(true);
143+
});
144+
145+
it('a MISSING column satisfies $notContains', () => {
146+
expect(matchesHaving(MISSING, { tag: { $notContains: 'lph' } })).toBe(true);
147+
});
148+
149+
it('a present value WITHOUT the substring still satisfies $notContains (unchanged)', () => {
150+
expect(matchesHaving(VALUED_OUT, { tag: { $notContains: 'lph' } })).toBe(true);
151+
});
152+
153+
it('a present value WITH the substring still fails $notContains (unchanged)', () => {
154+
expect(matchesHaving(VALUED_IN, { tag: { $notContains: 'lph' } })).toBe(false);
155+
});
156+
157+
it('applyHaving keeps both no-value rows and drops only the containing value', () => {
158+
expect(applyHaving(GRID, { tag: { $notContains: 'lph' } }).map((r) => r.customer_id))
159+
.toEqual(['nulled', 'missing', 'valued_out']);
160+
});
161+
});
162+
163+
describe('the control: positive operators still reject a no-value column', () => {
164+
it('$in rejects NULLED and MISSING', () => {
165+
expect(matchesHaving(NULLED, { tag: { $in: ['alpha', 'beta'] } })).toBe(false);
166+
expect(matchesHaving(MISSING, { tag: { $in: ['alpha', 'beta'] } })).toBe(false);
167+
});
168+
169+
it('$contains rejects NULLED and MISSING', () => {
170+
expect(matchesHaving(NULLED, { tag: { $contains: 'lph' } })).toBe(false);
171+
expect(matchesHaving(MISSING, { tag: { $contains: 'lph' } })).toBe(false);
172+
});
173+
174+
it('$ne — already exempt before #5905 — is unchanged for both shapes', () => {
175+
expect(matchesHaving(NULLED, { tag: { $ne: 'alpha' } })).toBe(true);
176+
expect(matchesHaving(MISSING, { tag: { $ne: 'alpha' } })).toBe(true);
177+
expect(matchesHaving(VALUED_IN, { tag: { $ne: 'alpha' } })).toBe(false);
178+
});
179+
});
180+
181+
/**
182+
* The NULL-safety lives at the LEAF, so `$not` inverts it rather than
183+
* inheriting it — the same design driver-sql writes down for its own
184+
* `$not` rewrite (a nested negation totalises its own operand). A no-value
185+
* row satisfies `$nin`, therefore it does NOT satisfy `$not: { $nin }`.
186+
*/
187+
it('$not inverts the leaf answer instead of re-applying the guard', () => {
188+
expect(matchesHaving(MISSING, { $not: { tag: { $nin: ['alpha'] } } })).toBe(false);
189+
expect(matchesHaving(NULLED, { $not: { tag: { $notContains: 'lph' } } })).toBe(false);
190+
expect(matchesHaving(VALUED_IN, { $not: { tag: { $nin: ['alpha'] } } })).toBe(true);
191+
});
192+
});

packages/objectql/src/having-filter.ts

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,27 @@
1919
// item's `alias` for structured entries — after date bucketing). It is an
2020
// ordinary FilterCondition over those columns: implicit equality, the
2121
// comparison / set / null / existence / string operators, and `$and` / `$or` /
22-
// `$not` composition. Operator semantics mirror the Filter Protocol as
23-
// driver-memory's matcher implements it, with ONE deliberate divergence:
22+
// `$not` composition. Operator semantics follow the Filter Protocol, with TWO
23+
// deliberate divergences from driver-memory's matcher — the face this module
24+
// was originally written against:
2425
//
25-
// AN UNKNOWN OPERATOR THROWS. The memory matcher ignores operators it does not
26-
// know; here an ignored operator would silently return UNFILTERED aggregates —
27-
// the precise failure mode (#4286, ADR-0078) this module exists to end. The
28-
// rejection names the operator and the supported set.
26+
// 1. AN UNKNOWN OPERATOR THROWS. The memory matcher ignores operators it does
27+
// not know; here an ignored operator would silently return UNFILTERED
28+
// aggregates — the precise failure mode (#4286, ADR-0078) this module exists
29+
// to end. The rejection names the operator and the supported set.
30+
//
31+
// 2. [#5905] THE NEGATION-CARRYING OPERATORS ARE NULL-SAFE. `$ne` / `$nin` /
32+
// `$notContains` are satisfied by a row whose column HAS NO VALUE — "the
33+
// column has no value" satisfies a test for "not this value". That is the
34+
// ruling the maintainer took on #5298 (option A, 2026-08-06), landed by
35+
// PR #5962 across driver-sql, formula, service-analytics and the
36+
// `FILTER_LOGIC_*` conformance table. HAVING is the FIFTH evaluation face of
37+
// the same vocabulary and was not in that PR's inventory, which left this
38+
// file as the lone holdout (#5905) — and the only face no conformance table
39+
// covers, since `FILTER_LOGIC_CASES` does not drive the HAVING path.
40+
// driver-memory / driver-mongodb still answer the old way only because
41+
// #5499 freezes them; the divergence is against a frozen face, not against
42+
// the ruling.
2943

3044
import type { FilterCondition } from '@objectstack/spec/data';
3145

@@ -49,6 +63,27 @@ function unknownOperator(op: string, where: 'logical' | 'condition'): Error {
4963
);
5064
}
5165

66+
/**
67+
* [#5905] Operators whose answer for a column with NO VALUE is decided by the
68+
* operator's own arm below, not by the early exit in {@link checkCondition}.
69+
*
70+
* That exit exists so a POSITIVE test (`$gt`, `$in`, `$contains`, …) can never
71+
* be accidentally satisfied by a column the aggregated row does not carry. The
72+
* operators listed here are the ones for which "no value" is a real answer
73+
* rather than an accident:
74+
*
75+
* - `$exists` / `$null` — answering about absence IS their whole job;
76+
* - `$ne` / `$nin` / `$notContains` — they carry their own negation, and #5298
77+
* ruled (option A) that a value-less column satisfies them, on every backend.
78+
*
79+
* `$nin` and `$notContains` were missing from this list, which is the defect
80+
* #5905 records: the exit fired first and answered FALSE for them, so the arms
81+
* below — which would have answered TRUE — were never reached.
82+
*/
83+
const NO_VALUE_ANSWERED_BY_OPERATOR: ReadonlySet<string> = new Set([
84+
'$exists', '$ne', '$null', '$nin', '$notContains',
85+
]);
86+
5287
/**
5388
* Filter aggregated rows by the query's `having` condition. An absent or empty
5489
* condition returns the rows unchanged (same vacuous-filter convention as
@@ -109,7 +144,7 @@ function checkCondition(value: any, condition: any): boolean {
109144
for (const op of keys) {
110145
if (op === '$options') continue; // consumed by $regex below
111146
const target = (condition as Record<string, any>)[op];
112-
if (value === undefined && op !== '$exists' && op !== '$ne' && op !== '$null') return false;
147+
if (value === undefined && !NO_VALUE_ANSWERED_BY_OPERATOR.has(op)) return false;
113148
switch (op) {
114149
// eslint-disable-next-line eqeqeq
115150
case '$eq': if (value != target) return false; break;
@@ -134,7 +169,16 @@ function checkCondition(value: any, condition: any): boolean {
134169
if (target === false && value == null) return false;
135170
break;
136171
case '$contains': if (typeof value !== 'string' || !value.includes(target)) return false; break;
137-
case '$notContains': if (typeof value !== 'string' || value.includes(target)) return false; break;
172+
// [#5905] The mirror of `$contains`, NOT its copy-with-a-negated-test.
173+
// `$contains` fails a non-string value because "contains" cannot hold for
174+
// something that is not text; `$notContains` SUCCEEDS for the same value
175+
// for the same reason — it cannot contain the substring. Reusing the
176+
// `typeof value !== 'string' ⇒ false` guard here (what this line used to
177+
// do) made a value-less column fail BOTH an operator and its negation,
178+
// the two-valued reading #5298 ruled out. This is `formula`'s shape
179+
// (`matches-filter.ts`: `!(typeof actual === 'string' && …)`), which
180+
// driver-sql's polarity table already follows for the same operator.
181+
case '$notContains': if (typeof value === 'string' && value.includes(target)) return false; break;
138182
case '$startsWith': if (typeof value !== 'string' || !value.startsWith(target)) return false; break;
139183
case '$endsWith': if (typeof value !== 'string' || !value.endsWith(target)) return false; break;
140184
case '$regex': {

0 commit comments

Comments
 (0)