Skip to content

Commit 6a9dec6

Browse files
os-zhuangclaude
andauthored
fix(spec): lower equality triples with a $field comparand to {$eq: ref} (#7597) (#7672)
`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. Claude-Session: https://claude.ai/code/session_01VkfGjiPTZjvhjE2fSuWdBW Co-authored-by: Claude <noreply@anthropic.com>
1 parent 779ace4 commit 6a9dec6

9 files changed

Lines changed: 437 additions & 25 deletions
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/driver-sql": minor
4+
---
5+
6+
fix(spec): lower equality triples with a `$field` comparand to `{ $eq: ref }` (#7597)
7+
8+
`parseFilterAST` lowered one authored intent two different ways depending only on
9+
how the operator was spelled:
10+
11+
| authored | lowered to | what it did |
12+
| :--- | :--- | :--- |
13+
| `['amount', '>', { $field: 'budget' }]` | `{ amount: { $gt: { $field: 'budget' } } }` | worked on both evaluation paths |
14+
| `['amount', '=', { $field: 'budget' }]` | `{ amount: { $field: 'budget' } }` | matched **nothing**, silently |
15+
16+
The four equality spellings (`=`, `==`, `equals`, `eq`) dropped the operator,
17+
because a LITERAL comparand's implicit-equality form is `{ field: value }`
18+
correct for a literal, and for a field reference it produces a field spec whose
19+
only key is `$field`. Every consumer reads an all-`$` key set as an OPERATOR
20+
SPEC, and nothing implements an operator named `$field`: the in-memory evaluator
21+
(`@objectstack/formula`) dispatches it to its operator switch, finds no arm, and
22+
returns the fail-closed `false` — so the filter matched no record on the very
23+
path that produced it, with no error anywhere. On SQL push-down the same shape
24+
arrived as an unknown operator and was refused.
25+
26+
An equality triple whose comparand is a `FieldReferenceSchema` now lowers to the
27+
explicit `{ field: { $eq: ref } }` — the spelling both evaluation paths already
28+
implement (the memory evaluator resolves the reference; `driver-sql` compiles it
29+
to a column-to-column comparison, #5222). `['amount', '=', ref]` and
30+
`['amount', '>', ref]` are now the same kind of thing.
31+
32+
Unchanged, deliberately:
33+
34+
- **Literal comparands.** `['amount', '=', 5]` still lowers to `{ amount: 5 }`.
35+
The fix branches on the comparand being a field reference, never on the
36+
operator, and a `$field` carrying a non-string is not a field reference on any
37+
path — it keeps the literal lowering too.
38+
- **The in-memory evaluator's unknown-operator posture.** #6520 examined it and
39+
kept it; a hand-authored bare `{ amount: { $field: 'budget' } }`
40+
`FilterCondition` keeps exactly its current fate on every backend — fail-closed
41+
`false` in memory, and `driver-sql`'s actionable refusal naming `$eq` (#5222).
42+
Only what the ARRAY sugar produces has changed.
43+
44+
`@objectstack/driver-sql` gains `CROSS_FIELD_AUTHORED_CASES` — the conformance
45+
corpus's new AUTHORING arm, entering through the lowering sink instead of at the
46+
already-lowered object, run by both SQL drivers' cross-field suites. Its only
47+
other change is documentation.

packages/drivers/driver-sql/src/cross-field-conformance-cases.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,96 @@ export const CROSS_FIELD_CASES: readonly CrossFieldCase[] = [
236236
},
237237
] as const;
238238

239+
/**
240+
* [#7597] The AUTHORING arm: the same conformance obligation, entered through
241+
* the array sugar a caller actually writes rather than through the lowered
242+
* `FilterCondition` object.
243+
*
244+
* ## Why the corpus needed a second entrance
245+
*
246+
* Every case above is a hand-written `FilterCondition`. That is the shape a
247+
* DRIVER sees, and it is not the shape anyone AUTHORS: the ObjectUI client, the
248+
* `FilterBuilder` and every stored view carry the array triple
249+
* `['amount', '=', { $field: 'budget' }]`, which `parseFilterAST`
250+
* (`@objectstack/spec`, the single lowering sink per #5158) turns into one of
251+
* the objects above. A corpus that only enters at the object skips that sink —
252+
* and the sink is exactly where #7597 was: the four EQUALITY spellings dropped
253+
* the operator, because implicit equality (`{ field: comparand }`) is right for
254+
* a literal and produces `{ amount: { $field: 'budget' } }` for a reference —
255+
* a field spec whose only key is `$field`, which no backend reads as an
256+
* equality. `['amount', '>', ref]` kept its operator and worked; `['amount',
257+
* '=', ref]` silently matched nothing. One intent, two spellings, two fates.
258+
*
259+
* So these cases assert TWO things per row, and the pair is the point:
260+
* `loweredTo` pins what the sink produces (a lowering regression fails here,
261+
* in the conformance suite, rather than in a spec unit test nobody reads
262+
* beside the driver), and `expected` holds the lowered filter to the same
263+
* both-paths-same-rows rule as every case above.
264+
*
265+
* The `>` control rides along deliberately: it is the spelling that ALWAYS
266+
* worked, so a run where the equality rows pass and the control fails means
267+
* the harness moved, not the fix.
268+
*/
269+
export interface CrossFieldAuthoredCase {
270+
name: string;
271+
/** The authored filter ARRAY, exactly as a client sends it. */
272+
authored: unknown;
273+
/** What `parseFilterAST` must lower it to. */
274+
loweredTo: unknown;
275+
/** Ids of matching rows, ascending — for the LOWERED filter, on both paths. */
276+
expected: string[];
277+
note?: string;
278+
}
279+
280+
/**
281+
* The four `$eq` spellings `AST_OPERATOR_MAP` carries (`=`, `==`, `equals`,
282+
* `eq`), which is the whole set the sink folds into implicit equality — all
283+
* four were bare before #7597, so all four are pinned.
284+
*/
285+
const EQUALITY_SPELLINGS: readonly string[] = ['=', '==', 'equals', 'eq'];
286+
287+
export const CROSS_FIELD_AUTHORED_CASES: readonly CrossFieldAuthoredCase[] = [
288+
// ── The equality spellings, on each storage class ────────────────────────
289+
//
290+
// Replicated across the three class pairs for the same reason the object
291+
// cases are: the lowering is class-blind, so a class-dependent answer here
292+
// would be a driver fact showing up in an authoring test.
293+
...CLASS_PAIRS.flatMap(({ label, target, ref }) =>
294+
EQUALITY_SPELLINGS.map((op) => ({
295+
name: `['${target}', '${op}', { $field: '${ref}' }] on the ${label} pair`,
296+
authored: [target, op, { $field: ref }],
297+
loweredTo: { [target]: { $eq: { $field: ref } } },
298+
expected: ['3', '6'],
299+
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).',
300+
})),
301+
),
302+
303+
// ── The control: the spelling that never lost its operator ───────────────
304+
{
305+
name: "['amount', '>', { $field: 'budget' }] still lowers to $gt",
306+
authored: ['amount', '>', { $field: 'budget' }],
307+
loweredTo: { amount: { $gt: { $field: 'budget' } } },
308+
expected: ['1'],
309+
note: 'Untouched by #7597 and asserted anyway: if this moves, the harness moved rather than the lowering.',
310+
},
311+
312+
// ── The sugar's own structures, carrying a reference leaf ────────────────
313+
{
314+
name: 'a legacy flat array ANDs an equality reference with a literal',
315+
authored: [['amount', '=', { $field: 'budget' }], ['stage', '=', 'mid']],
316+
loweredTo: { $and: [{ amount: { $eq: { $field: 'budget' } } }, { stage: 'mid' }] },
317+
expected: ['3'],
318+
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.',
319+
},
320+
{
321+
name: 'an explicit `or` node carries an equality reference branch',
322+
authored: ['or', ['amount', '=', { $field: 'budget' }], ['stage', '=', 'lost']],
323+
loweredTo: { $or: [{ amount: { $eq: { $field: 'budget' } } }, { stage: 'lost' }] },
324+
expected: ['2', '3', '6'],
325+
note: 'The lowering is applied at the comparison leaf, so nesting cannot route around it.',
326+
},
327+
] as const;
328+
239329
/**
240330
* The refusal arm — the boundary of v1, and the half of this issue that is a
241331
* SECURITY surface rather than a capability one.

packages/drivers/driver-sql/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,14 @@ export type {
3737
// which is the argument `@objectstack/spec/data` makes for exporting its own
3838
// conformance corpora. Test-only DATA — no runtime path in this package reads it.
3939
export {
40+
CROSS_FIELD_AUTHORED_CASES,
4041
CROSS_FIELD_CASES,
4142
CROSS_FIELD_OBJECT_FIELDS,
4243
CROSS_FIELD_REFUSALS,
4344
CROSS_FIELD_ROWS,
4445
} from './cross-field-conformance-cases.js';
4546
export type {
47+
CrossFieldAuthoredCase,
4648
CrossFieldCase,
4749
CrossFieldRefusalCase,
4850
CrossFieldRow,

packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,11 @@
5454

5555
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
5656
import { matchesFilterCondition } from '@objectstack/formula';
57-
import type { FilterCondition } from '@objectstack/spec/data';
57+
import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data';
5858
import { SqlDriver } from './index.js';
5959
import { DIALECT_CELLS, declareUnprovisionedCell, type DialectCell } from './live-dialect-matrix.testkit.js';
6060
import {
61+
CROSS_FIELD_AUTHORED_CASES,
6162
CROSS_FIELD_CASES,
6263
CROSS_FIELD_OBJECT_FIELDS,
6364
CROSS_FIELD_REFUSALS,
@@ -136,6 +137,27 @@ describe(`[#5222] driver-sql — cross-field \`$field\` push-down conformance ($
136137
});
137138
}
138139

140+
describe('[#7597] the AUTHORING arm — the array sugar a client actually sends', () => {
141+
// The sink these cases enter through (`parseFilterAST`) is where #7597
142+
// was: the four equality spellings dropped the operator on a `{ $field }`
143+
// comparand and produced a field spec no backend reads as an equality, so
144+
// `['amount', '=', ref]` silently matched nothing while `['amount', '>',
145+
// ref]` worked. Lowering and row set are asserted together because either
146+
// one alone can be right while the pair is wrong.
147+
for (const authoredCase of CROSS_FIELD_AUTHORED_CASES) {
148+
it(`${authoredCase.name} — lowers as declared, same rows on both paths`, async () => {
149+
const note = authoredCase.note ? `\n${authoredCase.note}` : '';
150+
const lowered = parseFilterAST(authoredCase.authored);
151+
expect(lowered, `parseFilterAST lowered the authored array to an unexpected shape${note}`)
152+
.toEqual(authoredCase.loweredTo);
153+
154+
const expected = [...authoredCase.expected].sort();
155+
expect(memoryIds(lowered), `in-memory evaluator disagreed${note}`).toEqual(expected);
156+
expect(await sqlIds(lowered), `SQL push-down disagreed${note}`).toEqual(expected);
157+
});
158+
}
159+
});
160+
139161
describe('the refusal arm — narrowed, never removed (ADR-0112 envelope)', () => {
140162
for (const refusal of CROSS_FIELD_REFUSALS) {
141163
it(`${refusal.name} → 400 INVALID_FILTER`, async () => {

packages/drivers/driver-sql/src/sql-driver-cross-field-reference.test.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,17 +227,36 @@ describe('[#5222] SqlDriver `$field` position matrix — compiled vs refused', (
227227
});
228228

229229
it('the bare `{ field: { $field } }` spelling names the operator form to use', async () => {
230-
// What `parseFilterAST(['amount', '=', { $field: 'budget' }])` lowers to.
231-
// Refused because the in-memory evaluator answers `false` for it rather
232-
// than reading it as an equality — compiling it would open a divergence
233-
// in the change that closes one — so the message points at `$eq`.
234-
const lowered = parseFilterAST([['amount', '=', { $field: 'budget' }]] as any);
235-
const err = await refusalOf(() => find(lowered));
230+
// HAND-AUTHORED, and that spelling matters (#7597). This used to be
231+
// derived from `parseFilterAST(['amount', '=', ref])`, because the sink
232+
// dropped the operator on an equality triple and produced exactly this
233+
// shape — the defect #7597 fixed. The sink now lowers that triple to
234+
// `{ $eq: ref }` (pinned in the conformance suite's authoring arm), so
235+
// the bare form no longer has an authoring route into the driver.
236+
//
237+
// The refusal itself is UNCHANGED and stays pinned here on the shape a
238+
// caller can still write by hand: the in-memory evaluator answers
239+
// `false` for it rather than reading it as an equality (#6520's
240+
// unknown-operator posture, deliberately kept), so compiling it would
241+
// open a divergence — and the message points at the `$eq` spelling that
242+
// does compile.
243+
const err = await refusalOf(() => find({ amount: { $field: 'budget' } }));
236244
expect(err.code).toBe('INVALID_FILTER');
237245
expect(err.status).toBe(400);
238246
expect(err.message).toContain('$eq');
239247
expect(err.message).toContain('budget');
240248
});
249+
250+
it('the equality TRIPLE no longer lowers to that bare spelling (#7597)', async () => {
251+
// The other half of the case above, and the reason it had to change:
252+
// the authoring route that used to reach the bare form now reaches the
253+
// compiled one. Asserted here — beside the refusal it replaced — so a
254+
// regression that restores the bare lowering fails next to the pin
255+
// whose comment explains it, not only in the conformance sweep.
256+
const lowered = parseFilterAST([['amount', '=', { $field: 'budget' }]] as any);
257+
expect(lowered).toEqual({ amount: { $eq: { $field: 'budget' } } });
258+
await expect(find(lowered)).resolves.toBeDefined();
259+
});
241260
});
242261

243262
// ── The general arm #5041 installed, untouched by the narrowing ──────────

packages/drivers/driver-sql/src/sql-driver.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,22 +1026,28 @@ function crossFieldComparisonError(field: string, op: string, ref: string, index
10261026
* field spec, i.e. in the implicit-equality position where a literal comparand
10271027
* would mean `field = value`.
10281028
*
1029-
* It is not a hypothetical spelling: `parseFilterAST` lowers the authored
1030-
* triple `['amount', '=', { $field: 'budget' }]` (and its `equals` word form)
1031-
* to exactly this shape, while `['amount', '>', …]` lowers to `{ $gt: … }`.
1032-
* So one authoring dialect produces both a supported and an unsupported
1033-
* spelling of the same intent, and the caller cannot see why from the generic
1034-
* "unsupported operator" message this used to fall through to.
1035-
*
1036-
* Refused rather than compiled to `$eq`, deliberately, and the reason is the
1037-
* conformance rule the rest of this capability is held to: the in-memory
1038-
* evaluator does NOT read this shape as an equality. `matches-filter.ts`
1039-
* `evalField` sees an all-`$` key set and dispatches `$field` to `evalOp`,
1040-
* which has no arm for it and answers `false` (its fail-closed default). So
1041-
* compiling a column-to-column equality here would make SQL answer rows for a
1042-
* filter the memory path answers `false` for — a NEW divergence, in the same
1043-
* change that closes one. The two paths must move together, which is a spec
1044-
* question rather than a driver one; filed separately.
1029+
* ## [#7597] It no longer has an AUTHORING route — and is still refused
1030+
*
1031+
* `parseFilterAST` used to lower the authored triple
1032+
* `['amount', '=', { $field: 'budget' }]` (and its `==` / `equals` / `eq`
1033+
* spellings) to exactly this shape, while `['amount', '>', …]` kept its
1034+
* operator and lowered to `{ $gt: … }` — one authoring dialect producing both
1035+
* a supported and an unsupported spelling of the same intent, with the
1036+
* unsupported one silent on the path that produced it. #7597 fixed the sink:
1037+
* an equality triple whose comparand is a `FieldReferenceSchema` now lowers to
1038+
* `{ $eq: ref }`, which this driver compiles. The array sugar therefore cannot
1039+
* reach this error any more.
1040+
*
1041+
* What CAN still reach it is a hand-authored `FilterCondition` carrying the
1042+
* bare form, and that keeps being refused rather than compiled to `$eq`,
1043+
* deliberately: the in-memory evaluator does NOT read this shape as an
1044+
* equality. `matches-filter.ts` `evalField` sees an all-`$` key set and
1045+
* dispatches `$field` to `evalOp`, which has no arm for it and answers `false`
1046+
* (its fail-closed default). Compiling a column-to-column equality here would
1047+
* make SQL answer rows for a filter the memory path answers `false` for — a
1048+
* NEW divergence. #7597 ruled that the evaluator's unknown-operator posture
1049+
* stays as #6520 left it and moved the LOWERING instead, so this message keeps
1050+
* pointing at the `$eq` spelling that does compile.
10451051
*/
10461052
function bareFieldReferenceError(field: string, ref: string): Error {
10471053
return unsupportedFilterError(

packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@
2929

3030
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
3131
import { matchesFilterCondition } from '@objectstack/formula';
32-
import type { FilterCondition } from '@objectstack/spec/data';
32+
import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data';
3333
import {
34+
CROSS_FIELD_AUTHORED_CASES,
3435
CROSS_FIELD_CASES,
3536
CROSS_FIELD_OBJECT_FIELDS,
3637
CROSS_FIELD_REFUSALS,
@@ -92,6 +93,25 @@ describe('[#5222] driver-sqlite-wasm — cross-field `$field` push-down conforma
9293
});
9394
}
9495

96+
describe('[#7597] the AUTHORING arm — the array sugar a client actually sends', () => {
97+
// Run here as well as on `driver-sql` for the reason the whole corpus is
98+
// shared: this driver inherits that compiler but executes through its own
99+
// sql.js dialect, and the lowered `$eq` reference has to mean the same
100+
// rows on both. See the corpus header for the defect it pins.
101+
for (const authoredCase of CROSS_FIELD_AUTHORED_CASES) {
102+
it(`${authoredCase.name} — lowers as declared, same rows on both paths`, async () => {
103+
const note = authoredCase.note ? `\n${authoredCase.note}` : '';
104+
const lowered = parseFilterAST(authoredCase.authored);
105+
expect(lowered, `parseFilterAST lowered the authored array to an unexpected shape${note}`)
106+
.toEqual(authoredCase.loweredTo);
107+
108+
const expected = [...authoredCase.expected].sort();
109+
expect(memoryIds(lowered), `in-memory evaluator disagreed${note}`).toEqual(expected);
110+
expect(await sqlIds(lowered), `wasm push-down disagreed${note}`).toEqual(expected);
111+
});
112+
}
113+
});
114+
95115
describe('the refusal arm — narrowed, never removed (ADR-0112 envelope)', () => {
96116
for (const refusal of CROSS_FIELD_REFUSALS) {
97117
it(`${refusal.name} → 400 INVALID_FILTER`, async () => {

0 commit comments

Comments
 (0)