From 6fe9d68e86698bf2b94208f688100f029bd96a03 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:09:05 +0000 Subject: [PATCH] fix(spec): lower equality triples with a $field comparand to {$eq: ref} (#7597) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseFilterAST` lowered one authored intent two ways depending only on the operator spelling: `['amount', '>', { $field: 'budget' }]` kept its operator and worked on both evaluation paths, while `['amount', '=', ref]` — and its `==` / `equals` / `eq` spellings — dropped it and produced `{ amount: { $field: 'budget' } }`, a field spec whose only key is `$field`. No backend reads that as an equality: the in-memory evaluator dispatches `$field` to its operator switch, finds no arm, and returns its fail-closed `false`, so the filter silently matched no record on the very path that produced it. An equality triple whose comparand is a `FieldReferenceSchema` now lowers to the explicit `{ field: { $eq: ref } }` — the spelling both paths already implement (memory resolves the reference; driver-sql compiles it to a column-to-column comparison, #5222). Single-sink change per #5158. Unchanged, deliberately: a LITERAL comparand keeps implicit equality (the branch is on the comparand, not on the operator), a `$field` carrying a non-string is not a field reference on any path and keeps the literal lowering, and the evaluator's unknown-operator posture stays as #6520 left it — a hand-authored bare `{ field: { $field } }` FilterCondition keeps its current fate on every backend. Tests: the cross-field conformance corpus gains an AUTHORING arm entering through the sink instead of at the already-lowered object, run by both SQL drivers; `packages/spec` gains the lowering pins plus a vocabulary sweep that fails if ANY operator spelling lowers a reference comparand to a bare field spec. driver-sql's bare-form refusal pin is re-authored by hand, since the array sugar no longer reaches it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VkfGjiPTZjvhjE2fSuWdBW --- .../equality-field-reference-lowering.md | 47 +++++ .../src/cross-field-conformance-cases.ts | 90 ++++++++++ packages/drivers/driver-sql/src/index.ts | 2 + ...sql-driver-cross-field-conformance.test.ts | 24 ++- .../sql-driver-cross-field-reference.test.ts | 31 +++- packages/drivers/driver-sql/src/sql-driver.ts | 38 ++-- ...qlite-wasm-cross-field-conformance.test.ts | 22 ++- .../filter-field-reference-lowering.test.ts | 163 ++++++++++++++++++ packages/spec/src/data/filter.zod.ts | 45 ++++- 9 files changed, 437 insertions(+), 25 deletions(-) create mode 100644 .changeset/equality-field-reference-lowering.md create mode 100644 packages/spec/src/data/filter-field-reference-lowering.test.ts diff --git a/.changeset/equality-field-reference-lowering.md b/.changeset/equality-field-reference-lowering.md new file mode 100644 index 0000000000..3ca30031c0 --- /dev/null +++ b/.changeset/equality-field-reference-lowering.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": minor +"@objectstack/driver-sql": minor +--- + +fix(spec): lower equality triples with a `$field` comparand to `{ $eq: ref }` (#7597) + +`parseFilterAST` lowered one authored intent two different ways depending only on +how the operator was spelled: + +| authored | lowered to | what it did | +| :--- | :--- | :--- | +| `['amount', '>', { $field: 'budget' }]` | `{ amount: { $gt: { $field: 'budget' } } }` | worked on both evaluation paths | +| `['amount', '=', { $field: 'budget' }]` | `{ amount: { $field: 'budget' } }` | matched **nothing**, silently | + +The four equality spellings (`=`, `==`, `equals`, `eq`) dropped the operator, +because a LITERAL comparand's implicit-equality form is `{ field: value }` — +correct for a literal, and for a field reference it produces a field spec whose +only key is `$field`. Every consumer reads an all-`$` key set as an OPERATOR +SPEC, and nothing implements an operator named `$field`: the in-memory evaluator +(`@objectstack/formula`) dispatches it to its operator switch, finds no arm, and +returns the fail-closed `false` — so the filter matched no record on the very +path that produced it, with no error anywhere. On SQL push-down the same shape +arrived as an unknown operator and was refused. + +An equality triple whose comparand is a `FieldReferenceSchema` now lowers to the +explicit `{ field: { $eq: ref } }` — the spelling both evaluation paths already +implement (the memory evaluator resolves the reference; `driver-sql` compiles it +to a column-to-column comparison, #5222). `['amount', '=', ref]` and +`['amount', '>', ref]` are now the same kind of thing. + +Unchanged, deliberately: + +- **Literal comparands.** `['amount', '=', 5]` still lowers to `{ amount: 5 }`. + The fix branches on the comparand being a field reference, never on the + operator, and a `$field` carrying a non-string is not a field reference on any + path — it keeps the literal lowering too. +- **The in-memory evaluator's unknown-operator posture.** #6520 examined it and + kept it; a hand-authored bare `{ amount: { $field: 'budget' } }` + `FilterCondition` keeps exactly its current fate on every backend — fail-closed + `false` in memory, and `driver-sql`'s actionable refusal naming `$eq` (#5222). + Only what the ARRAY sugar produces has changed. + +`@objectstack/driver-sql` gains `CROSS_FIELD_AUTHORED_CASES` — the conformance +corpus's new AUTHORING arm, entering through the lowering sink instead of at the +already-lowered object, run by both SQL drivers' cross-field suites. Its only +other change is documentation. diff --git a/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts b/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts index 7cda323229..7b293d45fb 100644 --- a/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts +++ b/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts @@ -236,6 +236,96 @@ export const CROSS_FIELD_CASES: readonly CrossFieldCase[] = [ }, ] as const; +/** + * [#7597] The AUTHORING arm: the same conformance obligation, entered through + * the array sugar a caller actually writes rather than through the lowered + * `FilterCondition` object. + * + * ## Why the corpus needed a second entrance + * + * Every case above is a hand-written `FilterCondition`. That is the shape a + * DRIVER sees, and it is not the shape anyone AUTHORS: the ObjectUI client, the + * `FilterBuilder` and every stored view carry the array triple + * `['amount', '=', { $field: 'budget' }]`, which `parseFilterAST` + * (`@objectstack/spec`, the single lowering sink per #5158) turns into one of + * the objects above. A corpus that only enters at the object skips that sink — + * and the sink is exactly where #7597 was: the four EQUALITY spellings dropped + * the operator, because implicit equality (`{ field: comparand }`) is right for + * a literal and produces `{ amount: { $field: 'budget' } }` for a reference — + * a field spec whose only key is `$field`, which no backend reads as an + * equality. `['amount', '>', ref]` kept its operator and worked; `['amount', + * '=', ref]` silently matched nothing. One intent, two spellings, two fates. + * + * So these cases assert TWO things per row, and the pair is the point: + * `loweredTo` pins what the sink produces (a lowering regression fails here, + * in the conformance suite, rather than in a spec unit test nobody reads + * beside the driver), and `expected` holds the lowered filter to the same + * both-paths-same-rows rule as every case above. + * + * The `>` control rides along deliberately: it is the spelling that ALWAYS + * worked, so a run where the equality rows pass and the control fails means + * the harness moved, not the fix. + */ +export interface CrossFieldAuthoredCase { + name: string; + /** The authored filter ARRAY, exactly as a client sends it. */ + authored: unknown; + /** What `parseFilterAST` must lower it to. */ + loweredTo: unknown; + /** Ids of matching rows, ascending — for the LOWERED filter, on both paths. */ + expected: string[]; + note?: string; +} + +/** + * The four `$eq` spellings `AST_OPERATOR_MAP` carries (`=`, `==`, `equals`, + * `eq`), which is the whole set the sink folds into implicit equality — all + * four were bare before #7597, so all four are pinned. + */ +const EQUALITY_SPELLINGS: readonly string[] = ['=', '==', 'equals', 'eq']; + +export const CROSS_FIELD_AUTHORED_CASES: readonly CrossFieldAuthoredCase[] = [ + // ── The equality spellings, on each storage class ──────────────────────── + // + // Replicated across the three class pairs for the same reason the object + // cases are: the lowering is class-blind, so a class-dependent answer here + // would be a driver fact showing up in an authoring test. + ...CLASS_PAIRS.flatMap(({ label, target, ref }) => + EQUALITY_SPELLINGS.map((op) => ({ + name: `['${target}', '${op}', { $field: '${ref}' }] on the ${label} pair`, + authored: [target, op, { $field: ref }], + loweredTo: { [target]: { $eq: { $field: ref } } }, + expected: ['3', '6'], + note: 'The `$eq` row set of the object corpus above — row 3 (equal) and row 6 (both NULL, which the memory evaluator matches and the emitted SQL is written TOTAL to match too).', + })), + ), + + // ── The control: the spelling that never lost its operator ─────────────── + { + name: "['amount', '>', { $field: 'budget' }] still lowers to $gt", + authored: ['amount', '>', { $field: 'budget' }], + loweredTo: { amount: { $gt: { $field: 'budget' } } }, + expected: ['1'], + note: 'Untouched by #7597 and asserted anyway: if this moves, the harness moved rather than the lowering.', + }, + + // ── The sugar's own structures, carrying a reference leaf ──────────────── + { + name: 'a legacy flat array ANDs an equality reference with a literal', + authored: [['amount', '=', { $field: 'budget' }], ['stage', '=', 'mid']], + loweredTo: { $and: [{ amount: { $eq: { $field: 'budget' } } }, { stage: 'mid' }] }, + expected: ['3'], + note: 'Row 6 drops out on the literal conjunct — which also pins that the LITERAL comparand keeps its implicit-equality lowering (`{ stage: "mid" }`, not `{ stage: { $eq: "mid" } }`). The fix branches on the comparand, not on the operator.', + }, + { + name: 'an explicit `or` node carries an equality reference branch', + authored: ['or', ['amount', '=', { $field: 'budget' }], ['stage', '=', 'lost']], + loweredTo: { $or: [{ amount: { $eq: { $field: 'budget' } } }, { stage: 'lost' }] }, + expected: ['2', '3', '6'], + note: 'The lowering is applied at the comparison leaf, so nesting cannot route around it.', + }, +] as const; + /** * The refusal arm — the boundary of v1, and the half of this issue that is a * SECURITY surface rather than a capability one. diff --git a/packages/drivers/driver-sql/src/index.ts b/packages/drivers/driver-sql/src/index.ts index 14114358df..b182cc0027 100644 --- a/packages/drivers/driver-sql/src/index.ts +++ b/packages/drivers/driver-sql/src/index.ts @@ -37,12 +37,14 @@ export type { // which is the argument `@objectstack/spec/data` makes for exporting its own // conformance corpora. Test-only DATA — no runtime path in this package reads it. export { + CROSS_FIELD_AUTHORED_CASES, CROSS_FIELD_CASES, CROSS_FIELD_OBJECT_FIELDS, CROSS_FIELD_REFUSALS, CROSS_FIELD_ROWS, } from './cross-field-conformance-cases.js'; export type { + CrossFieldAuthoredCase, CrossFieldCase, CrossFieldRefusalCase, CrossFieldRow, diff --git a/packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts b/packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts index 8437b7c5be..b6ed4473fb 100644 --- a/packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts @@ -54,10 +54,11 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { matchesFilterCondition } from '@objectstack/formula'; -import type { FilterCondition } from '@objectstack/spec/data'; +import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; import { SqlDriver } from './index.js'; import { DIALECT_CELLS, declareUnprovisionedCell, type DialectCell } from './live-dialect-matrix.testkit.js'; import { + CROSS_FIELD_AUTHORED_CASES, CROSS_FIELD_CASES, CROSS_FIELD_OBJECT_FIELDS, CROSS_FIELD_REFUSALS, @@ -136,6 +137,27 @@ describe(`[#5222] driver-sql — cross-field \`$field\` push-down conformance ($ }); } + describe('[#7597] the AUTHORING arm — the array sugar a client actually sends', () => { + // The sink these cases enter through (`parseFilterAST`) is where #7597 + // was: the four equality spellings dropped the operator on a `{ $field }` + // comparand and produced a field spec no backend reads as an equality, so + // `['amount', '=', ref]` silently matched nothing while `['amount', '>', + // ref]` worked. Lowering and row set are asserted together because either + // one alone can be right while the pair is wrong. + for (const authoredCase of CROSS_FIELD_AUTHORED_CASES) { + it(`${authoredCase.name} — lowers as declared, same rows on both paths`, async () => { + const note = authoredCase.note ? `\n${authoredCase.note}` : ''; + const lowered = parseFilterAST(authoredCase.authored); + expect(lowered, `parseFilterAST lowered the authored array to an unexpected shape${note}`) + .toEqual(authoredCase.loweredTo); + + const expected = [...authoredCase.expected].sort(); + expect(memoryIds(lowered), `in-memory evaluator disagreed${note}`).toEqual(expected); + expect(await sqlIds(lowered), `SQL push-down disagreed${note}`).toEqual(expected); + }); + } + }); + describe('the refusal arm — narrowed, never removed (ADR-0112 envelope)', () => { for (const refusal of CROSS_FIELD_REFUSALS) { it(`${refusal.name} → 400 INVALID_FILTER`, async () => { diff --git a/packages/drivers/driver-sql/src/sql-driver-cross-field-reference.test.ts b/packages/drivers/driver-sql/src/sql-driver-cross-field-reference.test.ts index 36dcd412e6..b2f01eea42 100644 --- a/packages/drivers/driver-sql/src/sql-driver-cross-field-reference.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-cross-field-reference.test.ts @@ -227,17 +227,36 @@ describe('[#5222] SqlDriver `$field` position matrix — compiled vs refused', ( }); it('the bare `{ field: { $field } }` spelling names the operator form to use', async () => { - // What `parseFilterAST(['amount', '=', { $field: 'budget' }])` lowers to. - // Refused because the in-memory evaluator answers `false` for it rather - // than reading it as an equality — compiling it would open a divergence - // in the change that closes one — so the message points at `$eq`. - const lowered = parseFilterAST([['amount', '=', { $field: 'budget' }]] as any); - const err = await refusalOf(() => find(lowered)); + // HAND-AUTHORED, and that spelling matters (#7597). This used to be + // derived from `parseFilterAST(['amount', '=', ref])`, because the sink + // dropped the operator on an equality triple and produced exactly this + // shape — the defect #7597 fixed. The sink now lowers that triple to + // `{ $eq: ref }` (pinned in the conformance suite's authoring arm), so + // the bare form no longer has an authoring route into the driver. + // + // The refusal itself is UNCHANGED and stays pinned here on the shape a + // caller can still write by hand: the in-memory evaluator answers + // `false` for it rather than reading it as an equality (#6520's + // unknown-operator posture, deliberately kept), so compiling it would + // open a divergence — and the message points at the `$eq` spelling that + // does compile. + const err = await refusalOf(() => find({ amount: { $field: 'budget' } })); expect(err.code).toBe('INVALID_FILTER'); expect(err.status).toBe(400); expect(err.message).toContain('$eq'); expect(err.message).toContain('budget'); }); + + it('the equality TRIPLE no longer lowers to that bare spelling (#7597)', async () => { + // The other half of the case above, and the reason it had to change: + // the authoring route that used to reach the bare form now reaches the + // compiled one. Asserted here — beside the refusal it replaced — so a + // regression that restores the bare lowering fails next to the pin + // whose comment explains it, not only in the conformance sweep. + const lowered = parseFilterAST([['amount', '=', { $field: 'budget' }]] as any); + expect(lowered).toEqual({ amount: { $eq: { $field: 'budget' } } }); + await expect(find(lowered)).resolves.toBeDefined(); + }); }); // ── The general arm #5041 installed, untouched by the narrowing ────────── diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index e50844ba9a..7927a5f7fb 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -1026,22 +1026,28 @@ function crossFieldComparisonError(field: string, op: string, ref: string, index * field spec, i.e. in the implicit-equality position where a literal comparand * would mean `field = value`. * - * It is not a hypothetical spelling: `parseFilterAST` lowers the authored - * triple `['amount', '=', { $field: 'budget' }]` (and its `equals` word form) - * to exactly this shape, while `['amount', '>', …]` lowers to `{ $gt: … }`. - * So one authoring dialect produces both a supported and an unsupported - * spelling of the same intent, and the caller cannot see why from the generic - * "unsupported operator" message this used to fall through to. - * - * Refused rather than compiled to `$eq`, deliberately, and the reason is the - * conformance rule the rest of this capability is held to: the in-memory - * evaluator does NOT read this shape as an equality. `matches-filter.ts` - * `evalField` sees an all-`$` key set and dispatches `$field` to `evalOp`, - * which has no arm for it and answers `false` (its fail-closed default). So - * compiling a column-to-column equality here would make SQL answer rows for a - * filter the memory path answers `false` for — a NEW divergence, in the same - * change that closes one. The two paths must move together, which is a spec - * question rather than a driver one; filed separately. + * ## [#7597] It no longer has an AUTHORING route — and is still refused + * + * `parseFilterAST` used to lower the authored triple + * `['amount', '=', { $field: 'budget' }]` (and its `==` / `equals` / `eq` + * spellings) to exactly this shape, while `['amount', '>', …]` kept its + * operator and lowered to `{ $gt: … }` — one authoring dialect producing both + * a supported and an unsupported spelling of the same intent, with the + * unsupported one silent on the path that produced it. #7597 fixed the sink: + * an equality triple whose comparand is a `FieldReferenceSchema` now lowers to + * `{ $eq: ref }`, which this driver compiles. The array sugar therefore cannot + * reach this error any more. + * + * What CAN still reach it is a hand-authored `FilterCondition` carrying the + * bare form, and that keeps being refused rather than compiled to `$eq`, + * deliberately: the in-memory evaluator does NOT read this shape as an + * equality. `matches-filter.ts` `evalField` sees an all-`$` key set and + * dispatches `$field` to `evalOp`, which has no arm for it and answers `false` + * (its fail-closed default). Compiling a column-to-column equality here would + * make SQL answer rows for a filter the memory path answers `false` for — a + * NEW divergence. #7597 ruled that the evaluator's unknown-operator posture + * stays as #6520 left it and moved the LOWERING instead, so this message keeps + * pointing at the `$eq` spelling that does compile. */ function bareFieldReferenceError(field: string, ref: string): Error { return unsupportedFilterError( diff --git a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts index 4ffb537146..de9362b705 100644 --- a/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts @@ -29,8 +29,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { matchesFilterCondition } from '@objectstack/formula'; -import type { FilterCondition } from '@objectstack/spec/data'; +import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; import { + CROSS_FIELD_AUTHORED_CASES, CROSS_FIELD_CASES, CROSS_FIELD_OBJECT_FIELDS, CROSS_FIELD_REFUSALS, @@ -92,6 +93,25 @@ describe('[#5222] driver-sqlite-wasm — cross-field `$field` push-down conforma }); } + describe('[#7597] the AUTHORING arm — the array sugar a client actually sends', () => { + // Run here as well as on `driver-sql` for the reason the whole corpus is + // shared: this driver inherits that compiler but executes through its own + // sql.js dialect, and the lowered `$eq` reference has to mean the same + // rows on both. See the corpus header for the defect it pins. + for (const authoredCase of CROSS_FIELD_AUTHORED_CASES) { + it(`${authoredCase.name} — lowers as declared, same rows on both paths`, async () => { + const note = authoredCase.note ? `\n${authoredCase.note}` : ''; + const lowered = parseFilterAST(authoredCase.authored); + expect(lowered, `parseFilterAST lowered the authored array to an unexpected shape${note}`) + .toEqual(authoredCase.loweredTo); + + const expected = [...authoredCase.expected].sort(); + expect(memoryIds(lowered), `in-memory evaluator disagreed${note}`).toEqual(expected); + expect(await sqlIds(lowered), `wasm push-down disagreed${note}`).toEqual(expected); + }); + } + }); + describe('the refusal arm — narrowed, never removed (ADR-0112 envelope)', () => { for (const refusal of CROSS_FIELD_REFUSALS) { it(`${refusal.name} → 400 INVALID_FILTER`, async () => { diff --git a/packages/spec/src/data/filter-field-reference-lowering.test.ts b/packages/spec/src/data/filter-field-reference-lowering.test.ts new file mode 100644 index 0000000000..c42bd7bb66 --- /dev/null +++ b/packages/spec/src/data/filter-field-reference-lowering.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7597] An equality triple whose comparand is a `{ $field }` reference lowers + * to the EXPLICIT `$eq`, not to the bare implicit-equality form. + * + * ## The defect, as measured on `main` + * + * `parseFilterAST` lowered the same authored intent two different ways + * depending only on how the operator was spelled: + * + * | authored | lowered to | fate | + * |---|---|---| + * | `['amount', '>', { $field: 'budget' }]` | `{ amount: { $gt: { $field: 'budget' } } }` | works on both paths | + * | `['amount', '=', { $field: 'budget' }]` | `{ amount: { $field: 'budget' } }` | matches NOTHING, silently | + * + * The equality spellings drop the operator because a LITERAL comparand's + * implicit-equality form is `{ field: value }` — correct for a literal, and for + * a field reference it produces a field spec whose only key is `$field`. Every + * consumer reads an all-`$` key set as an OPERATOR SPEC, and no backend + * implements an operator called `$field`: the in-memory evaluator + * (`@objectstack/formula` `matches-filter.ts`) dispatches it to `evalOp`, which + * has no arm for it and returns its fail-closed `false`, so the filter matched + * no record on the very path that produced it; `driver-sql` saw an operator + * named `$field` and refused (#5222 gave that refusal wording naming `$eq`). + * + * ## What this file pins, and what it deliberately does not + * + * The fix branches on the COMPARAND, never on the operator: a literal keeps the + * implicit-equality lowering it has always had. And it is a change to the ARRAY + * SUGAR only — a hand-authored `{ amount: { $field: 'budget' } }` + * `FilterCondition` keeps exactly today's fate on every backend, because the + * evaluator's unknown-operator posture is #6520's decision and #7597's ruling + * left it standing. + * + * The execution half — that the lowered filter returns the SAME rows in memory + * and through SQL push-down — lives in the driver suites, where the rows are: + * `cross-field-conformance-cases.ts`'s authoring arm, run by + * `sql-driver-cross-field-conformance.test.ts` and its `driver-sqlite-wasm` + * twin. + */ + +import { describe, it, expect } from 'vitest'; +import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS } from './filter.zod'; + +/** + * The spellings `AST_OPERATOR_MAP` folds onto `$eq` — the whole set the sink + * reads as implicit equality, and therefore the whole set the defect covered. + * The vocabulary sweep at the bottom of this file is what keeps this list from + * silently going short. + */ +const EQUALITY_SPELLINGS = ['=', '==', 'equals', 'eq'] as const; + +const REF = { $field: 'budget' } as const; + +describe('[#7597] equality triples with a `{ $field }` comparand', () => { + // ── The fix ─────────────────────────────────────────────────────────────── + + for (const op of EQUALITY_SPELLINGS) { + it(`['amount', '${op}', ref] lowers to the explicit { $eq: ref }`, () => { + expect(parseFilterAST(['amount', op, REF])).toEqual({ + amount: { $eq: { $field: 'budget' } }, + }); + }); + } + + it('uppercase and mixed-case spellings lower the same way', () => { + // The sink folds case before it looks the operator up, and a stored view + // legitimately carries `Equals`. A fix applied to the lowercase branch + // only would leave the defect alive at a spelling the same authoring tool + // produces. + expect(parseFilterAST(['amount', 'EQUALS', REF])).toEqual({ amount: { $eq: REF } }); + expect(parseFilterAST(['amount', 'Eq', REF])).toEqual({ amount: { $eq: REF } }); + }); + + it('the reference survives the lowering unchanged, dotted paths included', () => { + // The sink does not judge the reference — whether a dotted path is + // COMPILABLE is the driver's call (`driver-sql` refuses one, the memory + // evaluator walks it), and rewriting or rejecting it here would move that + // decision to a layer that cannot see the object's fields. + expect(parseFilterAST(['amount', '=', { $field: 'account.budget' }])).toEqual({ + amount: { $eq: { $field: 'account.budget' } }, + }); + }); + + // ── The other spellings are untouched ──────────────────────────────────── + + it('an ordering triple still lowers to its own operator', () => { + expect(parseFilterAST(['amount', '>', REF])).toEqual({ amount: { $gt: REF } }); + expect(parseFilterAST(['amount', 'gt', REF])).toEqual({ amount: { $gt: REF } }); + }); + + it('a LITERAL comparand keeps the implicit-equality lowering', () => { + // The half a comparand-blind fix would have broken: `{ a: 5 }` is the + // established output for every literal, it is what the drivers' + // implicit-equality arms read, and #7597 changes none of it. The branch is + // on the comparand being a field reference, not on the operator. + expect(parseFilterAST(['amount', '=', 5])).toEqual({ amount: 5 }); + expect(parseFilterAST(['stage', 'equals', 'won'])).toEqual({ stage: 'won' }); + expect(parseFilterAST(['stage', '=', null])).toEqual({ stage: null }); + expect(parseFilterAST(['stage', '=', ['a', 'b']])).toEqual({ stage: ['a', 'b'] }); + }); + + it('an object comparand that is NOT a field reference keeps implicit equality', () => { + // `FieldReferenceSchema` is `{ $field: string }`. An object without that + // key is an ordinary (deep-equality) comparand and must not be promoted + // into a `$eq` the drivers would then have to un-recognise. + expect(parseFilterAST(['author', '=', { name: 'ada' }])).toEqual({ author: { name: 'ada' } }); + }); + + it('a `$field` whose value is not a STRING is not a field reference', () => { + // The predicate matches `driver-sql`'s `fieldReferenceOf`, which requires a + // string. A non-string carrier is not a reference on ANY path, so it keeps + // the literal lowering and is refused downstream as the uncompilable + // object it is — rather than being handed to the drivers as a `$eq` + // reference they do not recognise. + expect(parseFilterAST(['amount', '=', { $field: 42 }])).toEqual({ amount: { $field: 42 } }); + }); + + // ── The sugar's own structures ─────────────────────────────────────────── + + it('the lowering applies at the leaf, so nesting cannot route around it', () => { + expect(parseFilterAST(['and', ['amount', '=', REF], ['stage', '=', 'won']])).toEqual({ + $and: [{ amount: { $eq: REF } }, { stage: 'won' }], + }); + expect(parseFilterAST(['or', ['amount', 'eq', REF], ['stage', '=', 'lost']])).toEqual({ + $or: [{ amount: { $eq: REF } }, { stage: 'lost' }], + }); + expect(parseFilterAST([['amount', '=', REF], ['stage', '=', 'won']])).toEqual({ + $and: [{ amount: { $eq: REF } }, { stage: 'won' }], + }); + }); + + it('`isFilterAST` still accepts the triple — the fix is downstream of the gate', () => { + // The two functions are a pair: a shape accepted by the gate and lowered + // to nothing (or to the wrong thing) is how a filter gets DROPPED, which + // widens the query instead of narrowing it (#3948). The gate reads the + // OPERATOR, so a comparand change must not move it. + for (const op of EQUALITY_SPELLINGS) { + expect(isFilterAST(['amount', op, REF])).toBe(true); + } + }); + + // ── The vocabulary sweep: no spelling may drop the operator ────────────── + + it('NO operator in the vocabulary lowers a reference comparand to a bare field spec', () => { + // The drift pin, and the reason this file does not merely list four + // spellings. The bare form is precisely "the lowered field spec's ONLY key + // is `$field`" — the shape nothing implements. Sweeping the exported + // vocabulary means a fifth `$eq` spelling entering `AST_OPERATOR_MAP` + // cannot re-open the defect at a name this file never heard of. + const offenders: string[] = []; + for (const op of VALID_AST_OPERATORS) { + const lowered = parseFilterAST(['amount', op, REF]) as Record; + const spec = lowered?.amount; + if (spec && typeof spec === 'object' && !Array.isArray(spec)) { + const keys = Object.keys(spec as Record); + if (keys.length === 1 && keys[0] === '$field') offenders.push(op); + } + } + expect(offenders, 'these spellings lower a `{ $field }` comparand to a bare, unimplemented field spec').toEqual([]); + }); +}); diff --git a/packages/spec/src/data/filter.zod.ts b/packages/spec/src/data/filter.zod.ts index 1be2baa6c2..d93d73b777 100644 --- a/packages/spec/src/data/filter.zod.ts +++ b/packages/spec/src/data/filter.zod.ts @@ -1350,6 +1350,22 @@ export function isFilterAST(filter: unknown): boolean { // AST Array → FilterCondition Conversion // ============================================================================ +/** + * [#7597] Is this comparand a Filter Protocol FIELD REFERENCE — the + * {@link FieldReferenceSchema} shape `{ $field: 'other_column' }`? + * + * The `$field` value must be a STRING, which is what the schema declares. The + * predicate deliberately matches `driver-sql`'s `fieldReferenceOf`: a comparand + * whose `$field` is not a string is not a field reference on any path, so it + * keeps the literal lowering below and is refused downstream as the + * uncompilable object it is — rather than being promoted here into a `$eq` the + * drivers would then have to un-recognise. + */ +function isFieldReferenceComparand(value: unknown): boolean { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + return typeof (value as Record).$field === 'string'; +} + /** * Convert a single AST comparison node `[field, operator, value]` to a FilterCondition object. */ @@ -1360,8 +1376,35 @@ function convertComparison(node: [string, string, unknown]): FilterCondition { // Special case: equality shorthand. `equals`/`eq` are the view vocabulary's // spellings of the same thing and must produce the same output, or one filter // would compile two different ways depending on how the author spelled it. + // + // [#7597] …with ONE comparand excepted: a `{ $field }` reference. The + // implicit-equality form `{ field: comparand }` says "equals" only because a + // LITERAL sitting in a field's value position means equality. A field + // reference sitting there produces `{ amount: { $field: 'budget' } }` — an + // object whose only key starts with `$`, which every consumer reads as an + // OPERATOR SPEC named `$field`, not as a comparand. Nothing implements that + // operator: the in-memory evaluator (`matches-filter.ts` `evalField` → + // `evalOp`) has no arm for it and answers its fail-closed `false`, so the + // filter silently matched NO record, while `['amount', '>', ref]` — which + // keeps its operator — worked on both paths. Two spellings of one intent, + // two fates, and the losing one is silent. + // + // So the reference comparand is lowered to the EXPLICIT `$eq` spelling, which + // is the spelling both evaluation paths already implement (the memory + // evaluator resolves the reference in `resolveValue`; `driver-sql` compiles it + // to a column-to-column comparison, #5222). This is a change to what the ARRAY + // SUGAR produces and nothing else — a hand-authored bare + // `{ field: { $field: … } }` FilterCondition keeps exactly today's fate + // (memory fail-closed `false`; SQL's refusal naming `$eq`), because the + // evaluator's unknown-operator posture is #6520's decision and stands. + // + // All four `$eq` spellings of {@link AST_OPERATOR_MAP} are covered — the + // condition below is that key set, and `filter.test.ts` pins the two lists + // together so a fifth spelling cannot enter the map and miss this branch. if (op === '=' || op === '==' || op === 'equals' || op === 'eq') { - return { [field]: value } as FilterCondition; + return isFieldReferenceComparand(value) + ? ({ [field]: { $eq: value } } as FilterCondition) + : ({ [field]: value } as FilterCondition); } // Null / empty predicates — direction comes from the operator NAME, not the