Skip to content

Commit 5aae790

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): $not 取反前先把操作数编译成全域谓词,NULL 行不再被静默排除 (#5146) (#5296)
SQL 是三值逻辑:`NULL = 'won'` 是 UNKNOWN,`NOT UNKNOWN` 仍是 UNKNOWN,而 `WHERE` 只保留 TRUE。于是 `applyFilterCondition` 发出的裸 `not (stage = 'won')` 把「该列没有 值」的行整批丢掉,而 `driver-memory` 的 `match()` 与 `formula` 的 `matchesFilterCondition` 用普通两值 JS 求值(`undefined !== 'won'` → 行匹配),把同 一批行**全部返回**。一个 spec 声明的算子,答案取决于跑它的是哪个驱动 —— 而权限规则 里的 CEL `!expr` 经 `cel-to-filter.ts` 正是降解成 `{ $not: {…} }`,所以同一条 read scope 在不同后端准入的行集不同。#5146 判定以 JS 家族的答案为准(2:1 多数派),本次 把 SQL 侧对齐。 `$not` 的操作数在取反前经 `nullSafeNegationOperand` 改写,使每个叶子都编译成**全域 (total)**谓词 —— 永远 TRUE 或 FALSE,不会 UNKNOWN: -- 之前:not (`stage` = 'won') -- 现在:not ((`stage` is not null) and (`stage` = 'won')) 对 issue 给出的扁平形状,这与 `NOT (…) OR col IS NULL` 等价。守卫下推到**叶子**而非 挂在 `NOT` 旁边,是为了在操作数嵌套时仍然正确:`$not` 内含 `$or` 时,顶层的 `OR col IS NULL` 会把 JS 家族排除的行(某列为 NULL、但另一分支成立)重新放进来。 守卫方向按算子逐个判定,不是一刀切:`$ne` / `$nin` / `$notContains` 走 `col IS NULL OR (…)`,`$eq` / `$in` / `$gt` / `$contains` 一族走 `col IS NOT NULL AND (…)`,`$null` / `$exists` / `$eq: null` / `$ne: null` 本就是全域 谓词、一个字节不加。无条件加 `OR col IS NULL` 会把 `{$not: {a: {$ne: 5}}}`(语义是 「a 就是 5」)静默放松成也返回 NULL 行 —— 正是 #2704 / #5134 那一族缺陷。 只有 `$not` 路径被改写:普通比较的 SQL 逐字符不变,没有非否定谓词因此失去索引。 #5134 / PR #5243 的布尔单位元与 ADR-0112 形状拒收全部保持;`{ field: {} }`(#5240) 刻意不在此裁定,编译结果与之前完全一致。 `driver-memory` 与 `formula` 无需改动 —— 三家各补一组 pin 测试,把「值缺失行在 `$not` 下的去留」钉在一起,并把两个 JS 后端彼此不一致的角落(`$notContains`、 `$exists`、缺键 `$nin`)按实测钉住而不是假装一致。 Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent f8cfbb4 commit 5aae790

6 files changed

Lines changed: 836 additions & 5 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
fix(driver-sql): `$not` 改为 NULL-safe —— 被比较列为 NULL 的行不再被否定条件静默排除
6+
7+
**这是一处可观察的查询行为变更,且直接关系到 RLS 的可见集合。**
8+
`{ $not: { stage: 'won' } }` 以前**不返回** `stage IS NULL` 的行,现在**返回**它们。
9+
如果你的规则依赖了旧行为,它依赖的是「同一条规则在不同后端给出不同可见集合」。
10+
11+
SQL 是三值逻辑:`NULL = 'won'` 是 UNKNOWN,`NOT UNKNOWN` 仍是 UNKNOWN,而 `WHERE`
12+
只保留 TRUE。于是 `applyFilterCondition` 编译出的裸 `NOT (stage = 'won')` 会把
13+
「该列没有值」的行整批丢掉;同一条 filter 在 `driver-memory``formula`
14+
`matchesFilterCondition` 上是普通的两值 JS 求值(`undefined !== 'won'` → 行匹配),
15+
两边把这些行**都返回**。一个 spec 声明的算子,答案取决于跑它的是哪个驱动。
16+
17+
这不是「数目对不上」而已:权限规则里的 CEL `!expr``cel-to-filter.ts` 正是降解成
18+
`{ $not: {…} }`,所以同一条 read scope 在 SQL 数据源与内存数据源上准入的行集不同。
19+
#5146 判定以 JS 家族的答案为准(2:1 的多数派;写 `!(stage == 'won')` 的人不会预期
20+
「stage 为空的行被隐藏」),本次把 SQL 侧对齐过去。
21+
22+
**编译出来的形状。** `$not` 的操作数在取反之前先被改写成**全域(total)谓词** ——
23+
永远是 TRUE 或 FALSE,不会是 UNKNOWN:
24+
25+
```sql
26+
-- 之前
27+
not (`stage` = 'won')
28+
-- 现在
29+
not ((`stage` is not null) and (`stage` = 'won'))
30+
```
31+
32+
对 issue 里给出的扁平形状,这与 `NOT (…) OR col IS NULL` 完全等价。把守卫下推到
33+
**每个叶子**而不是挂在 `NOT` 旁边,是为了在操作数嵌套时仍然正确:`$not` 里套一个
34+
`$or` 时,顶层的 `OR col IS NULL` 会把 JS 家族排除的行重新放进来(某一列为 NULL、
35+
但另一个析取分支成立的行)。
36+
37+
**守卫方向按算子逐个判定,不是一刀切。** `{ $not: { a: { $ne: 5 } } }` 的语义是
38+
「a 就是 5」,两个 JS 后端都把 NULL 行排除在外;无条件加 `OR a IS NULL` 会把这些行
39+
交回去 —— 正是本驱动反复付过学费的静默放松(#2704 / #5134)。因此
40+
`$ne` / `$nin` / `$notContains` 用的是 `col IS NULL OR (…)`,`$eq` / `$in` /
41+
`$gt` / `$contains` 一族用 `col IS NOT NULL AND (…)`,而 `$null` / `$exists` /
42+
`$eq: null` / `$ne: null` 本来就是全域谓词,一个字节都不加。
43+
44+
**只有 `$not` 路径被改写。** 普通比较的 SQL 逐字符不变(`{ a: 1 }` 仍然是
45+
`a = 1`),因此没有任何非否定谓词因此失去索引;`$not` 路径上的 `IS NOT NULL` 守卫
46+
本身处在一个原本就不可 sargable 的 `NOT (…)` 里。
47+
48+
`#5134` / PR #5243 定下的布尔单位元(`{ $not: {} }` → 零行、`$not` of FALSE →
49+
全部行、非 filter 节点的操作数按 ADR-0112 响亮拒收)全部保持不变;`{ field: {} }`
50+
(#5240)也刻意不在此裁定 —— 它编译出的 SQL 与之前完全一致。
51+
52+
`driver-memory``formula` 无需改动,本次为三家各补了一组 pin 测试,把「值缺失
53+
行在 `$not` 下的去留」钉在一起。跨驱动 conformance case(`FILTER_LOGIC_CASES`)与
54+
契约 TSDoc 归 spec 车道,随 #5239 落地。
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5146] `$not` and absent values — the answers this evaluator gives, pinned.
5+
*
6+
* `matchesFilterCondition` needed no change for #5146: it already negates in
7+
* ordinary two-valued JS, so `{ $not: { stage: 'won' } }` matches a record whose
8+
* `stage` is null or missing. `driver-sql` used to disagree (SQL's
9+
* `NOT (stage = 'won')` is UNKNOWN for a NULL column, and a `WHERE` drops it),
10+
* so the SAME rule admitted different rows per backend — and this evaluator is
11+
* the RLS write-side `check`, i.e. the half that decides whether a write is
12+
* allowed, against read scopes compiled elsewhere. #5146 ruled this answer
13+
* canonical and rewrote the SQL compiler to match it.
14+
*
15+
* These cases are therefore a PIN on the reference behaviour, mirrored id-for-id
16+
* by `driver-sql`'s `sql-driver-not-null-safe.test.ts` and `driver-memory`'s
17+
* `memory-matcher-not-null-safe.test.ts`. Moving an expectation here silently
18+
* re-opens the divergence.
19+
*
20+
* `cel-to-filter.ts` is why this matters in practice: a CEL `!expr` in a
21+
* permission rule lowers to exactly these `$not` shapes.
22+
*
23+
* Home for these eventually: `FILTER_LOGIC_CASES` in `@objectstack/spec/data`
24+
* (spec lane, with #5239).
25+
*/
26+
27+
import { describe, it, expect } from 'vitest';
28+
import { matchesFilterCondition } from './matches-filter.js';
29+
import type { FilterCondition } from '@objectstack/spec/data';
30+
31+
/** Fields present but null — how a SQL NULL round-trips into a record. */
32+
const NULLED: Array<Record<string, unknown>> = [
33+
{ id: '1', stage: 'won', owner: 'u1', amount: 10 },
34+
{ id: '2', stage: 'lost', owner: 'u2', amount: 20 },
35+
{ id: '3', stage: null, owner: 'u1', amount: null },
36+
{ id: '4', stage: null, owner: null, amount: 40 },
37+
];
38+
39+
/** The same rows with the null fields ABSENT — a partial write's post-image. */
40+
const MISSING: Array<Record<string, unknown>> = [
41+
{ id: '1', stage: 'won', owner: 'u1', amount: 10 },
42+
{ id: '2', stage: 'lost', owner: 'u2', amount: 20 },
43+
{ id: '3', owner: 'u1' },
44+
{ id: '4', amount: 40 },
45+
];
46+
47+
const ALL = ['1', '2', '3', '4'];
48+
49+
const ids = (rows: Array<Record<string, unknown>>, filter: unknown): string[] =>
50+
rows.filter((r) => matchesFilterCondition(r, filter as FilterCondition)).map((r) => String(r.id));
51+
52+
/** Both readings of "no value" must give the same answer unless noted. */
53+
const matched = (filter: unknown): string[] => {
54+
const nulled = ids(NULLED, filter);
55+
expect(ids(MISSING, filter), 'a null field and an absent field must match alike').toEqual(nulled);
56+
return nulled;
57+
};
58+
59+
describe('[#5146] matchesFilterCondition — $not over records with no value', () => {
60+
describe('a record with no value does not satisfy the negated condition', () => {
61+
it('$not on an implicit equality matches the value-less records', () => {
62+
expect(matched({ $not: { stage: 'won' } })).toEqual(['2', '3', '4']);
63+
});
64+
65+
it('$not over multiple keys matches a record missing EITHER', () => {
66+
expect(matched({ $not: { stage: 'won', owner: 'u1' } })).toEqual(['2', '3', '4']);
67+
});
68+
69+
it('the RLS shape: a CEL `!(stage == "won")` check keeps stage-less records', () => {
70+
expect(matched({ $not: { stage: 'won' } })).toHaveLength(3);
71+
});
72+
});
73+
74+
describe('nesting', () => {
75+
it('$not of a $or rejects a value-less record whose OTHER branch matches', () => {
76+
// Record 3 has no stage but owner = 'u1', so the $or holds and the
77+
// negation rejects it. This is the case that forced `driver-sql` to put
78+
// its NULL guard on each leaf rather than beside the `NOT`.
79+
expect(matched({ $not: { $or: [{ stage: 'won' }, { owner: 'u1' }] } })).toEqual(['2', '4']);
80+
});
81+
82+
it('$not of a $and matches every record failing either conjunct', () => {
83+
expect(matched({ $not: { $and: [{ stage: 'won' }, { owner: 'u1' }] } })).toEqual(['2', '3', '4']);
84+
});
85+
86+
it('a double negation is the positive filter again', () => {
87+
expect(matched({ $not: { $not: { stage: 'won' } } })).toEqual(['1']);
88+
expect(matched({ $not: { $not: { stage: 'won' } } })).toEqual(matched({ stage: 'won' }));
89+
});
90+
91+
it('$not ANDs with its sibling keys', () => {
92+
expect(matched({ $not: { stage: 'won' }, owner: 'u1' })).toEqual(['3']);
93+
});
94+
});
95+
96+
describe('operator polarity — a negation is not a blanket "and also the empty ones"', () => {
97+
it('$not of $ne still means "the field IS that value"', () => {
98+
expect(matched({ $not: { stage: { $ne: 'won' } } })).toEqual(['1']);
99+
});
100+
101+
it('$not of $nin still means "the field IS among them"', () => {
102+
expect(matched({ $not: { stage: { $nin: ['won'] } } })).toEqual(['1']);
103+
});
104+
105+
it('$not of $in matches the value-less records', () => {
106+
expect(matched({ $not: { stage: { $in: ['won'] } } })).toEqual(['2', '3', '4']);
107+
});
108+
109+
it('$not of an ordering comparison matches the value-less records', () => {
110+
expect(matched({ $not: { amount: { $gt: 15 } } })).toEqual(['1', '3']);
111+
});
112+
113+
it('$not of $contains matches the value-less records', () => {
114+
expect(matched({ $not: { stage: { $contains: 'w' } } })).toEqual(['2', '3', '4']);
115+
});
116+
117+
it('$not of $notContains does NOT match them — the mirror case', () => {
118+
// A value-less field satisfies `$notContains` here, so the negation
119+
// rejects it. `driver-sql` follows this answer; `driver-memory` answers
120+
// the opposite for a null-valued field, which is filed on its own.
121+
expect(matched({ $not: { stage: { $notContains: 'w' } } })).toEqual(['1']);
122+
});
123+
124+
it('$not of a null predicate', () => {
125+
expect(matched({ $not: { stage: { $null: true } } })).toEqual(['1', '2']);
126+
expect(matched({ $not: { stage: { $null: false } } })).toEqual(['3', '4']);
127+
expect(matched({ $not: { stage: null } })).toEqual(['1', '2']);
128+
expect(matched({ $not: { stage: { $eq: null } } })).toEqual(['1', '2']);
129+
});
130+
});
131+
132+
describe('the boolean identities still hold here too (#5134)', () => {
133+
it('$not: {} matches nothing — NOT TRUE ≡ FALSE', () => {
134+
expect(matched({ $not: {} })).toEqual([]);
135+
});
136+
137+
it('$not of an empty $or matches everything', () => {
138+
expect(matched({ $not: { $or: [] } })).toEqual(ALL);
139+
});
140+
});
141+
142+
// ── Where this evaluator and `driver-memory` disagree — pinned, not fixed ──
143+
144+
describe('known disagreement with driver-memory (NOT ruled on by #5146)', () => {
145+
it('$exists reads "the key is present", so a null value EXISTS', () => {
146+
// `driver-memory` reads `$exists` as "has a value", so it answers the
147+
// opposite for a present-but-null field. `driver-sql` cannot tell the two
148+
// apart at all (a NULL column is a NULL column) and keeps its existing
149+
// `IS NOT NULL` compilation.
150+
expect(ids(NULLED, { $not: { stage: { $exists: true } } })).toEqual([]);
151+
expect(ids(MISSING, { $not: { stage: { $exists: true } } })).toEqual(['3', '4']);
152+
});
153+
});
154+
});
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5146] `$not` and absent values — the answers this matcher gives, pinned.
5+
*
6+
* This backend needed no change for #5146: it already evaluates a negation in
7+
* ordinary two-valued JS, so `{ $not: { stage: 'won' } }` matches a record whose
8+
* `stage` is null or missing (`undefined !== 'won'`). `driver-sql` used to
9+
* DISAGREE — SQL's `NOT (stage = 'won')` is UNKNOWN for a NULL column and a
10+
* `WHERE` drops it — which meant one CEL `!expr` permission rule admitted a
11+
* different set of rows depending on which driver ran it. #5146 ruled this
12+
* backend's answer canonical (it is the 2:1 majority with `formula`) and
13+
* `driver-sql` was rewritten to match.
14+
*
15+
* So these cases are a PIN, not a change: they are the reference the SQL
16+
* compiler was aligned to, and `sql-driver-not-null-safe.test.ts` asserts the
17+
* same ids over the same fixture. Changing an expectation here silently
18+
* re-opens the divergence — the point of writing them down is that the next
19+
* edit has to move both files, deliberately.
20+
*
21+
* Where this matcher and `formula`'s `matchesFilterCondition` disagree, the case
22+
* says so and pins what each actually answers rather than pretending to a
23+
* consensus; those disagreements are filed separately and are NOT what #5146
24+
* ruled on.
25+
*
26+
* Home for these eventually: `FILTER_LOGIC_CASES` in `@objectstack/spec/data`,
27+
* so all five backends are held to one table (spec lane, with #5239).
28+
*/
29+
30+
import { describe, it, expect } from 'vitest';
31+
import { match } from './memory-matcher.js';
32+
33+
/** Fields present but null — how a SQL NULL round-trips into a record. */
34+
const NULLED: Array<Record<string, unknown>> = [
35+
{ id: '1', stage: 'won', owner: 'u1', amount: 10 },
36+
{ id: '2', stage: 'lost', owner: 'u2', amount: 20 },
37+
{ id: '3', stage: null, owner: 'u1', amount: null },
38+
{ id: '4', stage: null, owner: null, amount: 40 },
39+
];
40+
41+
/** The same rows with the null fields ABSENT — the shape a partial write leaves. */
42+
const MISSING: Array<Record<string, unknown>> = [
43+
{ id: '1', stage: 'won', owner: 'u1', amount: 10 },
44+
{ id: '2', stage: 'lost', owner: 'u2', amount: 20 },
45+
{ id: '3', owner: 'u1' },
46+
{ id: '4', amount: 40 },
47+
];
48+
49+
const ALL = ['1', '2', '3', '4'];
50+
51+
const ids = (rows: Array<Record<string, unknown>>, filter: unknown): string[] =>
52+
rows.filter((r) => match(r, filter)).map((r) => String(r.id));
53+
54+
/** Both readings of "no value" must give the same answer unless noted. */
55+
const matched = (filter: unknown): string[] => {
56+
const nulled = ids(NULLED, filter);
57+
expect(ids(MISSING, filter), 'a null field and an absent field must match alike').toEqual(nulled);
58+
return nulled;
59+
};
60+
61+
describe('[#5146] memory-matcher — $not over records with no value', () => {
62+
describe('a record with no value does not satisfy the negated condition', () => {
63+
it('$not on an implicit equality matches the value-less records', () => {
64+
expect(matched({ $not: { stage: 'won' } })).toEqual(['2', '3', '4']);
65+
});
66+
67+
it('$not over multiple keys matches a record missing EITHER', () => {
68+
expect(matched({ $not: { stage: 'won', owner: 'u1' } })).toEqual(['2', '3', '4']);
69+
});
70+
71+
it('the RLS shape: a CEL `!(stage == "won")` scope keeps stage-less records', () => {
72+
expect(matched({ $not: { stage: 'won' } })).toHaveLength(3);
73+
});
74+
});
75+
76+
describe('nesting', () => {
77+
it('$not of a $or rejects a value-less record whose OTHER branch matches', () => {
78+
// Record 3 has no stage but owner = 'u1', so the $or holds and the
79+
// negation must reject it. This is the case that forced `driver-sql` to
80+
// compile its NULL guard onto each leaf instead of beside the `NOT`.
81+
expect(matched({ $not: { $or: [{ stage: 'won' }, { owner: 'u1' }] } })).toEqual(['2', '4']);
82+
});
83+
84+
it('$not of a $and matches every record failing either conjunct', () => {
85+
expect(matched({ $not: { $and: [{ stage: 'won' }, { owner: 'u1' }] } })).toEqual(['2', '3', '4']);
86+
});
87+
88+
it('a double negation is the positive filter again', () => {
89+
expect(matched({ $not: { $not: { stage: 'won' } } })).toEqual(['1']);
90+
expect(matched({ $not: { $not: { stage: 'won' } } })).toEqual(matched({ stage: 'won' }));
91+
});
92+
93+
it('$not ANDs with its sibling keys', () => {
94+
expect(matched({ $not: { stage: 'won' }, owner: 'u1' })).toEqual(['3']);
95+
});
96+
});
97+
98+
describe('operator polarity — a negation is not a blanket "and also the empty ones"', () => {
99+
it('$not of $ne still means "the field IS that value"', () => {
100+
expect(matched({ $not: { stage: { $ne: 'won' } } })).toEqual(['1']);
101+
});
102+
103+
it('$not of $in matches the value-less records', () => {
104+
expect(matched({ $not: { stage: { $in: ['won'] } } })).toEqual(['2', '3', '4']);
105+
});
106+
107+
it('$not of an ordering comparison matches the value-less records', () => {
108+
expect(matched({ $not: { amount: { $gt: 15 } } })).toEqual(['1', '3']);
109+
});
110+
111+
it('$not of $contains matches the value-less records', () => {
112+
expect(matched({ $not: { stage: { $contains: 'w' } } })).toEqual(['2', '3', '4']);
113+
});
114+
115+
it('$not of a null predicate', () => {
116+
expect(matched({ $not: { stage: { $null: true } } })).toEqual(['1', '2']);
117+
expect(matched({ $not: { stage: { $null: false } } })).toEqual(['3', '4']);
118+
});
119+
});
120+
121+
describe('the boolean identities still hold here too (#5134)', () => {
122+
it('$not: {} matches nothing — NOT TRUE ≡ FALSE', () => {
123+
expect(matched({ $not: {} })).toEqual([]);
124+
});
125+
126+
it('$not of an empty $or matches everything', () => {
127+
expect(matched({ $not: { $or: [] } })).toEqual(ALL);
128+
});
129+
});
130+
131+
// ── Where this matcher and `formula` disagree — pinned, not harmonised ─────
132+
133+
describe('known disagreements with formula.matchesFilterCondition (NOT ruled on by #5146)', () => {
134+
it('$nin: an ABSENT field is treated differently from a null one', () => {
135+
// The early `value === undefined` guard in `checkCondition` exempts only
136+
// `$exists` / `$ne` / `$null`, so an absent field fails `$nin` outright
137+
// while a null field passes it. `formula` answers "not among" for both.
138+
// Pinned as measured; the ruling belongs to the issue that records it.
139+
expect(ids(NULLED, { $not: { stage: { $nin: ['won'] } } })).toEqual(['1']);
140+
expect(ids(MISSING, { $not: { stage: { $nin: ['won'] } } })).toEqual(['1', '3', '4']);
141+
});
142+
143+
it('$notContains: a value-less field does NOT satisfy it here', () => {
144+
// `typeof null !== 'string'` → false, so the negation matches. `formula`
145+
// answers true for the same record, and `driver-sql` follows `formula`.
146+
expect(matched({ $not: { stage: { $notContains: 'w' } } })).toEqual(['1', '3', '4']);
147+
});
148+
149+
it('$exists: a present-but-null field counts as NOT existing here', () => {
150+
// `formula` reads `$exists` as "the key is present" (a null value exists);
151+
// this matcher reads it as "has a value". Same answer for an absent field,
152+
// different for a null one.
153+
expect(ids(NULLED, { $not: { stage: { $exists: true } } })).toEqual(['3', '4']);
154+
expect(ids(MISSING, { $not: { stage: { $exists: true } } })).toEqual(['3', '4']);
155+
});
156+
});
157+
});

0 commit comments

Comments
 (0)