From 5bd70892a8ed7a64918db2d7a2c0a84e9380c3e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 13:00:54 +0000 Subject: [PATCH] fix(drivers,analytics,formula): $ne / $nin / $notContains NULL-safe outside $not (#5298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the #5146 ruling from `$not` to the three operators that carry their own negation. SQL is three-valued, so a bare `col <> ?` is UNKNOWN for a NULL column and `WHERE` drops the row, while driver-memory / formula evaluate the same filter in two-valued JS and return it. Ruled 2026-08-06: "the column has no value" satisfies a test for "not this value", on every backend. - driver-sql: five emitter outlets are NULL-safe — `$ne` / `$nin` / `$notContains` in the main emitter, plus the easily-missed normalized datetime bypass in `applyNormalizedComparison`. Uniform `(col IS NULL OR …)` OR-expansion, not a dialect equivalent: `NOT LIKE` has no such form, the SQLite spelling depends on an engine version this repo does not pin, and the measured query plans are identical. Positive comparisons are byte-identical. - driver-sql: `$exists` with a non-boolean comparand is now refused (INVALID_FILTER / 400) beside the `$null` gate — #5369 via #5347's disposition A. The emitter and polarity-table arms are deliberately unchanged; #5369's "tighten to === true" points the wrong way. - service-analytics read-scope-sql: same three outlets, same PR on purpose — one RLS rule is lowered here for the read path and evaluated by formula for the write-side check, so aligning only one side IS the defect. - formula: `$exists` now means "has a value", the strict mirror of `$null`. Field existence is a schema property, not a record property, so the "key is present" reading is unimplementable on SQL. - spec FILTER_LOGIC_ROWS grows a nullable `d` column, wired into all eleven harnesses, with the `$null` partition enrolled as its control. driver-memory / driver-mongodb are frozen under #5499: zero source changes, existing conformance assertions still green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx --- .../formula-exists-means-has-a-value.md | 27 +++ ...read-scope-null-safe-negative-operators.md | 41 ++++ ...ec-filter-logic-conformance-null-column.md | 34 +++ ...sql-driver-null-safe-negative-operators.md | 70 +++++++ ...ry-driver-filter-logic-conformance.test.ts | 8 +- .../mongodb-filter-logic-conformance.test.ts | 2 + .../src/sql-driver-not-null-safe.test.ts | 61 ++++-- .../src/sql-driver-or-filter.test.ts | 3 + ...river-out-of-contract-filter-input.test.ts | 73 ++++++- packages/drivers/driver-sql/src/sql-driver.ts | 193 ++++++++++++++++-- ...lite-wasm-filter-logic-conformance.test.ts | 3 + .../turso-filter-logic-conformance.test.ts | 2 + ...so-remote-filter-logic-conformance.test.ts | 2 + .../src/matches-filter-not-null-safe.test.ts | 51 ++++- packages/formula/src/matches-filter.ts | 25 ++- ...ative-sql-filter-logic-conformance.test.ts | 11 +- .../read-scope-not-null-safe.test.ts | 38 +++- .../read-scope-sql-conformance.test.ts | 9 +- .../service-analytics/src/read-scope-sql.ts | 37 +++- .../spec/src/data/filter-logic-conformance.ts | 150 ++++++++++---- 20 files changed, 730 insertions(+), 110 deletions(-) create mode 100644 .changeset/formula-exists-means-has-a-value.md create mode 100644 .changeset/read-scope-null-safe-negative-operators.md create mode 100644 .changeset/spec-filter-logic-conformance-null-column.md create mode 100644 .changeset/sql-driver-null-safe-negative-operators.md diff --git a/.changeset/formula-exists-means-has-a-value.md b/.changeset/formula-exists-means-has-a-value.md new file mode 100644 index 0000000000..3fb49b2e81 --- /dev/null +++ b/.changeset/formula-exists-means-has-a-value.md @@ -0,0 +1,27 @@ +--- +"@objectstack/formula": patch +--- + +fix(formula): `matchesFilterCondition` 的 `$exists` 改读「有值」,与 `$null` 成严格互补 + +**行为变更,影响 RLS 写侧 `check` 的判定。** `{ x: { $exists: true } }` 对 +`{ x: null }` 以前答 `true`(键存在),现在答 `false`(没有值)。 + +`matchesFilterCondition` 是 RLS `check` 子句(insert/update 的 post-image)的求值器 —— +写路径上没有查询可以下推,只能逐记录判定。它此前把 `$exists` 读成「键是否存在」 +(`actual !== undefined`),而 `driver-sql` 一直把同一个算子编译成 `IS NOT NULL`。 +于是同一条规则里的 `$exists`,写侧放行的记录读侧看不见。 + +2026-08-06 裁定取「有值」,理由是另一种读法在最要紧的地方**无法兑现**:SQL 里列 +**就是** schema,一行不可能「缺一个键」,所以 `driver-sql` 除了 `IS NOT NULL` 别无 +可编译的东西。字段的存在性是 **schema** 的属性,不是**记录**的属性;spec 若声明 +「键是否存在」,就是在承诺两个后端永远交付不了的语义。因此 `driver-sql` 的发射器 +一字未动,移动的是本求值器。 + +对齐之后 `$exists` 与 `$null` 在每个后端上都是严格互补: +`$exists: true` ≡ `$null: false`,`$exists: false` ≡ `$null: true`。 +「键缺失」与「值为 null」在这里是同一个事实 —— 这也正是 `getPath` 对两者本来就 +返回同一个 `undefined` 的原因。 + +`$ne` / `$nin` / `$notContains` / `$null` 四个算子本来就是本次裁定的目标语义, +一字未改。 diff --git a/.changeset/read-scope-null-safe-negative-operators.md b/.changeset/read-scope-null-safe-negative-operators.md new file mode 100644 index 0000000000..783bc143cc --- /dev/null +++ b/.changeset/read-scope-null-safe-negative-operators.md @@ -0,0 +1,41 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): read scope 的 `$ne` / `$nin` / `$notContains` 改为 NULL-safe,与写侧 `check` 对齐 + +**这是一次安全相关的行为变更,涉及分析查询的可见行集合。** +read scope 里的 `{ stage: { $ne: 'won' } }` 以前**不返回** `stage IS NULL` 的行, +现在**返回**它们。`$nin` / `$notContains` 同理。 + +`read-scope-sql.ts` 是 RLS / 租户 read scope 降解成 SQL 的唯一通道(ADR-0021 D-C)。 +它此前把这三个算子编译成裸的 `col <> ?` / `col NOT IN (…)` / `col NOT LIKE ?`, +而 SQL 是三值逻辑:被比较列为 NULL 时谓词是 UNKNOWN,`WHERE` 只保留 TRUE,于是 +「该列没有值」的行被整批丢掉。 + +**为什么必须与 `driver-sql` 同一个 PR 落地,而不是排到下一批。** 同一条 RLS 规则被 +写一次、在**两侧**求值:读路径由本文件降解成 SQL,写路径由 `formula` 的 +`matchesFilterCondition` 逐记录求值。`formula` 一直用两值 JS(`undefined !== 'won'` +为真)返回这些行。只对齐其中一侧,得到的不是「更小的修复」,而正是那个缺陷本身 —— +一条权限规则准入两个不同的行集,写侧允许的记录读侧看不见。 + +```sql +-- 之前 +"t"."stage" <> ? +"t"."stage" NOT IN (?) +"t"."stage" NOT LIKE ? ESCAPE ? +-- 现在 +("t"."stage" IS NULL OR "t"."stage" <> ?) +("t"."stage" IS NULL OR "t"."stage" NOT IN (?)) +("t"."stage" IS NULL OR "t"."stage" NOT LIKE ? ESCAPE ?) +``` + +括号不是排版:`compileField` 用裸 ` AND ` 连接同一字段的多个算子,不加括号的 +`col IS NULL OR …` 会比那个 AND 结合得更松,从而**静默放宽整条 scope**。 + +与 `driver-sql` 一样统一用 OR 展开而非方言等价物(`NOT LIKE` 没有对应形式;SQLite +写法依赖本仓不锁定的引擎版本;实测执行计划相同)。正向比较逐字符不变, +`$ne: null` 仍是 `IS NOT NULL`(空值谓词,不是比较)。 + +`$not` 路径的逐叶守卫(#5146 / #5326)按原样保留,两条路径读同一张极性表。 +`filter-normalizer`(Cube 面)不在本次范围内,归本裁决第二批。 diff --git a/.changeset/spec-filter-logic-conformance-null-column.md b/.changeset/spec-filter-logic-conformance-null-column.md new file mode 100644 index 0000000000..59ee4a9dbb --- /dev/null +++ b/.changeset/spec-filter-logic-conformance-null-column.md @@ -0,0 +1,34 @@ +--- +"@objectstack/spec": patch +--- + +feat(spec): `FILTER_LOGIC_ROWS` 新增可空列 `d`,`FILTER_LOGIC_CASES` 收入 `$null` 用例 + +跨后端 filter conformance 表此前**刻意不含**任何空值处理,自陈理由是「三值 SQL 引擎 +与两值 JS 匹配器无法被同一个答案约束」。#5146(`$not`)与 #5298 +(`$ne`/`$nin`/`$notContains`)两次裁定取消了这个前提:「该列没有值」现在有唯一的 +跨后端答案,于是它和其他语义一样属于这张标准表。 + +**给第三方驱动作者的迁移要点。** `FilterLogicRow` 新增 `d: string | null` +(第 1-2 行有值,第 3-4 行为 NULL)。用这张表校验自建后端时: + +- DDL / schema 声明里必须把 `d` 声明为**可空**。`NOT NULL` 列,或把 `null` 替换成 + `''` 的 seed,会让新用例因为**错误的原因**变绿 —— 这两条用例要测的恰恰就是 + 「一行没有值」时发生什么,fixture 里没有这样的行就什么都没测到。 +- 新增两条用例:`{d: {$null: true}}` → `['3','4']`,`{d: {$null: false}}` → `['1','2']`。 + 互补的两条一起入表,是为了把 `$null` 钉成对整表的**划分**,而不是其中一半 —— + 这样一个 `NOT NULL` 的 fixture 列会响亮地失败,而不是安静地全绿。 + +单独开一列而不是把 `a` / `b` 挖空:`(a, b)` 的 2x2 真值表是「一对谓词被错误 OR 起来 +必然多出 id」的依据,在它上面开洞会为了空值用例削弱每一条组合子用例。 + +同批订正了模块文档的两处事实错误:独立实现的计数由「五个」改为**七个**(补上 +`driver-turso` 的 `RemoteTransport.buildWhereSQL` 与 `service-analytics` 的 +`filter-normalizer` —— 两者都是手写发射器,#5298 实测它们对空值族的答案与其余五个 +不同),以及原先「#5146 一族 every surface answers the same way」的说法(它漏掉了 +turso remote,该分叉由 #5903 跟踪)。 + +`$ne` / `$not` 两条对应用例**尚未入表**:实测 `driver-turso` remote(#5903)与 +`filter-normalizer`(本裁决第二批)还答不出来,而一条已知会红的用例不能强制任何裁决, +只会把别的车道的未完成工作变成这张表的失败。它们随各自的修复 PR 入表 —— 模块文档的 +「RULED but not yet enrolled」小节里写好了实测矩阵与两个 blocker。 diff --git a/.changeset/sql-driver-null-safe-negative-operators.md b/.changeset/sql-driver-null-safe-negative-operators.md new file mode 100644 index 0000000000..d7b7f97a2b --- /dev/null +++ b/.changeset/sql-driver-null-safe-negative-operators.md @@ -0,0 +1,70 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): `$ne` / `$nin` / `$notContains` 改为 NULL-safe;`$exists` 的非布尔比较值改为拒收 + +**这是一处可观察的查询行为变更,且直接关系到 RLS 的可见集合。** +`{ stage: { $ne: 'won' } }` 以前**不返回** `stage IS NULL` 的行,现在**返回**它们。 +`$nin` 与 `$notContains` 同理。 + +### 变更一:三个否定算子在 `$not` 之外也 NULL-safe(#5298) + +#5146 已经把 `$not` 判定为 NULL-safe(PR #5296),但**只改了 `$not` 内部**;算子自身 +携带否定的三个 —— `$ne` / `$nin` / `$notContains` —— 逐字符未变。于是留下一个使用者 +可见的裂缝:`{ $not: { stage: 'won' } }` 三家一致,`{ stage: { $ne: 'won' } }` 仍然 +分叉。 + +成因与 #5146 同源:SQL 是三值逻辑,`NULL <> 'won'` 是 UNKNOWN 而不是 TRUE,`WHERE` +只保留 TRUE;`driver-memory` 与 `formula` 的 `matchesFilterCondition` 用两值 JS 求值 +(`undefined !== 'won'` 直接为真),把这些行**都返回**。2026-08-06 裁定取「包含无值行」 +方向(与 #5146 同向),本次把 SQL 侧对齐过去。 + +```sql +-- 之前 +`stage` <> 'won' +`stage` not in ('won') +`stage` NOT LIKE '%won%' ESCAPE '\' +-- 现在 +(`stage` is null or `stage` <> 'won') +(`stage` is null or `stage` not in ('won')) +(`stage` is null or `stage` NOT LIKE '%won%' ESCAPE '\') +``` + +**统一用 OR 展开,不走方言等价物**(`IS DISTINCT FROM` / `IS NOT` / `<=>`),三条理由: +`NOT LIKE` 根本没有对应形式,走方言就必然要维护两种形状;SQLite 的写法依赖本仓并不 +锁定的引擎版本(sql.js 与 libSQL 各自演进);实测 `EXPLAIN QUERY PLAN` 两种写法计划 +完全相同 —— `<>` / `NOT IN` / `NOT LIKE` 改动前**本来就是全表扫描**,没有索引可失去, +也没有索引可赢回。 + +**正向比较一个字节都没动。** `{ a: 1 }` 仍然是 `a = 1`,`$in` 仍然是 `in (…)`, +`$gt` / `$contains` 一族同理,所以绝大多数普通查询的 SQL 形状不变。 +`$ne: null` 也不变 —— 它是空值**谓词**(`IS NOT NULL`)而不是比较,「有任何值」对 +一个没有值的行本来就是假。 + +**`$not` 路径不受影响。** `nullSafeNegationOperand` 的逐叶守卫按原样保留:它必须能在 +操作数任意嵌套时通过 De Morgan 组合,这与叶子发射器自身是否全域是两个独立的正确性 +来源,把它们耦合起来会让其中一个的回退静默破坏另一个。 + +### 变更二:`$exists` 的非布尔比较值改为拒收(#5369,套用 #5347 裁定 A) + +`FieldOperatorsSchema` 声明 `$exists: z.boolean()`,而从 `where` 到驱动之间没有任何 +环节按它校验,所以非布尔值真的会到达发射器。到达之后各后端分叉方向相反:本驱动的 +`opValue === false` 恒等判断把「除 false 以外的一切」读成 `IS NOT NULL`,`=== true` +的写法则把「除 true 以外的一切」读成 `IS NULL`。注意字符串 `"false"` 是**真值**, +所以它落在与作者本意**相反**的一侧 —— JSON 往返或 AI 生成的 scope 很容易产出它。 + +现在与 `$null` 的闸门并排,在 `reduceFilterKey` 的校验遍历里拒收,`INVALID_FILTER` / +400,信封与措辞同款。`{ $exists: true }` / `{ $exists: false }` 行为一字未变。 + +**发射器与极性表刻意不动。** 闸门落地后只有两个布尔值能到达它们,`opValue === false` +与 `value === false` 已经是穷尽的二选一。#5369 正文建议的「收紧为 `value === true`」 +方向写反了:极性表回答的是「NULL 列是否**满足**该算子」,而 NULL 列恰恰在调用方要求 +`$exists: false` 时满足它 —— `$null: true` 与 `$exists: false` 是同一个问题,两条 +分支正确地互为镜像,而不是互为副本。 + +### 相关 + +`driver-memory` / `driver-mongodb` 的对应半边按 #5499 冻结,本次零改动、既有一致性 +断言全绿;`driver-turso` 的 remote transport 是独立编译器,归 #5903; +`service-analytics` 的 `filter-normalizer`(Cube 面)归本裁决第二批。 diff --git a/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts b/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts index c5739987d9..8459363f05 100644 --- a/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts @@ -108,7 +108,7 @@ const CONFORMANCE_CUBE: Cube = { sql: TABLE, measures: { count: { name: 'count', label: 'Rows', type: 'count', sql: 'id' } }, dimensions: Object.fromEntries( - (['id', 'a', 'b', 'c', 'owner', 'status', 'parent_object', 'parent_id'] as const).map((f) => [ + (['id', 'a', 'b', 'c', 'd', 'owner', 'status', 'parent_object', 'parent_id'] as const).map((f) => [ f, { name: f, label: f, type: 'string' as const, sql: f }, ]), @@ -132,6 +132,10 @@ describe('[#5324] InMemoryDriver.find — filter logic conformance (the LIVE que a: { type: 'text', name: 'a' }, b: { type: 'text', name: 'b' }, c: { type: 'text', name: 'c' }, + // [#5298] Nullable by construction — rows 3-4 seed `d: null`. This driver + // stores what it is given, so no `nullable` flag exists to set; what + // matters is that the seed keeps the null instead of substituting ''. + d: { type: 'text', name: 'd' }, owner: { type: 'text', name: 'owner' }, status: { type: 'text', name: 'status' }, parent_object: { type: 'text', name: 'parent_object' }, @@ -188,7 +192,7 @@ describe('[#5345] MemoryAnalyticsService — the same table, through the THIRD f await driver.connect(); await driver.syncSchema(TABLE, { fields: Object.fromEntries( - (['id', 'a', 'b', 'c', 'owner', 'status', 'parent_object', 'parent_id'] as const).map((f) => [ + (['id', 'a', 'b', 'c', 'd', 'owner', 'status', 'parent_object', 'parent_id'] as const).map((f) => [ f, { type: 'text', name: f }, ]), diff --git a/packages/drivers/driver-mongodb/src/mongodb-filter-logic-conformance.test.ts b/packages/drivers/driver-mongodb/src/mongodb-filter-logic-conformance.test.ts index af14bf8ca7..841d7b9c6e 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-filter-logic-conformance.test.ts @@ -49,6 +49,8 @@ describe.skipIf(!sharedMongod)('driver-mongodb — filter logic conformance', () a: { type: 'string' }, b: { type: 'string' }, c: { type: 'string' }, + // [#5298] The NULL-bearing column; rows 3-4 seed it as `null`. + d: { type: 'string' }, owner: { type: 'string' }, status: { type: 'string' }, parent_object: { type: 'string' }, diff --git a/packages/drivers/driver-sql/src/sql-driver-not-null-safe.test.ts b/packages/drivers/driver-sql/src/sql-driver-not-null-safe.test.ts index 137cfa276d..f5f581a2cf 100644 --- a/packages/drivers/driver-sql/src/sql-driver-not-null-safe.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-not-null-safe.test.ts @@ -29,14 +29,25 @@ * row 3) and lets the direction of the guard follow each operator's own answer * for a missing value (`$ne` / `$nin` are NOT widened). * - * Nothing outside a `$not` changes: an ordinary comparison compiles to the same - * SQL it always did, so no plain predicate loses an index to this. + * A POSITIVE comparison is unchanged: it compiles to the same SQL it always did, + * so no plain predicate loses an index to this. + * + * # [#5298] The sequel, and why this file now pins BOTH directions + * + * #5146 deliberately stopped at `$not` and this suite pinned that boundary: + * two assertions asserted that a bare `$ne` still dropped the NULL rows, so a + * rewrite leaking past its scope would go red. The 2026-08-06 ruling on #5298 + * took the same direction for the operators that carry their own negation + * (`$ne` / `$nin` / `$notContains`), so those two assertions FLIPPED — see the + * block at the bottom of this file, which says so at the point of the flip. * * These expectations are duplicated by hand in the two JS backends' - * `*-not-null-safe.test.ts`. They belong in `FILTER_LOGIC_CASES` - * (`@objectstack/spec/data`) so every backend is held to them at once — that - * table is being extended under #5239 / the spec lane of #5146, which lands - * with driver-mongodb; until then these three files are the pin. + * `*-not-null-safe.test.ts`. `FILTER_LOGIC_CASES` (`@objectstack/spec/data`) + * grew a nullable column in #5298 and now carries the `$null` partition for + * every backend at once; the `$ne` and `$not` rows join it once `driver-turso`'s + * remote transport answers them the same way (#5903 — it is an independent + * filter compiler that inherits none of this). Until then these files are the + * pin for the SQL family. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -220,23 +231,43 @@ describe('[#5146] SqlDriver compiles $not NULL-safely', () => { // ── Nothing outside `$not` moves ─────────────────────────────────────────── - describe('only the $not path is rewritten', () => { - it('a plain comparison compiles to exactly the SQL it always did', () => { + describe('POSITIVE comparisons are still compiled exactly as before', () => { + it('a positive comparison compiles to exactly the SQL it always did', () => { expect(sqlFor({ stage: 'won' })).toBe("select `id` from `deal` where `stage` = 'won'"); - expect(sqlFor({ stage: { $ne: 'won' } })).toBe("select `id` from `deal` where `stage` <> 'won'"); expect(sqlFor({ amount: { $gt: 15 } })).toBe('select `id` from `deal` where `amount` > 15'); expect(sqlFor({ stage: { $in: ['won'] } })).toBe("select `id` from `deal` where `stage` in ('won')"); }); - it('a plain comparison returns the rows it always did', async () => { - expect(await ids({ stage: 'won' })).toEqual(['1']); - // Still SQL semantics outside a negation: `<> 'won'` drops the NULL rows. - // That divergence from the JS backends is real but out of #5146's scope — - // it is filed separately rather than smuggled in here. - expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2']); + /** + * FLIPPED PIN (#5298). Both assertions in this block used to pin the + * OPPOSITE answer, on purpose: when #5146 was implemented the non-negated + * `$ne` was explicitly out of its scope, so the suite pinned "`<> 'won'` + * still drops the NULL rows" to prove the rewrite had not leaked past the + * `$not` it was scoped to. The 2026-08-06 ruling on #5298 took the other + * direction for `$ne` / `$nin` / `$notContains`, so the pin flips with it — + * this is the reverse-verification anchor doing its job, not a regression. + * + * The `$not` half of the suite above is untouched and still green, which is + * what says the two rulings compose rather than one overwriting the other. + */ + it('$ne / $nin / $notContains are NULL-safe outside a $not too (#5298)', async () => { + expect(sqlFor({ stage: { $ne: 'won' } })).toBe( + "select `id` from `deal` where (`stage` is null or `stage` <> 'won')", + ); + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); + expect(await ids({ stage: { $nin: ['won'] } })).toEqual(['2', '3', '4']); + expect(await ids({ stage: { $notContains: 'wo' } })).toEqual(['2', '3', '4']); expect(await ids({})).toEqual(ALL); }); + it('a positive comparison returns the rows it always did', async () => { + expect(await ids({ stage: 'won' })).toEqual(['1']); + expect(await ids({ stage: { $in: ['won'] } })).toEqual(['1']); + // `$ne: null` is a null PREDICATE, not a comparison — "has any value" is + // still false for a row that has none. Unchanged by #5298. + expect(await ids({ stage: { $ne: null } })).toEqual(['1', '2']); + }); + it('a $or / $and of plain comparisons is unchanged', () => { expect(sqlFor({ $or: [{ stage: 'won' }, { owner: 'u2' }] })).toBe( "select `id` from `deal` where ((`stage` = 'won') or (`owner` = 'u2'))", diff --git a/packages/drivers/driver-sql/src/sql-driver-or-filter.test.ts b/packages/drivers/driver-sql/src/sql-driver-or-filter.test.ts index 10af2ecb52..2abecb3044 100644 --- a/packages/drivers/driver-sql/src/sql-driver-or-filter.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-or-filter.test.ts @@ -86,6 +86,9 @@ function declareFilterLogicSweep(cell: DialectCell): void { t.string('a'); t.string('b'); t.string('c'); + // [#5298] Nullable (knex's default) — rows 3-4 of the fixture have no `d`, + // and the null cases measure nothing against a NOT NULL column. + t.string('d'); t.string('owner'); t.string('status'); t.string('parent_object'); diff --git a/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts b/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts index 01d4f4a812..d937ba514b 100644 --- a/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-out-of-contract-filter-input.test.ts @@ -221,13 +221,68 @@ describe('[#5347/#5348] SqlDriver refuses out-of-contract filter input', () => { expect(await ids({ $not: { stage: 'won' } })).toEqual(['2']); }); - it('$exists is deliberately NOT tightened here', async () => { - // #5347 ruled on `$null`. `$exists` carries the identical `=== false` - // identity read and diverges too, but on its own axis — what "exists" - // means for a null-valued key is #5299's open question — so it keeps - // today's behaviour rather than being settled as a rider on this fix. - expect(await ids({ stage: { $exists: 'yes' } })).toEqual(['1']); - expect(await ids({ stage: { $exists: 0 } })).toEqual(['1']); + }); + + describe('[#5369] $exists with a non-boolean comparand', () => { + /** + * FLIPPED CARVE-OUT. This block used to be titled "$exists is deliberately + * NOT tightened here" and pinned the answers `['1']` for `$exists: 'yes'` + * and `$exists: 0` — #5347 ruled on `$null` alone, and what "exists" means + * for a null-valued key was still #5299's open question, so tightening it + * as a rider would have been settling a second ruling silently. + * + * The 2026-08-06 ruling on #5298 closed that question ("has a value") and + * applied #5347's disposition A to `$exists` by name. So the carve-out + * becomes the gate, with the same envelope and the same wording shape. + * + * What did NOT change with it: the emitter's `opValue === false` arm and + * the polarity table's `value === false` arm. Both are exhaustive two-way + * choices once only booleans reach them, and #5369's suggestion to tighten + * the latter to `=== true` points the wrong way — that table answers + * "does a NULL column SATISFY the operator", and a NULL column satisfies + * `$exists` when the caller asked for `false`. + */ + const NON_BOOLEAN: Array<[label: string, value: unknown]> = [ + ["the string 'yes'", 'yes'], + ['the number 1', 1], + ['the number 0', 0], + ['null', null], + ['undefined', undefined], + ['an object', {}], + ["the STRING 'false'", 'false'], + ]; + + for (const [label, value] of NON_BOOLEAN) { + it(`refuses ${label} with INVALID_FILTER / 400`, async () => { + const err = await refusalOf({ stage: { $exists: value } }); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('Operator "$exists" on field "stage" requires a boolean comparand'); + expect(err.message).toContain('filter.stage.$exists'); + expect(err.message).not.toContain('[sql-driver]'); + }); + } + + it('refuses inside a combinator, and inside $not', async () => { + for (const where of [ + { $and: [{ stage: { $exists: 'yes' } }] }, + { $or: [{ stage: 'won' }, { stage: { $exists: 1 } }] }, + { $not: { stage: { $exists: 'yes' } } }, + ]) { + const err = await refusalOf(where); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.message).toContain('requires a boolean comparand'); + } + }); + + it('true and false are unchanged, line by line', async () => { + expect(await ids({ stage: { $exists: true } })).toEqual(['1']); + expect(await ids({ stage: { $exists: false } })).toEqual(['2']); + expect(await ids({ $not: { stage: { $exists: true } } })).toEqual(['2']); + // The complement `$null` answers the mirror image, on every comparand — + // the invariant #5298 made the platform-wide reading of these two. + expect(await ids({ stage: { $exists: true } })).toEqual(await ids({ stage: { $null: false } })); + expect(await ids({ stage: { $exists: false } })).toEqual(await ids({ stage: { $null: true } })); }); }); @@ -237,7 +292,9 @@ describe('[#5347/#5348] SqlDriver refuses out-of-contract filter input', () => { it('the ordinary vocabulary still answers', async () => { expect(await ids({ stage: 'won' })).toEqual(['1']); expect(await ids({ stage: { $in: ['won'] } })).toEqual(['1']); - expect(await ids({ stage: { $ne: 'won' } })).toEqual([]); + // [#5298] NULL-safe: row 2's `stage` is NULL, and "not won" is true of a + // value that is not there. This line read `[]` before that ruling. + expect(await ids({ stage: { $ne: 'won' } })).toEqual(['2']); expect(await ids({ score: { $between: [5, 15] } })).toEqual(['1']); expect(await ids({ score: { $gte: 10 } })).toEqual(['1', '2']); expect(await ids({ stage: { $startsWith: 'w' } })).toEqual(['1']); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 0eae7377f1..986757eaeb 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -837,6 +837,43 @@ function unknownLogicalOperatorError(key: string, path: string): Error { * "exists" means for a null-valued key is #5299's open question), and #5347 * ruled on `$null`. Filed separately rather than settled as a rider. */ +/** + * [#5369, applying #5347's ruling A] `$exists` whose comparand is not a boolean. + * + * The symmetric twin of {@link nonBooleanNullComparandError}, and deliberately + * a copy of its disposition rather than a fresh judgement: `FieldOperatorsSchema` + * declares `$exists: z.boolean()` exactly as it declares `$null`, nothing between + * an authored `where` and this driver validates against it, and the backends + * split the same way on a third value — this emitter's `opValue === false` + * identity reads anything but `false` as IS NOT NULL, while the `=== true` + * spelling used elsewhere reads anything but `true` as IS NULL. Two complements, + * neither a rule anyone wrote down. + * + * #5347 ruled that shape REFUSED for `$null`; #5369 asked whether the same + * applies here and the 2026-08-06 ruling on #5298 said yes, "照 #5347-A". So the + * gate lands beside `$null`'s in {@link reduceFilterKey}, not in the emitter — + * same evaluation-order reason, same `INVALID_FILTER` envelope. + * + * Note what does NOT change with it: the emitter's `opValue === false` arm and + * {@link nullValueSatisfiesOperator}'s `value === false` arm stay exactly as + * they are. Once the gate holds, `true` and `false` are the only comparands that + * reach either, so both tests are already exhaustive two-way choices — the + * lenient-vs-strict question #5347 had to answer for `$null` does not arise + * here, because both spellings agree on the two surviving values. + */ +function nonBooleanExistsComparandError(field: string, value: unknown, path: string): Error { + return unsupportedFilterError( + `Operator "$exists" on field "${field}" requires a boolean comparand (true or false). ` + + `Received ${describeFilterOperand(value)} (${safeShapePreview(value)}) at ${path}. ` + + `@objectstack/spec FieldOperatorsSchema declares $exists as a boolean. It is refused rather ` + + `than coerced for the same reason $null is (#5347): a non-boolean lands on whichever side ` + + `the backend's two-branch conditional happens to default to, and those defaults point in ` + + `OPPOSITE directions — this driver's \`=== false\` test compiles IS NOT NULL for anything ` + + `but false, a \`=== true\` test compiles IS NULL for anything but true. Note "false" the ` + + `STRING is truthy, so it lands on the side opposite the false it was written to mean (#5369).`, + ); +} + function nonBooleanNullComparandError(field: string, value: unknown, path: string): Error { return unsupportedFilterError( `Operator "$null" on field "${field}" requires a boolean comparand (true or false). ` + @@ -973,6 +1010,20 @@ function reduceFilterKey(key: string, value: unknown, path: string): FilterVerdi throw nonBooleanNullComparandError(key, value.$null, `${here}.$null`); } + // [#5369] `$exists`'s comparand is a boolean by the same declaration, refused + // on the same walk, for the same evaluation-order reason. Kept as a separate + // `if` rather than folded into a loop over a two-name list: each operator gets + // its own message naming its own emitter's default direction, and a shared + // loop would be the start of the second vocabulary gate the block above warns + // about (#3948). + if ( + isFilterNode(value) && + Object.prototype.hasOwnProperty.call(value, '$exists') && + typeof value.$exists !== 'boolean' + ) { + throw nonBooleanExistsComparandError(key, value.$exists, `${here}.$exists`); + } + // A field key always contributes a predicate. return 'clause'; } @@ -1021,12 +1072,20 @@ function nullValueSatisfiesOperator(op: string, value: unknown): boolean { // The strict spelling cannot — it is the same "declared = enforced" reflex // the refusal itself is. case '$null': return value === true; - // `$exists` keeps its lenient identity read: unlike `$null` it has NO - // comparand gate (#5347 ruled on `$null` only), so a non-boolean still - // reaches this table, and the guard must keep answering it the same way the - // emitter's `opValue === false` arm does or the two can disagree about a - // row. Tightening this one without the matching refusal would be the - // divergence, not the fix — filed separately. + // [#5369] The gate this arm's old comment said was missing now EXISTS: + // `reduceFilterKey` refuses a non-boolean `$exists` comparand beside the + // `$null` one, so `true` and `false` are the only values that reach here. + // + // The line itself is deliberately unchanged, and #5369's suggestion to + // "tighten it to `value === true`" is not applied — it points the wrong way. + // This table answers "does a NULL column SATISFY the operator", and a NULL + // column satisfies `$exists` exactly when the caller asked for `false` + // ("no value"). `$null: true` and `$exists: false` are the same question, so + // their arms are correctly each other's mirror, not each other's copy. With + // the gate holding, `value === false` is already an exhaustive two-way + // choice over the declared domain — the lenient-vs-strict distinction that + // made #5347 rewrite the `$null` arm does not exist here, because both + // spellings agree on both surviving values. case '$exists': return value === false; // Negative-polarity set/substring tests: "not among" / "does not contain" // hold vacuously for a value that is absent. @@ -1126,9 +1185,15 @@ function nullGuardForFieldSpec(spec: unknown): NullGuard { * paying for (#2704, #5134). So each leaf is guarded in the direction its own * operator answers, per {@link nullValueSatisfiesOperator}. * - * The rewrite only ever runs INSIDE a `$not`; a plain comparison's SQL is - * untouched, so `{ a: 1 }` still compiles to `a = 1` and nothing outside a - * negation changes shape or loses an index. + * This rewrite only ever runs INSIDE a `$not`, and a POSITIVE comparison's SQL is + * untouched by it or by anything else — `{ a: 1 }` still compiles to `a = 1`. + * + * [#5298] What changed since is the other half: the three operators that carry + * their own negation (`$ne`, `$nin`, `$notContains`) are NULL-safe outside a + * `$not` too, emitted directly by {@link SqlDriver.applyNullSafeNegative} rather + * than through this rewrite. Both paths read the same polarity table and reach + * the same answer; they stay separate because this one has to compose through De + * Morgan over a whole operand tree, while that one guards a single leaf. * * A nested `$not` is deliberately left alone: its own branch totalises its * operand, and `NOT ` is itself total, so recursing into it here would @@ -6092,17 +6157,38 @@ export class SqlDriver implements IDataDriver { value: unknown, ): boolean { const raw = join === 'or' ? 'orWhereRaw' : 'whereRaw'; - const binary = (sqlOp: string): boolean => { + /** + * [#5298] Wrap a value test so a row whose column has no value SATISFIES it: + * `( IS NULL OR )`. + * + * `expr.sql` is a storage-normalising expression over ONE column + * ({@link filterColumnExpr}), and every dialect's `datetime()` / `CASE` + * form answers NULL for a NULL input — so testing the expression is the + * same question as testing the raw column, and keeps the whole predicate + * readable as one unit. `expr.bindings` is repeated because `expr.sql` + * appears twice. + */ + const nullSafe = (testSql: string, testBindings: any[]): void => { + builder[raw](`(${expr.sql} IS NULL OR ${testSql})`, [...expr.bindings, ...testBindings]); + }; + const binary = (sqlOp: string, negative = false): boolean => { // A null comparand is a null PREDICATE, not a comparison — hand it back so // the caller compiles `IS NULL` / `IS NOT NULL` as it always has. if (value == null) return false; - builder[raw](`${expr.sql} ${sqlOp} ?`, [...expr.bindings, value]); + const sql = `${expr.sql} ${sqlOp} ?`; + const bindings = [...expr.bindings, value]; + if (negative) nullSafe(sql, bindings); + else builder[raw](sql, bindings); return true; }; const list = (sqlOp: 'in' | 'not in'): boolean => { if (!Array.isArray(value) || value.length === 0) return false; const placeholders = value.map(() => '?').join(', '); - builder[raw](`${expr.sql} ${sqlOp} (${placeholders})`, [...expr.bindings, ...value]); + const sql = `${expr.sql} ${sqlOp} (${placeholders})`; + const bindings = [...expr.bindings, ...value]; + // [#5298] Only `not in` is negative-polarity; `in` stays a bare test. + if (sqlOp === 'not in') nullSafe(sql, bindings); + else builder[raw](sql, bindings); return true; }; @@ -6110,7 +6196,8 @@ export class SqlDriver implements IDataDriver { case '=': case '==': case '$eq': return binary('='); case '!=': case '<>': case '$ne': - return binary('<>'); + // [#5298] NULL-safe — the same ruling the plain-column arm below follows. + return binary('<>', true); case '>': case '$gt': return binary('>'); case '>=': case '$gte': @@ -6270,6 +6357,57 @@ export class SqlDriver implements IDataDriver { this.applyLike(builder, method, field, value, 'contains'); } + /** + * [#5298] Emit a NEGATIVE-polarity value test so a row whose column is NULL + * satisfies it: `(col IS NULL OR )`. + * + * # Why the non-negated operators need this at all + * + * SQL is three-valued and a `WHERE` keeps only TRUE, so `d <> 'v1'` is + * UNKNOWN — and therefore dropped — for every row where `d` is NULL, while + * `driver-memory` and `formula` evaluate the same filter in two-valued JS + * (`undefined !== 'v1'` is simply true) and return those rows. #5146 ruled + * that divergence for `$not`; #5298 ruled it the same way for the three + * operators that carry their negation in the operator itself — `$ne`, + * `$nin`, `$notContains`. "The column has no value" satisfies a test for + * "not this value", on every backend. + * + * It is a security fix as much as a consistency one: one RLS rule is + * evaluated by the read-side SQL lowering AND the write-side `check` + * evaluator, so a per-backend answer here means one permission rule admitting + * two different row sets (`read-scope-sql.ts` carries the same change). + * + * # Why OR-expansion and not a dialect equivalent + * + * `IS DISTINCT FROM` (Postgres) / `IS NOT` (SQLite) / `<=>` (MySQL) each + * express this in one operator, and all three were rejected: `NOT LIKE` has + * no such form at all, so `$notContains` would need the OR shape anyway and + * the driver would carry two shapes for one ruling; the SQLite spelling + * depends on an engine version this repo does not pin (sql.js / libSQL move + * independently); and measured `EXPLAIN QUERY PLAN` output is identical + * either way — `<>`, `NOT IN` and `NOT LIKE` were already full scans before + * this change, so there is no index to lose and none to win back. One shape, + * every dialect. + * + * # Why a group and not a raw string + * + * The callback form keeps the predicate ONE unit for the enclosing builder, + * so an `$or` branch attaches it as a single clause and the wrapping + * parentheses are Knex's, not hand-built — the `IS NULL OR` must never + * escape its own conjunct and widen a sibling. + */ + private applyNullSafeNegative( + builder: any, + method: string, + field: string, + emitValueTest: (qb: any) => void, + ): void { + (builder as any)[method]((qb: any) => { + qb.whereNull(field); + emitValueTest(qb); + }); + } + /** * Parameterized `LIKE`/`NOT LIKE` match with the LIKE metacharacters `%` / `_` * (and the escape char `\`) escaped in the user value so they match literally @@ -6345,13 +6483,18 @@ export class SqlDriver implements IDataDriver { * `applyFilters` path refused it and the two JS backends answered FALSE. One * declared shape, three answers; see {@link emptyFieldConstraintError}. * - * # NULL-safe negation (#5146) + * # NULL-safe negation (#5146 `$not`, #5298 the negative operators) * * `$not` negates a predicate that {@link nullSafeNegationOperand} has first * made TOTAL, because SQL's `NOT UNKNOWN` is UNKNOWN and a `WHERE` drops it — * which used to hide every row whose compared column was NULL, while - * `driver-memory` and `formula` returned those same rows. Only the `$not` - * path is rewritten; an ordinary comparison compiles exactly as before. + * `driver-memory` and `formula` returned those same rows. + * + * #5298 extended the same ruling to the non-negated path: `$ne`, `$nin` and + * `$notContains` emit `(col IS NULL OR )` via + * {@link SqlDriver.applyNullSafeNegative}. A POSITIVE comparison is still + * compiled exactly as it always was — `{ a: 1 }` is `a = 1`, `$in` is `in (…)` + * — so nothing on the majority path changed shape. */ protected applyFilterCondition(builder: Knex.QueryBuilder, condition: any, logicalOp: 'and' | 'or' = 'and', tableHint?: string | null) { if (!condition || typeof condition !== 'object') return; @@ -6477,8 +6620,11 @@ export class SqlDriver implements IDataDriver { break; case '$ne': // `<> NULL` matches nothing; a null comparand means "has any value". + // UNCHANGED by #5298: `IS NOT NULL` is already total, and both + // sides of the ruling agree a row with no value does NOT have + // "any value". Only the value COMPARISON below becomes NULL-safe. if (coerced == null) (builder as any)[logicalOp === 'or' ? 'orWhereNotNull' : 'whereNotNull'](field); - else (builder as any)[method](field, '<>', coerced); + else this.applyNullSafeNegative(builder, method, field, (qb) => qb.orWhere(field, '<>', coerced)); break; case '$gt': (builder as any)[method](field, '>', coerced); @@ -6498,8 +6644,11 @@ export class SqlDriver implements IDataDriver { break; } case '$nin': { - const mNotIn = logicalOp === 'or' ? 'orWhereNotIn' : 'whereNotIn'; - (builder as any)[mNotIn](field, coerced as any[]); + // [#5298] NULL-safe: "not among this list" holds vacuously for a + // value that is not there, which is what every JS backend answers. + this.applyNullSafeNegative(builder, method, field, (qb) => + qb.orWhereNotIn(field, coerced as any[]), + ); break; } case '$contains': @@ -6512,7 +6661,11 @@ export class SqlDriver implements IDataDriver { this.applyContainsLike(builder, method, field, opValue); break; case '$notContains': - this.applyLike(builder, method, field, opValue, 'contains', true); + // [#5298] NULL-safe: `NOT LIKE` is UNKNOWN for a NULL column, and + // "does not contain" is true of a value that is not there. + this.applyNullSafeNegative(builder, method, field, (qb) => + this.applyLike(qb, 'orWhere', field, opValue, 'contains', true), + ); break; case '$startsWith': this.applyLike(builder, method, field, opValue, 'starts'); diff --git a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-filter-logic-conformance.test.ts b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-filter-logic-conformance.test.ts index 3288e8551c..55b1040195 100644 --- a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-filter-logic-conformance.test.ts @@ -37,6 +37,9 @@ describe('driver-sqlite-wasm — filter logic conformance', () => { a: { type: 'string' }, b: { type: 'string' }, c: { type: 'string' }, + // [#5298] Nullable — the DDL this driver generates leaves a plain + // string column nullable, which is what rows 3-4 need. + d: { type: 'string' }, owner: { type: 'string' }, status: { type: 'string' }, parent_object: { type: 'string' }, diff --git a/packages/drivers/driver-turso/src/turso-filter-logic-conformance.test.ts b/packages/drivers/driver-turso/src/turso-filter-logic-conformance.test.ts index 2585c41277..1f0f36c314 100644 --- a/packages/drivers/driver-turso/src/turso-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-turso/src/turso-filter-logic-conformance.test.ts @@ -41,6 +41,8 @@ const CONFORMANCE_OBJECT = { a: { type: 'string' }, b: { type: 'string' }, c: { type: 'string' }, + // [#5298] The NULL-bearing column; rows 3-4 seed it as `null`. + d: { type: 'string' }, owner: { type: 'string' }, status: { type: 'string' }, parent_object: { type: 'string' }, diff --git a/packages/drivers/driver-turso/src/turso-remote-filter-logic-conformance.test.ts b/packages/drivers/driver-turso/src/turso-remote-filter-logic-conformance.test.ts index 3bf538c87f..02af6eaa26 100644 --- a/packages/drivers/driver-turso/src/turso-remote-filter-logic-conformance.test.ts +++ b/packages/drivers/driver-turso/src/turso-remote-filter-logic-conformance.test.ts @@ -44,6 +44,8 @@ const CONFORMANCE_OBJECT = { a: { type: 'string' }, b: { type: 'string' }, c: { type: 'string' }, + // [#5298] The NULL-bearing column; rows 3-4 seed it as `null`. + d: { type: 'string' }, owner: { type: 'string' }, status: { type: 'string' }, parent_object: { type: 'string' }, diff --git a/packages/formula/src/matches-filter-not-null-safe.test.ts b/packages/formula/src/matches-filter-not-null-safe.test.ts index 799ccea704..71d73c63e6 100644 --- a/packages/formula/src/matches-filter-not-null-safe.test.ts +++ b/packages/formula/src/matches-filter-not-null-safe.test.ts @@ -139,16 +139,47 @@ describe('[#5146] matchesFilterCondition — $not over records with no value', ( }); }); - // ── Where this evaluator and `driver-memory` disagree — pinned, not fixed ── - - describe('known disagreement with driver-memory (NOT ruled on by #5146)', () => { - it('$exists reads "the key is present", so a null value EXISTS', () => { - // `driver-memory` reads `$exists` as "has a value", so it answers the - // opposite for a present-but-null field. `driver-sql` cannot tell the two - // apart at all (a NULL column is a NULL column) and keeps its existing - // `IS NOT NULL` compilation. - expect(ids(NULLED, { $not: { stage: { $exists: true } } })).toEqual([]); - expect(ids(MISSING, { $not: { stage: { $exists: true } } })).toEqual(['3', '4']); + // ── `$exists` — RULED on #5298, and this evaluator is the side that moved ── + + describe('[#5298/#5369] $exists means "has a value", the strict mirror of $null', () => { + /** + * FLIPPED PIN. This block used to sit under "known disagreement with + * driver-memory (NOT ruled on by #5146)" and pinned the OPPOSITE answer for + * `NULLED`: `$exists` read "the key is present", so a present-but-null + * `stage` EXISTED and the negation matched nothing. + * + * The 2026-08-06 ruling on #5298 took "has a value". The deciding argument + * is that the other reading is unimplementable where it matters: a SQL + * column IS the schema, so a row cannot have an absent key, and `driver-sql` + * has always compiled `$exists` to `IS NOT NULL`. Field existence is a + * property of the schema, not of the record — a spec that declared + * otherwise would be promising a semantics two of its backends can never + * deliver. `driver-sql`'s emitter is therefore unchanged; THIS evaluator is + * the one that moved. + */ + it('a present-but-null field does NOT exist', () => { + expect(ids(NULLED, { stage: { $exists: true } })).toEqual(['1', '2']); + expect(ids(NULLED, { $not: { stage: { $exists: true } } })).toEqual(['3', '4']); + }); + + it('a missing key answers exactly as a present-but-null one does', () => { + // The two fixtures differ only in whether row 3/4 carry the key at all. + // Under "has a value" they are the same fact, so the answers must match — + // which is the property the old `!== undefined` reading broke. + for (const filter of [ + { stage: { $exists: true } }, + { stage: { $exists: false } }, + { $not: { stage: { $exists: true } } }, + ] as const) { + expect(ids(NULLED, filter), JSON.stringify(filter)).toEqual(ids(MISSING, filter)); + } + }); + + it('$exists is the strict complement of $null, comparand for comparand', () => { + for (const fixture of [NULLED, MISSING]) { + expect(ids(fixture, { stage: { $exists: true } })).toEqual(ids(fixture, { stage: { $null: false } })); + expect(ids(fixture, { stage: { $exists: false } })).toEqual(ids(fixture, { stage: { $null: true } })); + } }); }); }); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index d89188f45e..4ecf5ba1a5 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -184,7 +184,30 @@ function evalOp(actual: unknown, op: string, raw: unknown, record: Record [ + ['id', 'a', 'b', 'c', 'd', 'owner', 'status', 'parent_object', 'parent_id'].map((n) => [ n, { name: n, label: n, type: 'string', sql: n }, ]), @@ -89,16 +89,19 @@ describe('NativeSQLStrategy — filter logic conformance', () => { CREATE TABLE "t" ( "id" TEXT PRIMARY KEY, "a" TEXT, "b" TEXT, "c" TEXT, + -- [#5298] Nullable on purpose: rows 3-4 of the fixture have no "d", and + -- the no-value cases measure nothing against a NOT NULL column. + "d" TEXT, "owner" TEXT, "status" TEXT, "parent_object" TEXT, "parent_id" TEXT ); `); const insert = db.prepare( - `INSERT INTO "t" ("id","a","b","c","owner","status","parent_object","parent_id") - VALUES (?,?,?,?,?,?,?,?)`, + `INSERT INTO "t" ("id","a","b","c","d","owner","status","parent_object","parent_id") + VALUES (?,?,?,?,?,?,?,?,?)`, ); for (const r of FILTER_LOGIC_ROWS) { - insert.run([r.id, r.a, r.b, r.c, r.owner, r.status, r.parent_object, r.parent_id]); + insert.run([r.id, r.a, r.b, r.c, r.d, r.owner, r.status, r.parent_object, r.parent_id]); } insert.free(); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts index bcb7ec837d..7616baebe9 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-not-null-safe.test.ts @@ -355,23 +355,43 @@ describe('[#5297] read-scope `$not` — boolean identities and NULL safety', () // ── Nothing outside `$not` moves, and nothing stopped failing closed ─────── - describe('only the `$not` path is rewritten', () => { - it('a plain comparison compiles to exactly the SQL it always did', () => { + describe('POSITIVE comparisons are still compiled exactly as before', () => { + it('a positive comparison compiles to exactly the SQL it always did', () => { expect(compileScopedFilterToSql({ stage: 'won' } as FilterCondition, ALIAS).sql) .toBe('"t"."stage" = ?'); - expect(compileScopedFilterToSql({ stage: { $ne: 'won' } } as FilterCondition, ALIAS).sql) - .toBe('"t"."stage" <> ?'); expect(compileScopedFilterToSql({ amount: { $gt: 15 } } as FilterCondition, ALIAS).sql) .toBe('"t"."amount" > ?'); }); - it('a plain comparison returns the rows it always did', () => { - expect(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(ids({ stage: { $ne: 'won' } })).toEqual(['2']); + /** + * FLIPPED PIN (#5298). These two assertions used to pin the OPPOSITE + * answer — `"t"."stage" <> ?` and the single row `['2']` — deliberately: + * the non-negated `$ne` was explicitly outside #5146's scope, and pinning + * the old behaviour is what would have caught a rewrite leaking past the + * `$not` it was scoped to. The 2026-08-06 ruling on #5298 took the other + * direction, so the pin flips with the ruling. The reverse-verification + * anchor did its job; it is not a regression. + * + * This compiler moving in the SAME PR as `driver-sql` is the point rather + * than a convenience: one RLS rule is lowered here for the read path and + * evaluated by `formula` for the write-side `check`, so a batch that + * aligned only one of them would leave a permission rule admitting two + * different row sets — the defect, not a smaller version of the fix. + */ + it('$ne / $nin / $notContains are NULL-safe outside a $not too (#5298)', () => { + expect(compileScopedFilterToSql({ stage: { $ne: 'won' } } as FilterCondition, ALIAS).sql) + .toBe('("t"."stage" IS NULL OR "t"."stage" <> ?)'); + expect(ids({ stage: { $ne: 'won' } })).toEqual(['2', '3', '4']); + expect(ids({ stage: { $nin: ['won'] } })).toEqual(['2', '3', '4']); + expect(ids({ stage: { $notContains: 'wo' } })).toEqual(['2', '3', '4']); expect(ids({})).toEqual(ALL); }); + + it('a positive comparison returns the rows it always did', () => { + expect(ids({ stage: 'won' })).toEqual(['1']); + // `$ne: null` is a null PREDICATE, not a comparison — unchanged by #5298. + expect(ids({ stage: { $ne: null } })).toEqual(['1', '2']); + }); }); describe('the fail-closed guarantees survive the rewrite', () => { diff --git a/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts index be5f53e1a5..58c117e5d6 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts @@ -83,16 +83,19 @@ describe('compileScopedFilterToSql — filter logic conformance', () => { CREATE TABLE "t" ( "id" TEXT PRIMARY KEY, "a" TEXT, "b" TEXT, "c" TEXT, + -- [#5298] Nullable on purpose: rows 3-4 of the fixture have no "d", and + -- the no-value cases measure nothing against a NOT NULL column. + "d" TEXT, "owner" TEXT, "status" TEXT, "parent_object" TEXT, "parent_id" TEXT ); `); const insert = db.prepare( - `INSERT INTO "t" ("id","a","b","c","owner","status","parent_object","parent_id") - VALUES (?,?,?,?,?,?,?,?)`, + `INSERT INTO "t" ("id","a","b","c","d","owner","status","parent_object","parent_id") + VALUES (?,?,?,?,?,?,?,?,?)`, ); for (const r of FILTER_LOGIC_ROWS) { - insert.run([r.id, r.a, r.b, r.c, r.owner, r.status, r.parent_object, r.parent_id]); + insert.run([r.id, r.a, r.b, r.c, r.d, r.owner, r.status, r.parent_object, r.parent_id]); } insert.free(); }); diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 32868989ca..ada0a1df5e 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -330,10 +330,37 @@ function bindLike(params: unknown[], pattern: string): string { return `${bind(params, pattern)} ESCAPE ${bind(params, LIKE_ESCAPE_CHAR)}`; } +/** + * [#5298] Wrap a negative-polarity value test so a row whose column has no value + * SATISFIES it: `(col IS NULL OR )`. + * + * The read-scope twin of `driver-sql`'s `applyNullSafeNegative`, and the reason + * this compiler had to move in the same PR rather than a later one: an RLS rule + * is authored once and evaluated on BOTH sides — this file lowers it for the + * read path while `formula`'s `matchesFilterCondition` evaluates it for the + * write-side `check`. Leaving the two on different answers for `$ne` is one + * permission rule admitting two different row sets, which is the security + * defect #5146 named for `$not` and #5298 ruled for the rest. + * + * OR-expansion rather than `IS DISTINCT FROM` / `IS NOT` / `<=>`, for the three + * reasons recorded on the driver-side twin: `NOT LIKE` has no such form, the + * SQLite spelling depends on an engine version nothing here pins, and the + * measured query plans are identical either way. + * + * The parentheses are not optional. {@link compileField} joins a field's + * operators with bare ` AND `, so an unwrapped `col IS NULL OR …` would bind + * looser than that AND and silently widen the whole scope. + */ +function nullSafeNegative(col: string, test: string): string { + return `(${col} IS NULL OR ${test})`; +} + function compileOperator(col: string, op: string, val: unknown, field: string, params: unknown[]): string { switch (op) { case '$eq': return val === null ? `${col} IS NULL` : `${col} = ${bind(params, val)}`; - case '$ne': return val === null ? `${col} IS NOT NULL` : `${col} <> ${bind(params, val)}`; + // [#5298] `$ne: null` stays `IS NOT NULL` — already total, and "has any + // value" is false for a row that has none. Only the comparison is guarded. + case '$ne': return val === null ? `${col} IS NOT NULL` : nullSafeNegative(col, `${col} <> ${bind(params, val)}`); case '$gt': return `${col} > ${bind(params, val)}`; case '$gte': return `${col} >= ${bind(params, val)}`; case '$lt': return `${col} < ${bind(params, val)}`; @@ -346,7 +373,9 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p case '$nin': { if (!Array.isArray(val)) throw readScopeCompileError(`[read-scope-sql] $nin for "${field}" needs an array (fail-closed).`); if (val.length === 0) return '1 = 1'; // NOT IN () excludes nothing - return `${col} NOT IN (${val.map((v) => bind(params, v)).join(', ')})`; + // [#5298] NULL-safe: "not among this list" holds vacuously for a value + // that is not there. + return nullSafeNegative(col, `${col} NOT IN (${val.map((v) => bind(params, v)).join(', ')})`); } case '$between': { if (!Array.isArray(val) || val.length !== 2) throw readScopeCompileError(`[read-scope-sql] $between for "${field}" needs [min,max] (fail-closed).`); @@ -355,7 +384,9 @@ function compileOperator(col: string, op: string, val: unknown, field: string, p // [#5567] The comparand is a LITERAL, so it is escaped and the escape // character is bound with it. See {@link bindLike}. case '$contains': return `${col} LIKE ${bindLike(params, likePattern('contains', val))}`; - case '$notContains': return `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`; + // [#5298] NULL-safe: `NOT LIKE` is UNKNOWN for a NULL column, and "does not + // contain" is true of a value that is not there. + case '$notContains': return nullSafeNegative(col, `${col} NOT LIKE ${bindLike(params, likePattern('contains', val))}`); case '$startsWith': return `${col} LIKE ${bindLike(params, likePattern('starts', val))}`; case '$endsWith': return `${col} LIKE ${bindLike(params, likePattern('ends', val))}`; case '$null': return val ? `${col} IS NULL` : `${col} IS NOT NULL`; diff --git a/packages/spec/src/data/filter-logic-conformance.ts b/packages/spec/src/data/filter-logic-conformance.ts index 73d860ede2..5c7a70e3c8 100644 --- a/packages/spec/src/data/filter-logic-conformance.ts +++ b/packages/spec/src/data/filter-logic-conformance.ts @@ -6,7 +6,7 @@ * * ## Why this exists * - * `FilterCondition` is evaluated by five independent implementations, and they + * `FilterCondition` is evaluated by SEVEN independent implementations, and they * had drifted: * * | Backend | Where | @@ -16,13 +16,26 @@ * | Record-at-a-time evaluator | `formula` `matchesFilterCondition` (RLS write-side `check`) | * | Read-scope SQL lowering | `service-analytics` `read-scope-sql` | * | MongoDB query translator | `driver-mongodb` `translateFilter` | + * | Turso REMOTE filter compiler | `driver-turso` `RemoteTransport.buildWhereSQL` | + * | Cube filter lowering | `service-analytics` `filter-normalizer` | * - * #3774 said "four" and enrolled four: `translateFilter` was missed, not - * excluded, and ran unchecked against this standard until #4405 — the one - * backend whose target language has no document-level `$not` at all, so a - * negation leaves it as `$nor`. `driver-sqlite-wasm` runs the table too; it - * *inherits* the SQL compiler, so what its suite adds is the sql.js engine - * executing the compiled predicate rather than a sixth way of building one. + * The count has been wrong twice, in the same direction, and both corrections + * cost a real divergence first. #3774 said "four" and enrolled four: + * `translateFilter` was missed, not excluded, and ran unchecked against this + * standard until #4405 — the one backend whose target language has no + * document-level `$not` at all, so a negation leaves it as `$nor`. Then the + * header said "five" while two more had already been enrolled as harnesses: + * `RemoteTransport.buildWhereSQL` and `filter-normalizer` are each a hand-written + * emitter with its own operator vocabulary and its own combinator nesting, and + * #5298 measured both answering the NULL family differently from the other five. + * + * The lesson is worth more than the number: "does a suite for it exist" is not + * the same question as "is it an independent implementation". `driver-sqlite-wasm` + * and `driver-turso` LOCAL run the table too and are NOT on this list — both + * *inherit* `SqlDriver`, so what their suites add is a real engine executing the + * compiled predicate rather than another way of building one. Turso REMOTE is on + * the list precisely because it inherits nothing, which is also why it is the + * backend this table keeps catching (#5590, #5769, #5903). * * In #3774 the SQL compiler OR-ed the contents *within* a `$or` branch instead * of AND-ing them, so every `$or` filter matched more rows than it should — @@ -45,21 +58,28 @@ * > combine. * * The predicates are deliberately boring: string equality, `$in`, `$ne`, `$gte` - * / `$lt` on lexicographic strings. Nothing here exercises null handling, dates, - * numeric coercion, `LIKE` escaping, or case sensitivity — those legitimately - * differ between a SQL engine and a JS matcher, and folding them in would make - * the table unpassable rather than more useful. Keep it that way: a case belongs - * here only if **every** backend must agree on it. + * / `$lt` on lexicographic strings. Dates, numeric coercion, `LIKE` escaping and + * case sensitivity are still out — those legitimately differ between a SQL + * engine and a JS matcher, and folding them in would make the table unpassable + * rather than more useful. Keep it that way: a case belongs here only if + * **every** backend must agree on it. + * + * **Null handling is IN, as of #5298** — it used to be excluded by the same + * sentence, on the assumption that a three-valued SQL engine and a two-valued JS + * matcher could not be held to one answer. Two rulings removed that assumption: + * #5146 made `$not` NULL-safe and #5298 did the same for the non-negated + * `$ne` / `$nin` / `$notContains`, so "the column has no value" now has ONE + * cross-backend answer and belongs to the standard like any other. The + * {@link FilterLogicRow.d} column carries it; see the `d`-column cases below, + * and family 2 of the next section for the rows still waiting on a backend. * * ## Case families that are RULED but not yet enrolled * * Both remaining families were ruled by the maintainer and are implemented in * some backends. Neither is in the table yet — a red row here does not enforce * a ruling, it just turns another lane's unfinished work into this table's - * failure, and each family still has one blocker standing, named per family - * below. Both were re-measured at the 2026-08-05 sync against `cdfbee2f0`, so - * the next author does not have to re-measure. Add the rows in the PR that - * closes the gap, not before. + * failure, and each family still has a blocker standing, named per family + * below. Add the rows in the PR that closes the gap, not before. * * (Family 1 of this note — the boolean identities of the empty combinators — * is GONE because it graduated: the #5322 ruling took the identity reduction, @@ -67,20 +87,34 @@ * {@link FILTER_LOGIC_CASES} below, enrolled on every backend. The family * numbering of the two that remain is kept as their historical ids.) * - * ### 2. NULL-safe `$not` (#5146) + * ### 2. NULL-safe negation — `$not` (#5146) and `$ne`/`$nin`/`$notContains` (#5298) + * + * A row whose column has no value does not satisfy the negated condition and IS + * returned. One family, not two: #5298 ruled the non-negated operators the same + * way #5146 ruled `$not`, so the rows land together or not at all. * - * A row whose column is NULL does not satisfy the negated condition and IS - * returned. Landed in `driver-sql` via PR #5296; `driver-memory`, `formula`, - * `driver-sqlite-wasm` and `driver-mongodb` already agreed; `read-scope-sql` - * was aligned by #5326 (closing #5297) and `filter-normalizer` by #5335 - * (closing #5325), so as of the 2026-08-05 sync (`cdfbee2f0`) every surface - * answers this family the same way — no backend blocker remains. + * 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: * - * What still keeps it out of the table is the fixture: every column of - * {@link FILTER_LOGIC_ROWS} is non-null by construction, so a NULL-bearing - * column has to be added here AND declared in all seven harnesses that seed - * it. That is the whole remaining work item, and it is why this family is not - * a one-line addition. + * | 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`. + * + * `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 + * `SqlDriver`, so local mode inherited #5146 for free; remote mode compiles + * filters in `RemoteTransport.buildWhereSQL`, an independent emitter that + * inherited none of it. One driver, two answers, chosen by connection mode — + * exactly what this table exists to catch, and exactly what it could not see + * while the fixture had no nullable column. #5903 carries the fix. * * ### 3. `{ field: {} }` — a field constrained by zero operators (#5240) * @@ -101,7 +135,10 @@ import type { FilterCondition } from './filter.zod'; -/** A row in the conformance fixture. All columns are plain strings. */ +/** + * A row in the conformance fixture. Every column is a plain string except + * {@link FilterLogicRow.d}, which is nullable — see below. + */ export interface FilterLogicRow { id: string; /** 2x2 truth table over (a, b) — see {@link FILTER_LOGIC_ROWS}. */ @@ -109,6 +146,22 @@ export interface FilterLogicRow { b: string; /** Constant across every row; a predicate on it never changes a result. */ c: string; + /** + * [#5298] The NULL-bearing column — the fixture's only one, and the whole + * reason the null family could not be enrolled before it existed. + * + * Rows 1-2 carry a value (`v1`, `v2`), rows 3-4 are NULL. Harnesses MUST + * declare it nullable in their DDL / schema: a `NOT NULL` column, or a seed + * that substitutes `''` for `null`, turns every case below green for the + * wrong reason — the divergence these cases exist to catch is exactly what a + * row with no value does, so a fixture without one measures nothing. + * + * Deliberately a separate column rather than nulling part of `a`/`b`: the + * (a, b) truth table is what makes a wrongly-OR-ed pair of predicates show up + * as extra ids, and punching a hole in it would weaken every combinator case + * to pay for the null cases. + */ + d: string | null; /** Record-scope columns, for the shapes read scopes are actually written in. */ owner: string; status: string; @@ -119,14 +172,16 @@ export interface FilterLogicRow { /** * The fixture. Rows 1-4 are the 2x2 truth table over `(a, b)` — every * combination appears exactly once, so a wrongly-OR-ed pair of predicates always - * shows up as extra ids rather than by luck of the data. Rows 5-8 carry the - * record-scope columns used by the read-scope cases. + * shows up as extra ids rather than by luck of the data. The same four rows + * carry the record-scope columns used by the read-scope cases, and (since + * #5298) the nullable `d`: valued on rows 1-2, NULL on rows 3-4, so a filter + * that silently drops no-value rows loses exactly half the table. */ export const FILTER_LOGIC_ROWS: readonly FilterLogicRow[] = [ - { id: '1', a: 'x', b: 'y', c: 'z', owner: 'u1', status: 'active', parent_object: 'case', parent_id: 'c1' }, - { id: '2', a: 'x', b: 'zz', c: 'z', owner: 'u1', status: 'archived', parent_object: 'case', parent_id: 'c2' }, - { id: '3', a: 'qq', b: 'y', c: 'z', owner: 'u2', status: 'active', parent_object: 'todo', parent_id: 't1' }, - { id: '4', a: 'qq', b: 'zz', c: 'z', owner: 'u2', status: 'archived', parent_object: 'todo', parent_id: 'c1' }, + { id: '1', a: 'x', b: 'y', c: 'z', d: 'v1', owner: 'u1', status: 'active', parent_object: 'case', parent_id: 'c1' }, + { id: '2', a: 'x', b: 'zz', c: 'z', d: 'v2', owner: 'u1', status: 'archived', parent_object: 'case', parent_id: 'c2' }, + { id: '3', a: 'qq', b: 'y', c: 'z', d: null, owner: 'u2', status: 'active', parent_object: 'todo', parent_id: 't1' }, + { id: '4', a: 'qq', b: 'zz', c: 'z', d: null, owner: 'u2', status: 'archived', parent_object: 'todo', parent_id: 'c1' }, ] as const; /** One conformance case: a filter and the ids it must match, in id order. */ @@ -253,6 +308,31 @@ export const FILTER_LOGIC_CASES: readonly FilterLogicCase[] = [ note: '#5322: emitting nothing for it runs the query UNSCOPED — on an RLS lowering that is a permission bypass (#5297).', }, + // ── NULL / no-value semantics (#5298) ───────────────────────────────────── + // + // Rows 3 and 4 have no `d`, and these two cases are what makes that true of + // every harness: a fixture that quietly stored `''` instead of NULL, or a + // `NOT NULL` column that rejected the seed, fails HERE rather than by turning + // some later case green for the wrong reason. That is their first job — they + // are the control the `$ne` / `$not` rows will lean on when those land. + // + // Only the `$null` partition is enrolled. The `$ne` and `$not` rows that + // belong beside it are written out in the module doc above, under "RULED but + // not yet enrolled" — two backends cannot answer them yet, and enrolling a + // row two lanes have to go fix is how this table stops meaning anything. + { + name: '$null true selects exactly the no-value rows', + filter: { d: { $null: true } }, + expected: ['3', '4'], + note: 'The control for the two above: it pins WHICH rows have no value, so a case that returns 3-4 cannot be passing because the seed lost a value it should have kept.', + }, + { + name: '$null false selects exactly the valued rows', + filter: { d: { $null: false } }, + expected: ['1', '2'], + note: 'The complement, so `$null` is pinned as a partition of the table rather than one half of one. Together with the row-count control this makes a NOT NULL fixture column fail loudly instead of quietly passing everything.', + }, + // ── Shapes read scopes are actually written in ──────────────────────────── { name: 'read scope: own AND active, OR another owner\'s row',