diff --git a/.changeset/analytics-empty-combinator-identity.md b/.changeset/analytics-empty-combinator-identity.md new file mode 100644 index 0000000000..1220c38283 --- /dev/null +++ b/.changeset/analytics-empty-combinator-identity.md @@ -0,0 +1,32 @@ +--- +"@objectstack/service-analytics": patch +"@objectstack/spec": patch +--- + +fix(service-analytics): 空 `$and` / `$or` 按布尔单位元归约,两个编译器与五后端对齐 (#5322) + +同一个仓库对空组合子曾有两个对立答案:五个 `FILTER_LOGIC_CASES` 后端 +(`driver-sql` #5134/PR #5243、`driver-memory`、`formula`、`driver-sqlite-wasm`、 +`driver-mongodb` #5239)把 `{ $and: [] }` / `{ $or: [] }` 归约成布尔单位元,而 +service-analytics 的两个编译器 —— `read-scope-sql.ts` 的 `compileNode` 与 +`filter-normalizer.ts` 的 `buildNode` —— 成文地 fail-closed 抛错("An empty +combinator has no defensible reading…"),并有 pin 测试钉住。2026-08-04 维护者拍板 +(#5322)取单位元,本次把两处对齐: + +- `{ $and: [] }` = TRUE(全部行,AND 单位元);`{ $or: [] }` = FALSE(零行,OR + 单位元)。嵌套可归约:空组合子作 `$or` 分支时按 TRUE 吸收/FALSE 退出析取,作 + `$not` 操作数时取反(`{$not: {$and: []}}` = 零行、`{$not: {$or: []}}` = 全部 + 行)。`{}` = TRUE 与 `{ $not: {} }` = 零行两格已由 #5297(read-scope)/#5325 + (normalizer)先行落地,本次连同这四格由同一张一致性表钉住。 +- **迁移含义**:过去发出空组合子的调用方收到的是抛错(REST 面上是一次失败的请 + 求);现在按上表求值。`{ $or: [] }` 在 RLS/图表场景是 fail-closed 的 —— 析取列 + 表循环出零项时隐藏全部行,而不是放行全表。写作期对字面量空组合子的响亮拒收另立 + #5330(publish/lint),不在运行期。 +- **没有放宽的部分**:非数组的 `$and`/`$or`、非对象的分支、非对象的 `$not` 操作数 + 仍然抛错(#5325 的形状拒收原样保留)。归约让「无约束」成为有意义的裁决,静默把 + 畸形分支读成 TRUE 会让垃圾析取项吸收 `$or` 而放宽查询,所以畸形形状保持响亮。 +- 归约与 #5146/#5325 的 NULL-safe `$not` 重写的组合语义是「先归约、后 NULL-safe」 + —— 常量归约出的单位元不受重写影响,幸存的叶子照常加守卫,有测试钉住。 +- `packages/spec`:`FILTER_LOGIC_CASES` 补四条布尔单位元行(空 `$and`、空 `$or`、 + `{}` 析取项吸收、`{$not: {}}`),两个 analytics conformance suite 与五后端从此 + 被同一张表钉住这四格。 diff --git a/packages/rest/src/analytics-filter-refusal-envelope.test.ts b/packages/rest/src/analytics-filter-refusal-envelope.test.ts index 343f46c4bf..c273e3827b 100644 --- a/packages/rest/src/analytics-filter-refusal-envelope.test.ts +++ b/packages/rest/src/analytics-filter-refusal-envelope.test.ts @@ -96,16 +96,33 @@ function buildRoute(analyticsProvider?: any) { /** * A REAL `AnalyticsService` on the ObjectQL aggregate path. * - * `executeAggregate` returns a fixed bucket, so a query that gets far enough to - * touch data succeeds — which is what makes the refusal cases meaningful: they - * fail on the FILTER, on a route that demonstrably answers 200 otherwise. + * `executeAggregate` evaluates the engine-side filter it receives over one + * fixed bucket, so a query that gets far enough to touch data succeeds — which + * is what makes the refusal cases meaningful: they fail on the FILTER, on a + * route that demonstrably answers 200 otherwise. It is filter-AWARE (not a + * constant) so the #5322 identity cases are load-bearing too: the zero-row + * constant — `{$not: {}}`, the spelling `filterNodeToCondition` emits for + * FALSE — must come back as 200 with NO rows, distinguishable from both a 400 + * and from an ignored filter. */ function realAnalytics(): AnalyticsService { const silent: Logger = { debug() {}, info() {}, warn() {}, error() {} }; + const bucket = { stage: 'won', revenue: 100 }; + const matches = (cond: Record): boolean => + Object.entries(cond).every(([key, value]) => { + if (key === '$and') return (value as Record[]).every(matches); + if (key === '$or') return (value as Record[]).some(matches); + if (key === '$not') return !matches(value as Record); + if (value !== null && typeof value === 'object' && '$eq' in (value as object)) { + return (bucket as Record)[key] === (value as { $eq: unknown }).$eq; + } + return (bucket as Record)[key] === value; + }); return new AnalyticsService({ logger: silent, queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), - executeAggregate: async () => [{ stage: 'won', revenue: 100 }], + executeAggregate: async (_object: string, options: { filter?: Record }) => + matches(options?.filter ?? {}) ? [{ ...bucket }] : [], isRegisteredObject: () => true, }); } @@ -163,9 +180,14 @@ describe('[#5352] POST /analytics/dataset/query — a filter refusal reaches the message: /needs a two-element \[min, max\] array/, }, { - name: 'an empty $or', - runtimeFilter: { $or: [] }, - message: /"\$or" requires a non-empty array/, + // FLIPPED with the #5322 ruling (2026-08-04): this entry was `{$or: []}` + // pinning the "requires a non-empty array" refusal. The empty array is + // now the OR identity — FALSE, zero rows, asserted in the #5322 block + // below — so the refusal that survives at the same guard site is the + // non-array spelling, same envelope. + name: 'an $or that is not an array', + runtimeFilter: { $or: 'won' }, + message: /"\$or" requires an array of filter objects/, }, { name: 'an $or branch that is not a filter object', @@ -195,6 +217,57 @@ describe('[#5352] POST /analytics/dataset/query — a filter refusal reaches the } }); +describe('[#5322] empty combinators are boolean identities at the REST face — evaluated, not refused', () => { + // Until the 2026-08-04 #5322 ruling, `{$or: []}` sat in REFUSALS above and + // this route answered it 400 ("requires a non-empty array"). The ruling took + // the identity reduction the five FILTER_LOGIC_CASES backends already gave: + // these four shapes are ANSWERS now, so each asserts its 200 AND its row + // semantics — the row count is what separates the two identities from each + // other and from a filter that was silently dropped. + const IDENTITIES: Array<{ name: string; runtimeFilter: unknown; rows: unknown[] }> = [ + { + // FALSE — the OR identity. Zero rows is the fail-closed direction: a + // disjunct list that looped to zero items hides the data, it does not + // chart the whole dataset (#5134). + name: 'an empty $or → the zero-row constant', + runtimeFilter: { $or: [] }, + rows: [], + }, + { + // TRUE — the AND identity: a conjunction of zero conditions constrains + // nothing, so the bucket comes back. + name: 'an empty $and → no constraint', + runtimeFilter: { $and: [] }, + rows: [{ stage: 'won', revenue: 100 }], + }, + { + // NOT TRUE ≡ FALSE (#5325's square, crossing this seam). + name: 'a $not of {} → the zero-row constant', + runtimeFilter: { $not: {} }, + rows: [], + }, + { + // A `{}` disjunct is TRUE and ABSORBS the $or: every row, NOT the + // narrowed `stage = lost` branch (which would return zero rows here — + // the bucket is stage 'won' — so absorption and narrowing are + // distinguishable in this fixture). + name: 'a {} disjunct absorbs its $or', + runtimeFilter: { $or: [{ stage: 'lost' }, {}] }, + rows: [{ stage: 'won', revenue: 100 }], + }, + ]; + + for (const c of IDENTITIES) { + it(`${c.name} → 200, rows ${JSON.stringify(c.rows.length)}`, async () => { + const route = buildRoute(async () => realAnalytics()); + const res = await post(route, { dataset, selection: { ...selection, runtimeFilter: c.runtimeFilter } }); + expect(res.statusCode).toBe(200); + expect(res.body.code).toBeUndefined(); + expect(res.body.rows).toEqual(c.rows); + }); + } +}); + describe('[#5352] the message-sniffing fallback still classifies the families that carry no envelope', () => { // Every entry of the route's regex list, produced as its owner produces it: // a bare `Error`. Re-verified unenveloped while #5352 was implemented — diff --git a/packages/services/service-analytics/src/__tests__/empty-combinator-identity.test.ts b/packages/services/service-analytics/src/__tests__/empty-combinator-identity.test.ts new file mode 100644 index 0000000000..3d4802d4dc --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/empty-combinator-identity.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5322] Empty combinators reduce to their boolean identities in the + * analytics filter normalizer — `{$and: []}` = TRUE, `{$or: []}` = FALSE — + * matching the five `FILTER_LOGIC_CASES` backends row for row. Together with + * the `{}` / `{$not: {}}` identities #5325 already gave this module, the + * boolean algebra over the combinators is now complete. + * + * # The history this file flips + * + * Until the 2026-08-04 #5322 ruling, `buildNode` REFUSED the empty arrays. + * Its error message argued the opposite position, verbatim: + * + * > `"$and" requires a non-empty array. An empty combinator has no defensible + * > reading — dropping it widens the query, and treating it as "match + * > nothing" silently empties a chart.` + * + * "Treating it as match nothing" is exactly what #5134 ruled for `$or: []` + * and what `driver-sql` / `driver-memory` / `formula` / `driver-sqlite-wasm` + * / `driver-mongodb` (#5239) implement. The ruling took the reduction because + * only a reduction can evaluate a NESTED tree (a rejection must first reduce + * to decide whether `$and: []` inside a `$or` branch is an error — which + * concedes the point), and because `{$or: []}` = zero rows is fail-closed + * where it matters: a scope whose disjunct list loops to zero items hides + * every row rather than widening to the whole table. The loud authoring-time + * rejection of the literal spellings lives on as #5330 (publish/lint), not as + * runtime behavior. + * + * # What deliberately did NOT loosen + * + * Non-array `$and`/`$or` still throws (this file), as do non-object branches + * and non-object `$not` operands (pinned in + * `filter-normalizer-not-null-safe.test.ts`): reduction makes `null` ("no + * constraint") a meaningful verdict, so silently mapping junk to it would let + * a malformed disjunct ABSORB its `$or` and widen the query — the exact + * failure mode the old error message feared, reachable only through the + * lenient path. + * + * Row-level conformance for the four ruled shapes lives in the shared table + * (`filter-logic-conformance.ts`), executed against a real SQLite engine by + * `native-sql-filter-logic-conformance.test.ts` and + * `read-scope-sql-conformance.test.ts`. This file pins the TREE the + * normalizer produces and the seam where the ObjectQL engine path receives + * the boolean constant. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; + +import { + normalizeAnalyticsFilterTree, + collectFilterLeaves, +} from '../strategies/filter-normalizer.js'; +import { AnalyticsService } from '../analytics-service.js'; + +const tree = (where: unknown) => normalizeAnalyticsFilterTree({ where }); + +const FALSE_NODE = { kind: 'const', value: false }; +const TRUE_NODE = { kind: 'const', value: true }; + +describe('[#5322] buildNode reduces empty combinators to boolean identities', () => { + it('`{$and: []}` is TRUE — no constraint', () => { + expect(tree({ $and: [] })).toBeNull(); + }); + + it('`{$or: []}` is FALSE — the zero-row constant', () => { + expect(tree({ $or: [] })).toEqual(FALSE_NODE); + }); + + it('`$not` negates the REDUCED operand, in both directions', () => { + expect(tree({ $not: { $and: [] } })).toEqual(FALSE_NODE); // NOT TRUE ≡ FALSE + expect(tree({ $not: { $or: [] } })).toEqual(TRUE_NODE); // NOT FALSE ≡ TRUE + }); + + it('an empty-combinator branch carries its identity into the enclosing combinator', () => { + // A `{$and: []}` disjunct is TRUE and ABSORBS the whole `$or` — collapsing + // to the surviving branches instead is the narrowing #5325 fixed for the + // literal `{}` disjunct. + expect(tree({ $or: [{ a: 'x' }, { $and: [] }] })).toBeNull(); + // A `{$or: []}` conjunct is FALSE; the compiled conjunction carries the + // constant (row-set: zero rows — pinned via SQL in the conformance suite). + expect(JSON.stringify(tree({ $and: [{ a: 'x' }, { $or: [] }] }))).toContain('"value":false'); + expect(JSON.stringify(tree({ a: 'x', $or: [] }))).toContain('"value":false'); + }); + + it('the FALSE constant touches no member', () => { + expect(collectFilterLeaves(tree({ $or: [] }))).toEqual([]); + expect(collectFilterLeaves(tree({ $and: [{ $or: [] }] }))).toEqual([]); + }); + + it('non-array `$and`/`$or` still throws — #5322 loosened only the EMPTY array', () => { + expect(() => tree({ $and: 'x' })).toThrow(/requires an array/); + expect(() => tree({ $or: { a: 1 } })).toThrow(/requires an array/); + }); +}); + +// ── The engine-path seam: FALSE reaches ObjectQL as a real zero-row filter ── + +const dataset = DatasetSchema.parse({ + name: 'incidents', + label: 'Incidents', + object: 'incident', + dimensions: [{ name: 'severity', field: 'severity', type: 'string' }], + measures: [{ name: 'incident_count', aggregate: 'count' }], +}); + +const ROWS: Array<{ severity: string }> = [ + { severity: 'high' }, + { severity: 'high' }, + { severity: 'low' }, +]; + +/** + * Stand-in for `engine.aggregate`, mirroring how a driver receives the + * filter: `{$not: {}}` — the spelling `filterNodeToCondition` uses for the + * FALSE constant, because `formula` and `driver-memory` already pin it as the + * zero-row filter (#5134) — matches nothing, and an absent/empty filter + * matches everything. + */ +function makeEngine(captured: Array<{ filter?: Record }>) { + const matches = (row: Record, cond: Record): boolean => + Object.entries(cond).every(([key, value]) => { + if (key === '$and') return (value as Record[]).every((c) => matches(row, c)); + if (key === '$or') return (value as Record[]).some((c) => matches(row, c)); + if (key === '$not') return !matches(row, value as Record); + return row[key] === value; + }); + return async ( + _object: string, + options: { groupBy?: string[]; filter?: Record }, + ): Promise>> => { + captured.push({ filter: options.filter }); + const filtered = ROWS.filter((row) => matches(row, options.filter ?? {})); + return [{ incident_count: filtered.length }]; + }; +} + +describe('[#5322] the ObjectQL path hands the engine the constant, not silence', () => { + it('`{$or: []}` arrives as the zero-row `{$not: {}}` and counts zero rows', async () => { + const captured: Array<{ filter?: Record }> = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: makeEngine(captured), + }); + + const result = await svc.queryDataset!(dataset, { + measures: ['incident_count'], + runtimeFilter: { $or: [] }, + }); + + // The constant reached the engine as a real zero-row condition — NOT as an + // absent filter, which every driver reads as "every row". + expect(captured).toHaveLength(1); + expect(JSON.stringify(captured[0].filter)).toContain('"$not":{}'); + expect(result.rows).toEqual([{ incident_count: 0 }]); + }); + + it('`{$and: []}` arrives as no constraint and counts every row', async () => { + const captured: Array<{ filter?: Record }> = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: makeEngine(captured), + }); + + const result = await svc.queryDataset!(dataset, { + measures: ['incident_count'], + runtimeFilter: { $and: [] }, + }); + + expect(JSON.stringify(captured[0].filter ?? {})).not.toContain('$and'); + expect(result.rows).toEqual([{ incident_count: 3 }]); + }); + + it('a `{$and: []}` disjunct absorbs its `$or` instead of narrowing to the other branch', async () => { + const captured: Array<{ filter?: Record }> = []; + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: makeEngine(captured), + }); + + const result = await svc.queryDataset!(dataset, { + measures: ['incident_count'], + runtimeFilter: { $or: [{ severity: 'high' }, { $and: [] }] }, + }); + + // Narrowing to `severity = high` would count 2 — the #5297/#5325 seam. + expect(JSON.stringify(captured[0].filter ?? {})).not.toContain('severity'); + expect(result.rows).toEqual([{ incident_count: 3 }]); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts index 0194b4403d..70488b115f 100644 --- a/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts @@ -526,16 +526,29 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit // ── Nothing that failed closed stopped failing closed ───────────────────── describe('the fail-closed guarantees survive the rewrite', () => { - it('an empty `$and` / `$or` still THROWS — #5322 is its own ruling', async () => { - // The empty-combinator square is decided separately (#5322). This change - // must not quietly turn either of them into a boolean identity on the way - // past, so both stay pinned on the THROWING side, inside a `$not` as well - // as outside. - await expect(ids({ $and: [] })).rejects.toThrowError(/non-empty array/); - await expect(ids({ $or: [] })).rejects.toThrowError(/non-empty array/); - await expect(ids({ $not: { $and: [] } })).rejects.toThrowError(/non-empty array/); - await expect(ids({ $not: { $or: [] } })).rejects.toThrowError(/non-empty array/); - await expect(ids({ $or: [{ stage: 'won' }, { $and: [] }] })).rejects.toThrowError(/non-empty array/); + it('an empty `$and` / `$or` reduces to its boolean identity, inside a `$not` as well as outside (#5322)', async () => { + // FLIPPED pin. When this file was written the empty-combinator square was + // still an open ruling, so all five shapes were pinned on the THROWING + // side (`toThrowError(/non-empty array/)`). The 2026-08-04 #5322 ruling + // took the boolean identities, and the pins flipped with it: `{$and: []}` + // is TRUE, `{$or: []}` is FALSE, and — the half that survives from the + // old pin's intent — the `$not` negates the REDUCED operand rather than + // quietly changing the answer on the way past. + await expect(ids({ $and: [] })).resolves.toEqual(ALL); // TRUE — the AND identity + await expect(ids({ $or: [] })).resolves.toEqual([]); // FALSE — the OR identity + await expect(ids({ $not: { $and: [] } })).resolves.toEqual([]); // NOT TRUE ≡ FALSE + await expect(ids({ $not: { $or: [] } })).resolves.toEqual(ALL); // NOT FALSE ≡ TRUE + // A `{$and: []}` disjunct is a TRUE branch and ABSORBS its `$or` — + // exactly as the literal `{}` disjunct does two blocks up. + await expect(ids({ $or: [{ stage: 'won' }, { $and: [] }] })).resolves.toEqual(ALL); + // The other direction: a `{$or: []}` disjunct is FALSE, the OR identity — + // the disjunction collapses to its real branch, NULL-safety intact. + await expect(ids({ $not: { $or: [{ stage: 'won' }, { $or: [] }] } })).resolves.toEqual(['2', '3', '4']); + }); + + it('a non-array `$and` / `$or` still THROWS — #5322 loosened only the EMPTY array', async () => { + await expect(ids({ $and: 'x' })).rejects.toThrowError(/requires an array/); + await expect(ids({ $or: { stage: 'won' } })).rejects.toThrowError(/requires an array/); }); it('an unknown operator inside a `$not` still THROWS rather than being guarded', async () => { diff --git a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts index 92d8897ba5..8d52fb6c24 100644 --- a/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts +++ b/packages/services/service-analytics/src/__tests__/filter-refusal-envelope.test.ts @@ -85,16 +85,22 @@ const REFUSALS: Array<{ name: string; where: unknown; message: RegExp; issueBull message: /needs a two-element \[min, max\] array/, issueBullet: true, }, + // FLIPPED with the #5322 ruling: these two entries were "$and/$or with an + // empty array". The refusing SITE is unchanged (the combinator guard #5352's + // body bulleted, hence issueBullet stays true) but its empty-array half + // graduated to a boolean identity — `{$and: []}` = TRUE, `{$or: []}` = FALSE, + // asserted in ACCEPTED below — so the site's remaining refusal is the + // non-array spelling, still carrying the same envelope. { - name: '$and with an empty array', - where: { $and: [] }, - message: /"\$and" requires a non-empty array/, + name: '$and that is not an array', + where: { $and: 'won' }, + message: /"\$and" requires an array of filter objects/, issueBullet: true, }, { - name: '$or with an empty array', - where: { $or: [] }, - message: /"\$or" requires a non-empty array/, + name: '$or that is not an array', + where: { $or: { stage: 'won' } }, + message: /"\$or" requires an array of filter objects/, issueBullet: true, }, { @@ -176,6 +182,20 @@ const ACCEPTED: Array<{ name: string; where: unknown; tree: unknown }> = [ where: {}, tree: null, }, + { + // #5322: the AND identity — a conjunction of zero conditions constrains + // nothing. Was in REFUSALS ("requires a non-empty array") until the ruling. + name: 'an empty $and as TRUE (#5322)', + where: { $and: [] }, + tree: null, + }, + { + // #5322: the OR identity — a disjunction of zero conditions matches + // nothing. Fail-closed for a scope whose disjunct list looped to zero items. + name: 'an empty $or as the FALSE constant (#5322)', + where: { $or: [] }, + tree: { kind: 'const', value: false }, + }, { // #5334: `[]` is "no filter", not a failed filter. name: 'an empty `where` array as "no filter" (#5334)', 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 deb966dec9..bcb7ec837d 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 @@ -24,7 +24,9 @@ * what makes it a bypass rather than a wrong string, which is why the assertion * below goes through `NativeSQLStrategy.generateSql` and not only through * `compileScopedFilterToSql`. `$and: []` / `$or: []` in the same loop were - * fail-closed all along; `$not` was the one uncovered square. + * fail-closed when this file was written; #5322 has since ruled them boolean + * identities (TRUE / FALSE), and the fail-closed pin at the bottom of this + * file flipped with that ruling. * * **`$not` was not NULL-safe.** SQL is three-valued and a `WHERE` keeps only * TRUE, so `NOT (stage = 'won')` dropped every row whose `stage` is NULL, while @@ -373,17 +375,31 @@ describe('[#5297] read-scope `$not` — boolean identities and NULL safety', () }); describe('the fail-closed guarantees survive the rewrite', () => { - it('an empty `$and` / `$or` still THROWS, inside a `$not` as well as outside', () => { - expect(() => compileScopedFilterToSql({ $and: [] } as FilterCondition, ALIAS)) - .toThrowError(/non-empty array/); - expect(() => compileScopedFilterToSql({ $or: [] } as FilterCondition, ALIAS)) - .toThrowError(/non-empty array/); - // The empty-combinator square is its own ruling (#5322) — the rewrite must - // not quietly turn either of them into a boolean identity on the way past. - expect(() => compileScopedFilterToSql({ $not: { $or: [] } } as FilterCondition, ALIAS)) - .toThrowError(/non-empty array/); - expect(() => compileScopedFilterToSql({ $not: { $and: [] } } as FilterCondition, ALIAS)) - .toThrowError(/non-empty array/); + it('an empty `$and` / `$or` reduces to its boolean identity, inside a `$not` as well as outside (#5322)', () => { + // FLIPPED pin. This block used to assert `toThrowError(/non-empty + // array/)` four times: when it was written, the empty-combinator square + // was still an open ruling (#5322) and the rewrite was required not to + // change the answer on the way past. The 2026-08-04 ruling took the + // boolean identities, so the pinned answers flipped with it — and the + // second half of the old requirement still holds in its new form: the + // `$not` negates the REDUCED operand. + expect(ids({ $and: [] })).toEqual(ALL); // TRUE — the AND identity + expect(ids({ $or: [] })).toEqual([]); // FALSE — the OR identity + expect(compileScopedFilterToSql({ $or: [] } as FilterCondition, ALIAS)) + .toEqual({ sql: '1 = 0', params: [] }); + expect(ids({ $not: { $or: [] } })).toEqual(ALL); // NOT FALSE ≡ TRUE + expect(ids({ $not: { $and: [] } })).toEqual([]); // NOT TRUE ≡ FALSE + }); + + it('reduction composes with the NULL-safe rewrite: identities first, surviving leaves stay guarded (#5322)', () => { + // The FALSE disjunct drops out (the OR identity), leaving + // `{$not: {$or: [{stage: 'won'}]}}` — whose leaf the #5146 rewrite still + // totalises, so the NULL-stage rows 3 and 4 are returned exactly as the + // plain `{$not: {stage: 'won'}}` pin above returns them. + expect(ids({ $not: { $or: [{ stage: 'won' }, { $or: [] }] } })).toEqual(['2', '3', '4']); + // The TRUE disjunct ABSORBS the `$or`, so the whole `$not` is NOT TRUE — + // zero rows — and no leaf survives for the rewrite to guard. + expect(ids({ $not: { $or: [{ stage: 'won' }, { $and: [] }] } })).toEqual([]); }); it('an unknown operator inside a `$not` still THROWS rather than being guarded', () => { 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 958771a6ff..be5f53e1a5 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 @@ -106,7 +106,12 @@ describe('compileScopedFilterToSql — filter logic conformance', () => { const { sql, params } = compileScopedFilterToSql(c.filter, ALIAS); // The compiler returns a boolean expression, exactly as the analytics // query builder splices it — including the unparenthesized top level. - const stmt = db.prepare(`SELECT "id" FROM "t" AS "${ALIAS}" WHERE ${sql} ORDER BY "id"`); + // `''` is the compiler's TRUE (#5322: `{$and: []}` and an absorbed `$or` + // compile to it), the shape for which `applyReadScope` adds no `WHERE` + // at all — so it executes here as the unconstrained query it stands for. + const stmt = db.prepare( + `SELECT "id" FROM "t" AS "${ALIAS}" WHERE ${sql.length > 0 ? sql : '1 = 1'} ORDER BY "id"`, + ); stmt.bind(params as any[]); const got: string[] = []; while (stmt.step()) got.push(String(stmt.get()[0])); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-sql.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-sql.test.ts index a5cae29d06..4c03d540ec 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-sql.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-sql.test.ts @@ -82,8 +82,21 @@ describe('compileScopedFilterToSql', () => { expect(() => compileScopedFilterToSql({ account: { region: 'NA' } }, 't')).toThrowError(/nested\/relation value/); }); - it('THROWS on an empty $and (degenerate, fail-closed)', () => { - expect(() => compileScopedFilterToSql({ $and: [] }, 't')).toThrowError(/non-empty array/); + it('empty combinators reduce to their boolean identities (#5322)', () => { + // FLIPPED pin. Until the 2026-08-04 #5322 ruling this asserted + // `toThrowError(/non-empty array/)`: the compiler refused empty + // combinators fail-closed while the five FILTER_LOGIC_CASES backends + // reduced them to identities. The ruling took the reduction, so `''` + // (TRUE) and `1 = 0` (FALSE) are now the pinned answers — matching + // `driver-sql` / `driver-memory` / `formula` / `driver-sqlite-wasm` / + // `driver-mongodb` row for row. + expect(compileScopedFilterToSql({ $and: [] }, 't')).toEqual({ sql: '', params: [] }); + expect(compileScopedFilterToSql({ $or: [] }, 't')).toEqual({ sql: '1 = 0', params: [] }); + }); + + it('still THROWS on a non-array $and/$or (fail-closed — #5322 loosened only the EMPTY array)', () => { + expect(() => compileScopedFilterToSql({ $and: 'x' } as never, 't')).toThrowError(/requires an array/); + expect(() => compileScopedFilterToSql({ $or: { a: 1 } } as never, 't')).toThrowError(/requires an array/); }); it('THROWS on a non-object read scope', () => { diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index c8104cf2ac..cb2b38066f 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -43,6 +43,20 @@ import type { FilterCondition } from '@objectstack/spec/data'; * survives, and FALSE has a spelling ({@link FALSE_CLAUSE}) instead of being * representable only as silence. * + * ## Empty combinators are boolean identities (#5322) + * + * `{$and: []}` is TRUE (every row), `{$or: []}` is FALSE (zero rows), and + * `{$not: {}}` is `NOT TRUE` — FALSE. This compiler used to refuse the empty + * arrays fail-closed while the five `FILTER_LOGIC_CASES` backends reduced + * them; the 2026-08-04 #5322 ruling aligned this file and the analytics + * `filter-normalizer` with the reduction (see the note at the `length === 0` + * branch in {@link compileNode} for why). Reduction happens structurally over + * the whole tree, and it composes with the #5146 NULL-safe `$not` rewrite as + * "reduce first": {@link nullSafeNegationOperand} maps combinator arrays + * element-wise (an empty array stays empty, a `{}` leaf has no field to + * guard), so the identity a constant reduces to is untouched by the rewrite + * and the rewrite only ever guards leaves that survive it. + * * ## `$not` is NULL-safe (#5146) * * SQL is three-valued and a `WHERE` keeps only TRUE, so a bare `NOT (col = ?)` @@ -109,8 +123,23 @@ function compileNode(node: unknown, qAlias: string, params: unknown[]): string { const clauses: string[] = []; for (const [key, value] of Object.entries(node)) { if (key === '$and' || key === '$or') { - if (!Array.isArray(value) || value.length === 0) { - throw new Error(`[read-scope-sql] "${key}" requires a non-empty array (fail-closed).`); + if (!Array.isArray(value)) { + throw new Error(`[read-scope-sql] "${key}" requires an array (fail-closed).`); + } + if (value.length === 0) { + // Boolean identity (#5322 ruling, 2026-08-04): the empty `$and` is the + // AND identity — TRUE, no constraint — and the empty `$or` is the OR + // identity — FALSE, zero rows. Until that ruling this compiler REFUSED + // both ("requires a non-empty array (fail-closed)"), while the five + // FILTER_LOGIC_CASES backends reduced them; #5322 took the reduction: + // it is the only reading that lets a nested tree be evaluated at all + // (a rejection cannot answer what `$and: []` means as the third branch + // of a `$or`), and `{$or: []}` = zero rows is itself fail-closed for an + // RLS scope — a disjunct list that loops to zero items hides every row + // instead of exposing the table (#5134). Authoring-time loud rejection + // of the literal spelling is tracked separately (#5330). + if (key === '$or') clauses.push(FALSE_CLAUSE); + continue; } const compiled = (value as unknown[]).map((child) => compileSub(child, qAlias)); // A `''` branch is the constant TRUE. It ABSORBS a disjunction — one TRUE diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index 86898a54fc..1506449a6b 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -68,6 +68,12 @@ * `objectql-strategy.filterNodeToCondition` and its display-SQL twin * `renderFilterNodeSql`. * + * The EMPTY combinators complete the same boolean algebra (#5322 ruling): + * `{$and: []}` is TRUE and `{$or: []}` is FALSE — this module used to refuse + * both fail-closed while the five `FILTER_LOGIC_CASES` backends reduced them; + * see the note inside {@link buildNode}'s combinator branch for the history + * and the reasoning the ruling adopted. + * * # `$not` is NULL-safe (#5146) * * SQL is three-valued and a `WHERE` keeps only TRUE, so a bare `NOT (col = ?)` @@ -423,9 +429,10 @@ function fieldLeaves(key: string, raw: unknown): NormalizedFilterNode[] { * Every entry of one object ANDs with its siblings, at every depth — the rule * `filter-logic-conformance.ts` exists to hold each backend to (#3774). The * combinator handling deliberately mirrors `read-scope-sql.ts`'s - * `compileNode`, including its fail-closed empty-array rejection AND (since - * #5325) its treatment of the two boolean identities, so the two SQL-producing - * paths in this package cannot drift apart about what a filter MEANS. + * `compileNode` — the `{}`/`{$not: {}}` identities since #5325, and the EMPTY + * `$and`/`$or` identities since the #5322 ruling (see the note at the + * `length === 0` branch) — so the two SQL-producing paths in this package + * cannot drift apart about what a filter MEANS. */ function buildNode(cond: Record): NormalizedFilterNode | null { const children: NormalizedFilterNode[] = []; @@ -434,13 +441,31 @@ function buildNode(cond: Record): NormalizedFilterNode | null { if (raw === undefined) continue; if (key === '$and' || key === '$or') { - if (!Array.isArray(raw) || raw.length === 0) { + if (!Array.isArray(raw)) { throw invalidFilterError( - `[analytics] "${key}" requires a non-empty array. An empty combinator has no ` + - `defensible reading — dropping it widens the query, and treating it as "match ` + - `nothing" silently empties a chart.`, + `[analytics] "${key}" requires an array of filter objects, got ${JSON.stringify(raw)}. ` + + `Dropping it would silently widen the query to rows the filter excludes.`, ); } + if (raw.length === 0) { + // Boolean identity (#5322 ruling, 2026-08-04): the empty `$and` is the + // AND identity — TRUE, no constraint — and the empty `$or` is the OR + // identity — FALSE, zero rows. Until that ruling this function REFUSED + // both; its error message argued, verbatim, that "An empty combinator + // has no defensible reading — dropping it widens the query, and + // treating it as 'match nothing' silently empties a chart" — while the + // five FILTER_LOGIC_CASES backends already reduced them. The ruling + // took the reduction: only a reduction can evaluate a NESTED tree (a + // rejection must first reduce to judge `$and: []` as the third branch + // of a `$or`, which concedes the point), and `{$or: []}` = zero rows + // is fail-closed where it matters — a disjunct list that loops to zero + // items hides every row instead of widening (#5134). Loud + // AUTHORING-time rejection of the literal spellings is #5330's scope. + // Note the guard above did NOT loosen: a non-array `$and`/`$or` still + // throws, as do non-object branches below. + if (key === '$or') children.push(falseNode()); + continue; + } const branches = raw.map((sub) => { // A non-object element is refused rather than skipped: skipping it // NARROWS a `$or` to its remaining branches and, under the TRUE-absorbs diff --git a/packages/spec/src/data/filter-logic-conformance.ts b/packages/spec/src/data/filter-logic-conformance.ts index 8ca95a2a53..73d860ede2 100644 --- a/packages/spec/src/data/filter-logic-conformance.ts +++ b/packages/spec/src/data/filter-logic-conformance.ts @@ -51,42 +51,21 @@ * the table unpassable rather than more useful. Keep it that way: a case belongs * here only if **every** backend must agree on it. * - * ## Three case families that are RULED but not yet enrolled + * ## Case families that are RULED but not yet enrolled * - * All three were ruled by the maintainer and are implemented in some backends. - * None 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. Family 1 is - * recorded with what was actually measured against `main` at `175d789`; - * families 2 and 3 were re-measured at this PR's 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. + * 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. * - * ### 1. Boolean identities of the empty combinators (#5239) - * - * `{ $and: [] }` = TRUE / all rows, `{ $or: [] }` = FALSE / **zero** rows, - * `{ $or: [{ a: 'x' }, {}] }` = all rows (`{}` is a TRUE disjunct), - * `{ $not: {} }` = FALSE / zero rows. - * - * | backend | `$and:[]` | `$or:[]` | `$or:[{a},{}]` | `$not:{}` | - * |---|---|---|---|---| - * | `formula` | all | zero | all | zero | - * | `driver-memory` | all | zero | all | zero | - * | `driver-sql` (#5134/PR #5243) | all | zero | all | zero | - * | `driver-sqlite-wasm` | all | zero | all | zero | - * | `driver-mongodb` (#5239) | all | zero | all | zero | - * | `read-scope-sql` | **THROWS** | **THROWS** | **rows 1,2** | **whole table** | - * | analytics `filter-normalizer` | **THROWS** | **THROWS** | **rows 1,2** | **whole table** | - * - * The two analytics backends do not merely lag: they hold the OPPOSITE - * position, in writing. `read-scope-sql.ts` and `filter-normalizer.ts` both - * refuse an empty `$and`/`$or` fail-closed ("An empty combinator has no - * defensible reading — dropping it widens the query, and treating it as 'match - * nothing' silently empties a chart"), and `read-scope-sql.test.ts` pins that - * throw. "Reject loudly" is a defensible answer — it is the one #5240 took for - * `{ field: {} }` — but it is not the same answer as "reduce to the identity", - * and one of the two has to give. That is a contract ruling, so it is escalated - * rather than guessed at: **#5322**. + * (Family 1 of this note — the boolean identities of the empty combinators — + * is GONE because it graduated: the #5322 ruling took the identity reduction, + * both analytics compilers aligned, and its four rows now sit in + * {@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) * @@ -248,6 +227,32 @@ export const FILTER_LOGIC_CASES: readonly FilterLogicCase[] = [ note: 'The control: the shape that was always correct must stay correct.', }, + // ── Boolean identities of the empty combinators (#5322 ruling) ──────────── + { + name: 'empty $and is TRUE — the AND identity', + filter: { $and: [] }, + expected: ['1', '2', '3', '4'], + note: '#5322: a conjunction of zero conditions constrains nothing.', + }, + { + name: 'empty $or is FALSE — the OR identity', + filter: { $or: [] }, + expected: [], + note: '#5322/#5134: a disjunction of zero conditions matches nothing. Fail-closed for an RLS scope — a disjunct list that loops to zero items hides every row instead of exposing the table.', + }, + { + name: 'a {} branch is a TRUE disjunct and absorbs its $or', + filter: { $or: [{ a: 'x' }, {}] }, + expected: ['1', '2', '3', '4'], + note: '#5322: collapsing to the surviving branches instead compiles `a = x` — a silently NARROWED scope (#5297).', + }, + { + name: '$not of {} is FALSE — NOT TRUE', + filter: { $not: {} }, + expected: [], + note: '#5322: emitting nothing for it runs the query UNSCOPED — on an RLS lowering that is a permission bypass (#5297).', + }, + // ── Shapes read scopes are actually written in ──────────────────────────── { name: 'read scope: own AND active, OR another owner\'s row', diff --git a/packages/spec/src/data/filter.zod.ts b/packages/spec/src/data/filter.zod.ts index 730d59f3b5..7bd37b802d 100644 --- a/packages/spec/src/data/filter.zod.ts +++ b/packages/spec/src/data/filter.zod.ts @@ -289,22 +289,33 @@ export type FilterCondition = { * Directive #10, and this sentence is kept as the record that the tracking * worked. * + * ## Empty combinators are boolean identities (#5322, maintainer ruling 2026-08-04) + * + * `{ $and: [] }` is TRUE — the AND identity, no constraint. `{ $or: [] }` is + * FALSE — the OR identity, zero rows. A `{}` disjunct is TRUE and ABSORBS its + * `$or`; `{ $not: {} }` is `NOT TRUE` — FALSE. The ruling took the reduction + * over the analytics compilers' fail-closed throw for two reasons: only a + * reduction can evaluate a NESTED tree (a rejection must first reduce to + * judge an empty combinator sitting inside a `$or` branch, which concedes the + * point), and `{ $or: [] }` = zero rows is fail-closed exactly where it + * matters — an RLS scope whose disjunct list loops to zero items hides every + * row instead of exposing the table (#5134). An earlier revision of this + * paragraph kept the identities OUT of the contract because two compilers + * still refused them; that gap closed with PR #5365 (both + * `service-analytics` compilers reduce, and the four cases are enrolled in + * `filter-logic-conformance.ts` against every backend — the five drivers + * already reduced: `driver-sql` #5243, `driver-mongodb` #5323). Loud + * AUTHORING-time rejection of the literal spellings is a separate, optional + * lint concern (#5330), not a runtime semantic. + * * ## Deliberately NOT declared here * - * The boolean identities of the EMPTY combinators (`{ $and: [] }` = TRUE, - * `{ $or: [] }` = FALSE, `{ $not: {} }` = FALSE) are RULED — #5322 - * (maintainer, 2026-08-04) took the identity over the analytics compilers' - * fail-closed throw — but not yet stated here as contract: on main today - * `read-scope-sql` and `filter-normalizer` still refuse an empty `$and`/`$or`, - * and the ruling's implementation PR #5365 (aligns both compilers, enrolls the - * four cases in `FILTER_LOGIC_CASES`) is sequenced to land after this one. The - * declaration flips to stated contract with that PR, not here — declaring it - * first would out-run enforcement. Likewise `{ field: {} }` (a field - * constrained by zero operators): #5240 ruled it REJECTED and #5327 gated - * driver-sql / driver-sqlite-wasm / driver-memory / formula; `driver-mongodb` - * still answers it (tracked by #5376), and the schema-side narrowing stays - * with the spec lane. Declaring either before it is enforced everywhere would - * be exactly the `declared ≠ enforced` shape this file exists to prevent. + * `{ field: {} }` (a field constrained by zero operators): #5240 ruled it + * REJECTED and #5327 gated driver-sql / driver-sqlite-wasm / driver-memory / + * formula; `driver-mongodb` still answers it (tracked by #5376), and the + * schema-side narrowing stays with the spec lane. Declaring it before it is + * enforced everywhere would be exactly the `declared ≠ enforced` shape this + * file exists to prevent. */ export const FilterConditionSchema: z.ZodType = z.lazy(() => z.record(z.string(), z.unknown()).and(