From 152c6906c1a92412493934c41dbacd762ab09334 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 06:07:50 +0000 Subject: [PATCH] feat(driver-sql): compile $field to column-to-column comparison (#5222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FieldReferenceSchema` (`{ $field: 'col' }`) is declared in the spec and really is produced — `compileCelToFilter` emits it for a field-to-field comparison in a CEL permission/RLS rule — but its only implementation was the in-memory evaluator. #5041 measured that and installed a loud refusal (INVALID_FILTER/400, replacing a bare TypeError and a silent zero-row answer inside $in lists), deliberately leaving the capability to this issue. Until now one permission rule had two behaviours, chosen by whether the query reached a database. The six scalar comparison operators now compile the reference to a real column reference. The refusal gate is NARROWED, never removed — dot paths, undeclared columns, the tenant-isolation column (both sides, because = commutes), cross-class comparisons, list members and the string family all keep the ADR-0112 envelope. Every emitted predicate is written TOTAL across NULLs, so it agrees with the two-valued in-memory evaluator rather than with three-valued SQL: both columns NULL satisfies $eq, and $not over a cross-field leaf is its exact complement. A cross-path conformance suite proves that row for row on both SQL drivers. Closes #5222 --- .../sql-driver-cross-field-comparison.md | 74 ++++ packages/drivers/driver-sql/package.json | 1 + .../src/cross-field-conformance-cases.ts | 414 ++++++++++++++++++ packages/drivers/driver-sql/src/index.ts | 24 + ...sql-driver-cross-field-conformance.test.ts | 177 ++++++++ .../sql-driver-cross-field-reference.test.ts | 343 +++++++++------ packages/drivers/driver-sql/src/sql-driver.ts | 367 +++++++++++++++- .../drivers/driver-sqlite-wasm/package.json | 1 + ...qlite-wasm-cross-field-conformance.test.ts | 116 +++++ pnpm-lock.yaml | 6 + 10 files changed, 1382 insertions(+), 141 deletions(-) create mode 100644 .changeset/sql-driver-cross-field-comparison.md create mode 100644 packages/drivers/driver-sql/src/cross-field-conformance-cases.ts create mode 100644 packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts create mode 100644 packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts diff --git a/.changeset/sql-driver-cross-field-comparison.md b/.changeset/sql-driver-cross-field-comparison.md new file mode 100644 index 0000000000..775a97de18 --- /dev/null +++ b/.changeset/sql-driver-cross-field-comparison.md @@ -0,0 +1,74 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/driver-sqlite-wasm": minor +--- + +feat(driver-sql): compile `$field` to a column-to-column comparison on SQL push-down (#5222) + +`FieldReferenceSchema` (`{ $field: 'other_column' }`) is declared in the spec and +genuinely PRODUCED — `compileCelToFilter` emits it whenever a CEL permission/RLS +rule compares one field to another — but its only implementation was the +in-memory evaluator. #5041 measured the consequence and installed a loud refusal +(`INVALID_FILTER` / 400, replacing a bare `TypeError` and, inside an `$in` list, +a silent zero-row answer), deliberately leaving the capability itself to this +change. Until now, therefore, one permission rule had two behaviours chosen by +whether the query reached a database. + +The six scalar comparison operators — `$eq` / `$ne` / `$gt` / `$gte` / `$lt` / +`$lte`, including the array-triple authorings that lower to them — now compile +`{ $field: 'col' }` into a real column reference: + +```js +{ amount: { $gt: { $field: 'budget' } } } // → where "amount" > "budget" +``` + +**Nothing that worked before changes.** This is additive: every shape that +compiled still compiles identically, and the refusal gate was NARROWED, never +removed. A minor bump because a previously-400 filter now returns rows. + +**The refused arm, and why each entry is there** (all keep `INVALID_FILTER` / +400): + +- **Dotted paths** (`{ $field: 'account.owner_id' }`) — maintainer ruling: v1 is + same-table columns only. No JOIN planning, no alias-qualified columns. +- **Undeclared columns**, on either side — the `$field` value lands in a SQL + identifier position, so only fields the object declares are accepted, refused + at COMPILE time rather than by the database. Federated/external tables + (ADR-0015), whose column set this driver does not own, are refused wholesale. +- **The tenant-isolation column**, on either side — a privilege-escalation + comparison surface. Closed on both sides because the operands of `=` commute. +- **Cross-class comparisons** (a number against text, a date against text) — + SQLite orders by storage class first while the in-memory evaluator applies JS + coercion, so the two paths genuinely disagree and neither answer can be made + the other. Refused rather than shipped as a silent divergence. +- **`$in` / `$nin` / `$between` list members** — the in-memory evaluator does not + resolve a reference inside a list either (`resolveValue` returns an array + unchanged), so there is no correct semantics for SQL to be equivalent to. +- **The string operators** (`$contains`, `$startsWith`, …) — a column-side LIKE + pattern cannot be metacharacter-escaped portably, and an unescaped one is the + `%`-matches-every-row filter bypass. +- **The bare `{ field: { $field: 'other' } }` spelling** — what + `parseFilterAST(['a', '=', { $field: 'b' }])` lowers to. Still refused, because + the in-memory evaluator answers `false` for it rather than reading it as an + equality; the refusal now names `$eq` as the spelling that compiles instead of + falling through to a generic operator list. + +**Equivalence is proven, not asserted.** A cross-path conformance suite runs each +supported shape through the in-memory evaluator AND through SQL push-down against +the same seeded rows, holding both to the same declared id list. Its fixture +carries every NULL arrangement two columns can be in — target NULL, referent +NULL, and BOTH NULL — because three-valued SQL against a two-valued JS matcher is +the one place these paths can genuinely diverge. Every emitted predicate is +therefore written TOTAL: `{ a: { $eq: { $field: 'b' } } }` matches a row where +both columns are NULL, which a plain `a = b` would drop, and `$not` over any +cross-field leaf is its exact complement. + +The suite runs the full driver axis — SQLite always, live Postgres and MySQL +when the runner provisions them — and on both SQL drivers: `driver-sqlite-wasm` +inherits the compiler but executes through its own sql.js dialect, which binds +the identifier list itself. The dialect axis is not ceremony here: a cross-field +predicate is the one filter shape whose SQL carries two identifiers and no bound +value, and the class rule has a different failure per backend — comparing text to +a number is a silent wrong answer on SQLite (storage classes order before values) +but `operator does not exist: text > integer` on Postgres. The guard is what +keeps either from being reached. diff --git a/packages/drivers/driver-sql/package.json b/packages/drivers/driver-sql/package.json index d35d0e5560..6a60577785 100644 --- a/packages/drivers/driver-sql/package.json +++ b/packages/drivers/driver-sql/package.json @@ -46,6 +46,7 @@ } }, "devDependencies": { + "@objectstack/formula": "workspace:*", "@types/node": "^26.1.2", "better-sqlite3": "^13.0.2", "typescript": "^6.0.3", diff --git a/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts b/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts new file mode 100644 index 0000000000..7cda323229 --- /dev/null +++ b/packages/drivers/driver-sql/src/cross-field-conformance-cases.ts @@ -0,0 +1,414 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5222] Cross-path conformance corpus for `{ $field }` cross-field + * comparison: the SAME filter, run through the in-memory evaluator + * (`@objectstack/formula` `matchesFilterCondition`) and through SQL push-down, + * must return the SAME rows. + * + * ## Why this corpus exists, and why it is not in `spec/src/data` + * + * #5041 measured the defect this closes: `FieldReferenceSchema` is declared, + * `compileCelToFilter` really PRODUCES it for a field-to-field comparison in a + * CEL permission/RLS rule, and the only implementation was the in-memory + * evaluator — so one rule ran in memory and answered 400 the moment it was + * pushed down. A capability that exists on one path and not the other is a + * per-backend answer to one permission rule, which is the #3948 class; the + * conformance obligation is therefore the WHOLE deliverable, not a nicety. + * + * It lives here rather than in `packages/spec/src/data/*-conformance.ts` + * deliberately. That directory is the (driver × case-set) matrix + * `scripts/check-driver-conformance.mjs` scores, and every case-set there + * obliges EVERY driver to import it or carry a ledger entry. Cross-field + * push-down is a SQL-family capability in v1 — `driver-mongodb` and + * `driver-turso` REMOTE compile filters through their own emitters and were + * not in this issue's scope — so promoting the corpus would enrol three + * drivers that must then be written down as DEBT. That is "a gate that reports + * a known red", which `filter-logic-conformance.ts` names as the thing that + * teaches agents to discount CI's colour. Promote it when the capability + * reaches those backends, not before. + * + * The module is deliberately DEPENDENCY-FREE (no spec import, no driver + * import): `driver-sqlite-wasm`'s suite reads it across the package boundary, + * and a fixture that drags a type graph with it would make that import a + * build-order question. + * + * ## The fixture's shape is the argument + * + * Six rows, each pinning one cell of the comparison's truth table, and the + * SAME cell across three storage classes — numeric, text, calendar date — so + * every case below expects the SAME id set whichever pair it names. A + * class-dependent answer therefore shows up as a diff between three otherwise + * identical cases rather than as a single mystery. + * + * **Rows 4-6 are the load-bearing ones.** SQL is three-valued and the memory + * evaluator is two-valued JS, and that is the one place these two paths can + * genuinely diverge — so the fixture carries every NULL arrangement a pair of + * columns can be in: target NULL (4), referent NULL (5), and BOTH NULL (6). A + * corpus without row 6 would miss the case that decides `$eq`: the memory + * evaluator answers TRUE for it (`resolveValue` yields `null`, and + * `$eq`'s null arm reads `actual == null`), while a bare `a = b` in SQL is + * UNKNOWN and drops the row. Every emitted predicate is therefore written + * TOTAL — see `SqlDriver.applyCrossFieldComparison`. + */ + +/** One fixture row. Every nullable column is genuinely nullable in the DDL. */ +export interface CrossFieldRow { + id: string; + /** Numeric pair. */ + amount: number | null; + budget: number | null; + /** Text pair — ASCII only, so SQL collation and JS code-unit order agree. */ + stage: string | null; + owner: string | null; + /** Calendar-date pair (`YYYY-MM-DD` on both paths, ADR-0053 Phase 1). */ + starts_on: string | null; + ends_on: string | null; + /** The tenant-isolation column — referenced ONLY by the refusal cases. */ + organization_id: string; +} + +/** + * Field declarations the harnesses hand to `initObjects`. Shared so the two + * driver suites cannot drift into fixtures that differ in a way that matters + * (a `NOT NULL` column, or a type whose comparison class differs). + * + * `tags` (multiple) and `projected_total` (formula) carry no scalar column and + * exist only to be REFUSED — see {@link CROSS_FIELD_REFUSALS}. + */ +export const CROSS_FIELD_OBJECT_FIELDS: Record> = { + id: { type: 'text', name: 'id' }, + amount: { type: 'number', name: 'amount' }, + budget: { type: 'number', name: 'budget' }, + stage: { type: 'string', name: 'stage' }, + owner: { type: 'string', name: 'owner' }, + starts_on: { type: 'date', name: 'starts_on' }, + ends_on: { type: 'date', name: 'ends_on' }, + organization_id: { type: 'text', name: 'organization_id' }, + tags: { type: 'select', name: 'tags', multiple: true }, + projected_total: { type: 'formula', name: 'projected_total' }, +}; + +/** + * The fixture. One row per cell of the comparison truth table, replicated + * across all three column pairs so the expectations below are class-independent. + * + * | id | relation of the pair | numeric | text | date | + * |----|---------------------------|----------|---------------|-----------------| + * | 1 | target > referent | 10 / 5 | won / mid | 03-05 / 03-01 | + * | 2 | target < referent | 3 / 5 | lost / mid | 03-01 / 03-05 | + * | 3 | target = referent | 7 / 7 | mid / mid | 03-03 / 03-03 | + * | 4 | target NULL, referent set | – / 5 | – / mid | – / 03-05 | + * | 5 | target set, referent NULL | 10 / – | won / – | 03-05 / – | + * | 6 | BOTH NULL | – / – | – / – | – / – | + */ +export const CROSS_FIELD_ROWS: readonly CrossFieldRow[] = [ + { id: '1', amount: 10, budget: 5, stage: 'won', owner: 'mid', starts_on: '2026-03-05', ends_on: '2026-03-01', organization_id: 'o1' }, + { id: '2', amount: 3, budget: 5, stage: 'lost', owner: 'mid', starts_on: '2026-03-01', ends_on: '2026-03-05', organization_id: 'o1' }, + { id: '3', amount: 7, budget: 7, stage: 'mid', owner: 'mid', starts_on: '2026-03-03', ends_on: '2026-03-03', organization_id: 'o1' }, + { id: '4', amount: null, budget: 5, stage: null, owner: 'mid', starts_on: null, ends_on: '2026-03-05', organization_id: 'o1' }, + { id: '5', amount: 10, budget: null, stage: 'won', owner: null, starts_on: '2026-03-05', ends_on: null, organization_id: 'o1' }, + { id: '6', amount: null, budget: null, stage: null, owner: null, starts_on: null, ends_on: null, organization_id: 'o1' }, +] as const; + +/** One conformance case: a filter, and the ids BOTH paths must return. */ +export interface CrossFieldCase { + /** Stable identifier, usable as a test name. */ + name: string; + filter: unknown; + /** Ids of matching rows, ascending. */ + expected: string[]; + /** Why the case is here — surfaced in failure output. */ + note?: string; +} + +/** + * The supported arm: shapes that MUST compile and MUST agree with the memory + * evaluator, row for row. + * + * The per-class triples are generated rather than written out three times, so + * a case cannot be added to one class and forgotten in the others — the + * class-independence claim above is only worth something if it is total. + */ +const COMPARISON_EXPECTATIONS: ReadonlyArray<{ op: string; expected: string[]; note?: string }> = [ + { op: '$gt', expected: ['1'], note: 'Both sides must have a value — `evalOp` requires `actual != null && v != null`, and the emitted SQL says so explicitly rather than relying on UNKNOWN.' }, + { op: '$gte', expected: ['1', '3'], note: 'The equal row joins the strictly-greater one; the NULL rows stay out.' }, + { op: '$lt', expected: ['2'] }, + { op: '$lte', expected: ['2', '3'], note: 'On the DATE pair this also pins the calendar-day rule: memory `lteBound` reads `<= day` as `< next day`, which is the same answer as SQL `<=` when both sides are day-granular text.' }, + { op: '$eq', expected: ['3', '6'], note: 'Row 6 is the case a naive `a = b` gets WRONG: both columns NULL matches in memory (`resolveValue` → null, so `$eq` takes its `actual == null` arm) and is UNKNOWN in three-valued SQL.' }, + { op: '$ne', expected: ['1', '2', '4', '5'], note: 'The exact complement of `$eq` over all six rows — the two together partition the fixture, which is what makes both answers total rather than merely plausible.' }, +]; + +const CLASS_PAIRS: ReadonlyArray<{ label: string; target: string; ref: string }> = [ + { label: 'numeric', target: 'amount', ref: 'budget' }, + { label: 'text', target: 'stage', ref: 'owner' }, + { label: 'date', target: 'starts_on', ref: 'ends_on' }, +]; + +export const CROSS_FIELD_CASES: readonly CrossFieldCase[] = [ + // ── The six scalar operators, on each storage class ─────────────────────── + ...CLASS_PAIRS.flatMap(({ label, target, ref }) => + COMPARISON_EXPECTATIONS.map(({ op, expected, note }) => ({ + name: `${op} on the ${label} pair (${target} ${op} $field:${ref})`, + filter: { [target]: { [op]: { $field: ref } } }, + expected, + note, + })), + ), + + // ── Self-reference: the totality control ───────────────────────────────── + // + // A column compared to ITSELF has one answer that cannot be argued with, so + // it catches a predicate that is merely well-formed. `$eq` must match every + // row INCLUDING the ones where the column is NULL, and `$ne` must match + // none — if either NULL row goes missing from the first or appears in the + // second, the predicate is UNKNOWN somewhere rather than total. + { + name: 'a column equals itself on every row, NULLs included', + filter: { amount: { $eq: { $field: 'amount' } } }, + expected: ['1', '2', '3', '4', '5', '6'], + note: 'Three-valued `amount = amount` drops rows 4 and 6; the memory evaluator matches them.', + }, + { + name: 'a column differs from itself on no row', + filter: { amount: { $ne: { $field: 'amount' } } }, + expected: [], + note: 'The complement of the case above, and the direction that FAILS OPEN if the null arms are wrong — a `$ne` that admits the NULL rows would widen an RLS scope.', + }, + + // ── Combinator nesting, including De Morgan over a total predicate ──────── + { + name: '$not of a cross-field $gt returns exactly the rows $gt does not', + filter: { $not: { amount: { $gt: { $field: 'budget' } } } }, + expected: ['2', '3', '4', '5', '6'], + note: '#5146 made `$not` NULL-safe by totalising its leaves; a cross-field leaf is total by construction, so the negation is the exact complement — including the NULL rows the JS evaluator returns.', + }, + { + name: '$not of a cross-field $eq returns exactly the rows $eq does not', + filter: { $not: { amount: { $eq: { $field: 'budget' } } } }, + expected: ['1', '2', '4', '5'], + note: 'Row 6 (both NULL) must be EXCLUDED here — it satisfies the inner `$eq`. A guard that treated a NULL target as failing the operator would wrongly re-admit it.', + }, + { + name: 'two cross-field operators on one field AND within the constraint', + filter: { amount: { $gte: { $field: 'budget' }, $ne: { $field: 'budget' } } }, + expected: ['1'], + note: 'Everything inside ONE filter object ANDs, at every depth — the same rule `filter-logic-conformance` pins for literals.', + }, + { + name: 'a cross-field comparison ANDs with a literal predicate on another field', + filter: { amount: { $gt: { $field: 'budget' } }, stage: 'won' }, + expected: ['1'], + }, + { + name: '$or of two cross-field branches', + filter: { $or: [{ amount: { $lt: { $field: 'budget' } } }, { amount: { $eq: { $field: 'budget' } } }] }, + expected: ['2', '3', '6'], + }, + { + name: '$or of a cross-field branch and a literal branch', + filter: { $or: [{ amount: { $gt: { $field: 'budget' } } }, { stage: 'lost' }] }, + expected: ['1', '2'], + }, + { + name: '$and of a cross-field branch and a literal branch', + filter: { $and: [{ amount: { $gte: { $field: 'budget' } } }, { $not: { stage: 'won' } }] }, + expected: ['3'], + note: 'Row 1 is `won` and drops out; row 3 survives. Also exercises a NULL-safe literal `$not` beside a cross-field conjunct.', + }, + { + name: '$not over a $or of cross-field branches (De Morgan)', + filter: { + $not: { + $or: [ + { amount: { $gt: { $field: 'budget' } } }, + { amount: { $eq: { $field: 'budget' } } }, + ], + }, + }, + expected: ['2', '4', '5'], + note: 'The complement of {1} ∪ {3,6}. A guard hoisted to the top of the `$not` instead of sitting on each leaf re-admits rows here — the failure #5146 wrote its rewrite to avoid.', + }, + { + name: 'a cross-field comparison nested two combinators deep', + filter: { $or: [{ $and: [{ amount: { $gt: { $field: 'budget' } } }, { stage: 'won' }] }, { stage: 'lost' }] }, + expected: ['1', '2'], + }, +] 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. + * + * Every entry must throw `INVALID_FILTER` / 400 (ADR-0112) on BOTH SQL + * drivers. `messageIncludes` pins the part of the wording a caller needs to + * act on; the tests assert the envelope regardless. + * + * Read this table together with {@link CROSS_FIELD_CASES}: what makes the + * refusals defensible is that the supported arm above is proven equivalent, so + * the line between them is drawn at "cannot be proven equivalent", not at + * "was not attempted". + */ +export interface CrossFieldRefusalCase { + name: string; + filter: unknown; + /** Substrings the refusal message must contain. */ + messageIncludes: string[]; + /** + * Why the shape is refused — surfaced in failure output. Optional because + * several entries are one spelling of a reason the entry above them states + * in full (the string-operator family, the `$between` endpoints); repeating + * it per row would make the table read as five reasons where there is one. + */ + note?: string; +} + +export const CROSS_FIELD_REFUSALS: readonly CrossFieldRefusalCase[] = [ + // ── Ruling 1: same-table columns ONLY ───────────────────────────────────── + { + name: 'a dotted relation path is refused', + filter: { amount: { $gt: { $field: 'account.budget' } } }, + messageIncludes: ['dotted path', 'same-table'], + note: 'Maintainer ruling (2026-08-06) point 1 = A: no JOIN planning (disproportionate) and no alias-qualified columns (no alias contract). The memory evaluator DOES walk the path, so this is a deliberate, loudly-reported asymmetry rather than a silent one.', + }, + { + name: 'a dotted path is refused even when its head names a real column', + filter: { amount: { $gt: { $field: 'budget.nested' } } }, + messageIncludes: ['dotted path'], + note: 'The refusal is on the SHAPE, not on whether the first segment happens to resolve — otherwise the check would depend on data the compiler cannot see.', + }, + + // ── Ruling 2: declared-only enumeration ────────────────────────────────── + { + name: 'an undeclared column is refused at compile time', + filter: { amount: { $gt: { $field: 'no_such_column' } } }, + messageIncludes: ['not a declared field'], + note: 'The `$field` value lands in a SQL IDENTIFIER position. cloud#1051: letting it through unchecked is dismantling the guard rail — and a compile-time refusal is what makes AI-authored metadata wrong at authoring time rather than in the database.', + }, + { + name: 'an undeclared TARGET field is refused too', + filter: { no_such_column: { $gt: { $field: 'budget' } } }, + messageIncludes: ['not a declared field'], + note: 'A comparison is one surface — validating only the referent would leave half of it unchecked, and the type-class rule below needs both declarations anyway.', + }, + + // ── Ruling 2, the security half: the tenant-isolation column ───────────── + { + name: 'the tenant-isolation column is refused as the REFERENT', + filter: { stage: { $eq: { $field: 'organization_id' } } }, + messageIncludes: ['tenant-isolation column'], + note: 'The named ruling. A comparison against the isolation column is a privilege-escalation comparison surface: it lets a filter probe the tenant boundary the driver injects rather than being scoped by it.', + }, + { + name: 'the tenant-isolation column is refused as the TARGET', + filter: { organization_id: { $eq: { $field: 'stage' } } }, + messageIncludes: ['tenant-isolation column'], + note: 'Closed because the operands of `=` COMMUTE — a ban that a swap of the two sides walks around is not a ban. Ruling names the referent; this is the same surface spelled backwards.', + }, + + // ── The conformance boundary: comparison CLASS ─────────────────────────── + { + name: 'a TEXT column compared to a numeric column is refused (the measured divergence)', + filter: { stage: { $gt: { $field: 'amount' } } }, + messageIncludes: ['stored as'], + note: 'THE case that proves the class check is load-bearing, and it is directional. Measured with the check disabled: SQLite answers rows 1,2,3,5 — it orders by STORAGE CLASS first, so every TEXT sorts above every INTEGER — while the in-memory evaluator answers NONE, because JS coerces `"won" > 10` to a NaN comparison. Four rows of difference on one filter.', + }, + { + name: 'a numeric column compared to a text column is refused (the mirrored spelling)', + filter: { amount: { $gt: { $field: 'stage' } } }, + messageIncludes: ['stored as'], + note: 'The mirror of the case above, and measured to AGREE (both answer nothing) — kept in the refusal arm anyway, because a guard that admitted exactly the pairings one fixture measured as agreeing would be a rule about this data rather than about the types.', + }, + { + name: 'a date column compared to a text column is refused', + filter: { starts_on: { $gt: { $field: 'stage' } } }, + messageIncludes: ['stored as'], + note: 'Both are TEXT physically, so this one WOULD have compiled — and measured, both paths agree. It is refused because they agree by lexicographic accident rather than by any temporal reading, which is also why the class check reads declared TYPES rather than physical affinity.', + }, + { + name: 'a numeric column compared to a date column is refused', + filter: { amount: { $gt: { $field: 'starts_on' } } }, + messageIncludes: ['stored as'], + }, + + // ── Columns with no scalar stored form ────────────────────────────────── + { + name: 'a multi-valued (JSON) column is refused as the referent', + filter: { amount: { $gt: { $field: 'tags' } } }, + messageIncludes: ['no scalar stored'], + note: 'A JSON column holds a serialized array; SQL comparison operators have no element-wise reading of it, and #7398 already refuses the scalar operators on such a column for a value comparand.', + }, + { + name: 'a formula (virtual) column is refused as the referent', + filter: { amount: { $gt: { $field: 'projected_total' } } }, + messageIncludes: ['no scalar stored'], + note: 'A formula field is virtual — `createColumn` emits no column at all, so there is nothing to reference. Declared-only enumeration alone would have ADMITTED it, which is why the class check is a second gate rather than a restatement of the first.', + }, + + // ── List positions — refused, and NOT merely unimplemented ────────────── + // + // The memory evaluator does not resolve a `{ $field }` INSIDE a list either: + // `resolveValue` returns an array unchanged, so `$in`/`$nin` compare against + // the raw reference OBJECT (never equal to a stored value) and `$between` + // orders against it. There is therefore no correct in-memory semantics for + // SQL to be equivalent TO — refusing is the only answer that is not a guess, + // and the spec's declaration of `FieldReferenceSchema` in the `$between` + // endpoints is filed as its own finding rather than being resolved here. + { + name: 'a $field member of an $in list is refused', + filter: { amount: { $in: [{ $field: 'budget' }, 1] } }, + messageIncludes: ['index 0'], + note: 'Before #5041 this did not even crash: it compiled, ran, and returned ZERO ROWS. The index is named because it is the only thing distinguishing the bad member from its legitimate neighbours.', + }, + { + name: 'a $field member of a $nin list is refused', + filter: { amount: { $nin: [{ $field: 'budget' }] } }, + messageIncludes: ['index 0'], + note: 'The $nin direction is the dangerous one — a lost member drops an EXCLUSION the caller wrote, widening the result set.', + }, + { + name: 'a $field lower bound of a $between is refused', + filter: { amount: { $between: [{ $field: 'budget' }, 100] } }, + messageIncludes: ['index 0'], + }, + { + name: 'a $field upper bound of a $between is refused', + filter: { amount: { $between: [0, { $field: 'budget' }] } }, + messageIncludes: ['index 1'], + }, + + // ── String operators — refused in v1, and the reason is a filter bypass ── + // + // In memory the referenced value is matched as a LITERAL substring. Compiled + // to SQL it becomes the TEXT of a LIKE pattern, so any `%` / `_` / `\` + // STORED in the referenced column would act as a WILDCARD — the P0 + // filter-bypass class `applyLike` escapes literal comparands to prevent. + // Escaping a column-side value needs a per-dialect REPLACE chain that has + // not been proven, so v1 refuses. Recorded as a decision, not an omission. + { + name: '$startsWith against a field reference is refused', + filter: { stage: { $startsWith: { $field: 'owner' } } }, + messageIncludes: ['$field'], + note: 'v1 refusal: a column-side LIKE pattern cannot be metacharacter-escaped portably, and an unescaped one is the `%`-matches-every-row bypass.', + }, + { + name: '$contains against a field reference is refused', + filter: { stage: { $contains: { $field: 'owner' } } }, + messageIncludes: ['$field'], + }, + { + name: '$endsWith against a field reference is refused', + filter: { stage: { $endsWith: { $field: 'owner' } } }, + messageIncludes: ['$field'], + }, + { + name: '$notContains against a field reference is refused', + filter: { stage: { $notContains: { $field: 'owner' } } }, + messageIncludes: ['$field'], + }, + { + name: '$icontains against a field reference is refused', + filter: { stage: { $icontains: { $field: 'owner' } } }, + messageIncludes: ['$field'], + }, +] as const; diff --git a/packages/drivers/driver-sql/src/index.ts b/packages/drivers/driver-sql/src/index.ts index a9b4322504..14114358df 100644 --- a/packages/drivers/driver-sql/src/index.ts +++ b/packages/drivers/driver-sql/src/index.ts @@ -24,6 +24,30 @@ export type { SqlWindowFunctionQuery, } from './sql-driver.js'; +// [#5222] The cross-field `{ $field }` push-down conformance corpus: one +// filter, run through the in-memory evaluator AND through a SQL driver, must +// return the same rows. +// +// Exported rather than kept module-private for two reasons. `driver-sqlite-wasm` +// inherits this driver's compiler but executes through its own sql.js dialect, +// so it runs the same corpus from its own package — and a relative import +// across the package boundary is not available to it (`rootDir`), so a shared +// corpus has to be a real export or a second copy. And a third-party driver +// author extending `SqlDriver` can check a new backend against the same table, +// 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_CASES, + CROSS_FIELD_OBJECT_FIELDS, + CROSS_FIELD_REFUSALS, + CROSS_FIELD_ROWS, +} from './cross-field-conformance-cases.js'; +export type { + CrossFieldCase, + CrossFieldRefusalCase, + CrossFieldRow, +} from './cross-field-conformance-cases.js'; + // Managed-schema drift / reconcile (#2186), incl. the index dimension (#3728) export { applyIndexKeyParts, 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 new file mode 100644 index 0000000000..8437b7c5be --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-cross-field-conformance.test.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5222] Cross-path conformance: one `{ $field }` filter, two execution + * paths, the same rows. + * + * This is the acceptance face of the issue. #5041 measured a capability that + * existed on ONE path — `compileCelToFilter` emits `{ $field: path }` for a + * field-to-field comparison in a CEL permission/RLS rule, the in-memory + * evaluator resolves it, and SQL push-down answered 400 — so one permission + * rule had two behaviours chosen by whether the query reached a database. This + * suite is what makes the new compilation *equivalent* rather than merely + * present: every case runs through `@objectstack/formula`'s + * `matchesFilterCondition` AND through this driver, against the SAME seeded + * rows, and both are held to the same declared id list. + * + * ## Why both paths are asserted against `expected`, not just against each other + * + * Two paths agreeing proves nothing on its own — they can agree on a wrong + * answer, and a bug in the fixture makes them agree trivially. The declared + * `expected` list is a third, independent statement of the semantics, so a + * case fails loudly when both paths drift together. It is the same discipline + * `filter-logic-conformance` applies for the combinators. + * + * ## The NULL rows are the point + * + * SQL is three-valued and the memory evaluator is two-valued JS, and that is + * the ONE place these paths can genuinely diverge. The corpus therefore + * carries every NULL arrangement two columns can be in (rows 4-6), and + * `$eq`/`$ne` over row 6 — both columns NULL — is the case a naive `a = b` + * lowering gets wrong. See `cross-field-conformance-cases.ts`. + * + * The refusal arm is asserted here too, in the same file and against the same + * fixture: the boundary is only defensible while the supported arm is proven, + * so the two halves are read together rather than filed apart. + * + * ## The DIALECT axis, and why this suite runs it + * + * Every sweep below runs once per cell of `DIALECT_CELLS` — SQLite always, + * live Postgres and MySQL when the runner provisions them (ADR-0053 D-A3's + * "Postgres at minimum", and the reason `live-dialect-matrix.testkit.ts` + * exists rather than a hard-coded `client: 'better-sqlite3'` per suite). + * + * It earns the axis rather than inheriting it by convention. A cross-field + * predicate is the one filter shape whose SQL text carries TWO identifiers and + * no bound value, so it exercises per-dialect identifier quoting where an + * ordinary comparison exercises parameter binding. And the type rule this + * capability enforces has a genuinely different failure per dialect: comparing + * a text column to a numeric one is a silent wrong answer on SQLite (storage + * classes order before values), while Postgres raises an operator-does-not- + * exist error outright. The class guard is what neither backend ever gets to + * see — a matrix is how that stays true. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { matchesFilterCondition } from '@objectstack/formula'; +import 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_CASES, + CROSS_FIELD_OBJECT_FIELDS, + CROSS_FIELD_REFUSALS, + CROSS_FIELD_ROWS, +} from './cross-field-conformance-cases.js'; + +const TABLE = 'cross_field_deal'; + +function declareCrossFieldSweep(cell: DialectCell): void { +describe(`[#5222] driver-sql — cross-field \`$field\` push-down conformance (${cell.label})`, () => { + let driver: SqlDriver; + /** The seeded rows AS THE DRIVER RETURNS THEM — the memory path's input. */ + let records: Array>; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + // Live cells reuse one database, so the sweep starts from a dropped table. + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.initObjects([{ name: TABLE, fields: CROSS_FIELD_OBJECT_FIELDS } as any]); + for (const row of CROSS_FIELD_ROWS) await driver.create(TABLE, { ...row }); + // Reading the rows BACK (rather than evaluating the literals in + // CROSS_FIELD_ROWS) is deliberate: it makes the memory path's input the + // same storage round-trip the SQL path compares against, so a coercion + // that changed a value on write would fail here instead of being + // cancelled out by a fixture that never went through the database. It also + // makes the memory path see each DIALECT's read shape, which is the point + // of running this sweep per cell. + records = (await driver.find(TABLE, {})) as Array>; + }); + + afterAll(async () => { + await driver?.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver?.disconnect?.(); + }); + + it('the fixture round-tripped with its NULLs intact', () => { + // The control every null case depends on. A `NOT NULL` column or a write + // coercion that substituted `''` for `null` would turn the NULL cases + // green for the wrong reason — the divergence they exist to catch is + // exactly what a row with no value does. + expect(records).toHaveLength(CROSS_FIELD_ROWS.length); + const byId = new Map(records.map((r) => [r.id as string, r])); + expect(byId.get('6')!.amount).toBeNull(); + expect(byId.get('6')!.stage).toBeNull(); + expect(byId.get('6')!.starts_on).toBeNull(); + expect(byId.get('4')!.amount).toBeNull(); + expect(byId.get('4')!.budget).toBe(5); + expect(byId.get('5')!.budget).toBeNull(); + expect(byId.get('5')!.amount).toBe(10); + }); + + const sqlIds = async (filter: unknown): Promise => { + const rows = await driver.find(TABLE, { + fields: ['id'], + where: filter as FilterCondition, + }); + return rows.map((r: any) => String(r.id)).sort(); + }; + + const memoryIds = (filter: unknown): string[] => + records + .filter((r) => matchesFilterCondition(r, filter as FilterCondition)) + .map((r) => String(r.id)) + .sort(); + + for (const testCase of CROSS_FIELD_CASES) { + it(`${testCase.name} — same rows on both paths`, async () => { + const expected = [...testCase.expected].sort(); + const note = testCase.note ? `\n${testCase.note}` : ''; + + // The memory path first: it is the reference implementation this issue + // is bringing SQL into line with, so when a case fails it matters + // whether the reference itself moved. + expect(memoryIds(testCase.filter), `in-memory evaluator disagreed${note}`).toEqual(expected); + expect(await sqlIds(testCase.filter), `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 () => { + let error: (Error & { code?: string; status?: number }) | null = null; + try { + await sqlIds(refusal.filter); + } catch (e) { + error = e as Error & { code?: string; status?: number }; + } + expect(error, `expected a refusal${refusal.note ? `\n${refusal.note}` : ""}`).not.toBeNull(); + expect(error!.code).toBe('INVALID_FILTER'); + expect(error!.status).toBe(400); + // #5041's floor: never a bare TypeError from the binder, and never + // driver-internal wording on the wire (#3867). + expect(error!).not.toBeInstanceOf(TypeError); + expect(error!.message).not.toContain('can only bind'); + expect(error!.message).not.toContain('[sql-driver]'); + for (const fragment of refusal.messageIncludes) { + expect(error!.message).toContain(fragment); + } + }); + } + }); +}); +} + +// ── The driver axis ───────────────────────────────────────────────────────── + +for (const cell of DIALECT_CELLS) { + if (!cell.available) { + // Reported, never omitted — a named skip locally, a red under + // `OS_EXPECT_LIVE_DIALECT_MATRIX=1`. The guard is the testkit's, shared + // with the temporal / pagination / filter-logic matrices, so it cannot + // weaken in this consumer's copy alone. + declareUnprovisionedCell(cell, 'cross-field comparison'); + continue; + } + declareCrossFieldSweep(cell); +} 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 7018bd2e0c..36dcd412e6 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 @@ -1,40 +1,42 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#5041] A `{ $field }` cross-field comparison is REFUSED by this driver, in - * the ADR-0112 envelope — never as a bare `TypeError`, never as zero rows. + * [#5041 → #5222] The `{ $field }` POSITION MATRIX: which spellings of a + * cross-field comparison this driver compiles, and which it refuses. * - * `FieldReferenceSchema` (`packages/spec/src/data/filter.zod.ts`) is declared, - * and it is genuinely PRODUCED: `compileCelToFilter` emits `{ $field: path }` - * whenever a CEL permission/RLS rule compares one field to another. The only - * implementation in the repo is the in-memory evaluator - * (`packages/formula/src/matches-filter.ts` — `resolveValue`). Pushed down to - * SQL, the reference object was handed to Knex as a BIND VALUE: + * ## What changed, and what deliberately did not * - * ``` - * { amount: { $gt: { $field: 'budget' } } } - * → select `id` from `deal` where `amount` > {"$field":"budget"} - * → TypeError: SQLite3 can only bind numbers, strings, bigints, buffers, and null - * ``` + * #5041 measured the defect and installed a refusal in every position: pushed + * down to SQL, the reference object was handed to Knex as a BIND VALUE, so + * sqlite answered with a bare `TypeError` ("can only bind numbers, strings, + * bigints, buffers, and null") carrying no `code` and no `status` — outside + * the ADR-0112 envelope, and therefore an opaque 500 to the client. Inside an + * `$in` list it did not even crash: the query compiled, ran, and returned ZERO + * ROWS. The maintainer's adjudication took the minimum path and tracked + * column-to-column compilation as its own capability. * - * That error carried no `code` and no `status`, so it landed outside the - * envelope every sibling filter refusal in this driver already speaks (#4436 / - * ADR-0112) and reached the client as an opaque server error. The maintainer's - * adjudication on #5041 is the minimum path: refuse loudly here, keep the spec - * declaration, and track column-to-column compilation as its own capability. + * #5222 is that capability, and it NARROWS the refusal rather than removing + * it. This file is the matrix of the two arms — the same position grid the + * #5041 suite carried (six scalar operators, array triples symbolic and word, + * `$and`/`$or`/`$not` nesting, the string family, `$in`/`$between` list + * members), re-read as supported-vs-refused. * - * These tests assert the FULL envelope — `code`, `status`, and the message - * content a caller needs to act — not merely that something was thrown. + * **The row semantics are pinned elsewhere, on purpose.** This file asserts + * that a spelling compiles and which rows it returns for one small fixture; + * `sql-driver-cross-field-conformance.test.ts` is what proves those rows are + * the SAME rows the in-memory evaluator returns, over a fixture built to carry + * every NULL arrangement. Keep the equivalence claims there — a matrix that + * also tried to be the conformance suite would state the semantics twice and + * let the two copies drift. * * **Negative control** for the other half of the contract (the memory path - * still RESOLVES `$field` and matches correctly) lives with that implementation - * and is unchanged by this fix: `packages/formula/src/matches-filter.test.ts` - * ("$field reference (field-to-field)"). Nothing in this change touches the - * evaluator or the `cel-to-filter` producer. + * resolves `$field` against the record, dot paths included) lives with that + * implementation and is untouched by this change: + * `packages/formula/src/matches-filter.test.ts`. */ import { describe, it, expect, beforeEach } from 'vitest'; -import { SqlDriver } from '../src/index.js'; +import { SqlDriver } from './index.js'; import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; /** The shape `mapDataError` / `sendError` read off a thrown driver error. */ @@ -52,7 +54,7 @@ async function refusalOf(run: () => Promise): Promise throw new Error('expected the driver to refuse this filter, but it resolved'); } -describe('[#5041] SqlDriver refuses `$field` cross-field comparison in the ADR-0112 envelope', () => { +describe('[#5222] SqlDriver `$field` position matrix — compiled vs refused', () => { let driver: SqlDriver; beforeEach(async () => { @@ -67,138 +69,222 @@ describe('[#5041] SqlDriver refuses `$field` cross-field comparison in the ADR-0 fields: { id: { type: 'text', name: 'id' }, stage: { type: 'text', name: 'stage' }, + note: { type: 'text', name: 'note' }, amount: { type: 'number', name: 'amount' }, budget: { type: 'number', name: 'budget' }, + organization_id: { type: 'text', name: 'organization_id' }, }, } as any, ]); - // `amount > budget` is TRUE for this row, so a driver that silently dropped - // the predicate would return it — the failure mode is visible, not implied. - await driver.create('deal', { id: '1', stage: 'won', amount: 10, budget: 5 }); + // `amount > budget` is TRUE for this row and `amount = budget` is FALSE, so + // a compiled comparison and a dropped predicate are distinguishable — the + // failure mode stays visible rather than implied. + await driver.create('deal', { + id: '1', stage: 'won', note: 'won', amount: 10, budget: 5, organization_id: 'o1', + }); }); const find = (where: unknown) => driver.find('deal', { fields: ['id'], where: where as FilterCondition }); + const ids = async (where: unknown) => (await find(where)).map((r: any) => String(r.id)); - it('the issue repro — `{ amount: { $gt: { $field: "budget" } } }` — carries the full envelope', async () => { - const err = await refusalOf(() => find({ amount: { $gt: { $field: 'budget' } } })); - - // ADR-0112 wire identity: the catalogued code and a client-error status. - expect(err.code).toBe('INVALID_FILTER'); - expect(err.status).toBe(400); + // ── SUPPORTED: the six scalar comparison operators ──────────────────────── + // + // The issue's repro is the first row. Each expectation is decided by the one + // seeded row (amount 10, budget 5), so a predicate that compiled to + // something unrelated — or was dropped entirely — changes the answer. + describe('the six scalar operators compile to a column-to-column comparison', () => { + const supported: Array<[string, unknown, string[]]> = [ + ['$gt — the #5041 repro', { amount: { $gt: { $field: 'budget' } } }, ['1']], + ['$gte', { amount: { $gte: { $field: 'budget' } } }, ['1']], + ['$lt', { amount: { $lt: { $field: 'budget' } } }, []], + ['$lte', { amount: { $lte: { $field: 'budget' } } }, []], + ['$eq', { amount: { $eq: { $field: 'budget' } } }, []], + ['$ne', { amount: { $ne: { $field: 'budget' } } }, ['1']], + ]; - // NOT the pre-fix failure: a bare TypeError with neither. - expect(err).not.toBeInstanceOf(TypeError); - expect(err.message).not.toContain('can only bind'); + for (const [name, where, expected] of supported) { + it(`${name} → ${expected.length ? 'matches' : 'excludes'} the row`, async () => { + expect(await ids(where)).toEqual(expected); + }); + } - // #3867 — driver-internal wording never ships to a client. - expect(err.message).not.toContain('[sql-driver]'); + it('a reference object carrying EXTRA keys still resolves, matching the memory path', async () => { + // `fieldReferenceOf` recognises "an object, not an array, carrying a + // string `$field`" because `formula`'s `resolveValue` recognises exactly + // that (`'$field' in raw`) — both ignore any other key, and + // `FieldReferenceSchema` is a non-strict zod object, so the extra key is + // stripped rather than rejected. Pinned as SUPPORTED rather than left + // unstated: a driver that recognised a NARROWER shape than the evaluator + // would silently bind the remainder as a literal again, which is the + // #5041 defect returning at one more spelling. + expect(await ids({ amount: { $gt: { $field: 'budget', extra: 1 } } })).toEqual(['1']); + }); - // The actionable half: which field, which operator, which reference, and - // the reason — cross-field comparison is memory-path-only today. - expect(err.message).toContain('amount'); - expect(err.message).toContain('$gt'); - expect(err.message).toContain('budget'); - expect(err.message).toContain('$field'); - expect(err.message).toContain('in-memory'); - expect(err.message).toContain('matchesFilter'); + it('emits a real column reference, not a bound literal', async () => { + // The distinguishing measurement, and the reason this assertion exists + // beside the row-level ones: a compiler that bound the STRING 'budget' + // would answer `[]` for `$gt` (10 > 'budget' is false in SQLite's + // storage-class ordering) and would keep answering plausibly for other + // shapes. Comparing the column to itself can only be satisfied by a + // genuine identifier on the right-hand side. + expect(await ids({ amount: { $eq: { $field: 'amount' } } })).toEqual(['1']); + expect(await ids({ budget: { $lt: { $field: 'amount' } } })).toEqual(['1']); + }); }); - // One condition — "this comparison references another field" — gets one - // answer however the caller spelled it. Each of these bound the reference - // object as a VALUE before the fix. - const spellings: Array<[string, unknown]> = [ - ['$eq', { amount: { $eq: { $field: 'budget' } } }], - ['$ne', { amount: { $ne: { $field: 'budget' } } }], - ['$gte', { amount: { $gte: { $field: 'budget' } } }], - ['$lt', { amount: { $lt: { $field: 'budget' } } }], - ['$lte', { amount: { $lte: { $field: 'budget' } } }], - ['nested under $and', { $and: [{ amount: { $gt: { $field: 'budget' } } }] }], - ['nested under $or', { $or: [{ amount: { $gt: { $field: 'budget' } } }] }], - ['nested under $not', { $not: { amount: { $gt: { $field: 'budget' } } } }], - ['array triple, symbolic op', [['amount', '>', { $field: 'budget' }]]], - ['array triple, word op', [['amount', 'gt', { $field: 'budget' }]]], - ['LIKE family (would have stringified to `[object Object]`)', - { stage: { $startsWith: { $field: 'budget' } } }], - ]; - - for (const [name, where] of spellings) { - it(`${name} → 400 INVALID_FILTER naming the reference`, async () => { - const err = await refusalOf(() => find(where)); + // ── SUPPORTED: the authored array-triple dialect, once lowered ─────────── + // + // #5158 made `FilterArray` INPUT-ONLY authoring sugar: both doors into the + // runtime lower it before a driver is reached, so the triple is exercised + // the way it actually arrives. `>` and `gt` lower to `$gt`; the `=` / `equals` + // spellings lower to the BARE form, which has its own arm below. + describe('array triples compile once lowered by parseFilterAST (#5158)', () => { + const triples: Array<[string, unknown]> = [ + ['symbolic op', [['amount', '>', { $field: 'budget' }]]], + ['word op', [['amount', 'gt', { $field: 'budget' }]]], + ]; + + for (const [name, authored] of triples) { + it(`${name} → lowers to $gt and matches`, async () => { + expect(await ids(parseFilterAST(authored as any))).toEqual(['1']); + }); + } + + it('a raw (unlowered) array still hits the array refusal, unchanged (#5158)', async () => { + // Not a `$field` refusal — the array never reaches the comparand gate. + // Pinned so a future reader does not mistake the old suite's passing + // "array triple" rows for evidence about cross-field support: they were + // refused for being arrays. + const err = await refusalOf(() => find([['amount', '>', { $field: 'budget' }]])); expect(err.code).toBe('INVALID_FILTER'); expect(err.status).toBe(400); - expect(err).not.toBeInstanceOf(TypeError); - expect(err.message).toContain('$field'); - expect(err.message).toContain('budget'); }); - } + }); - // A `$field` inside a LIST did not even crash before the fix: it compiled and - // returned ZERO ROWS. A silent wrong answer on a permission-scoped read is - // the failure #3948 / #4209 exist to prevent, so it gets the same refusal. - const listCases: Array<[string, unknown]> = [ - ['$in', { amount: { $in: [{ $field: 'budget' }, 1] } }], - ['$nin', { amount: { $nin: [{ $field: 'budget' }] } }], - ['$between lower bound', { amount: { $between: [{ $field: 'budget' }, 100] } }], - ['$between upper bound', { amount: { $between: [0, { $field: 'budget' }] } }], - ]; - - for (const [name, where] of listCases) { - it(`${name} with a $field member → refused, not silently zero rows`, async () => { - const err = await refusalOf(() => find(where)); + // ── SUPPORTED: nested under every combinator ───────────────────────────── + describe('the combinators carry a cross-field leaf', () => { + it('under $and', async () => { + expect(await ids({ $and: [{ amount: { $gt: { $field: 'budget' } } }] })).toEqual(['1']); + }); + it('under $or', async () => { + expect(await ids({ $or: [{ amount: { $gt: { $field: 'budget' } } }] })).toEqual(['1']); + }); + it('under $not — negated, and the row drops out', async () => { + expect(await ids({ $not: { amount: { $gt: { $field: 'budget' } } } })).toEqual([]); + }); + it('under $not with the comparison inverted — the row comes back', async () => { + expect(await ids({ $not: { amount: { $lt: { $field: 'budget' } } } })).toEqual(['1']); + }); + }); + + // ── REFUSED: the boundary of v1 ────────────────────────────────────────── + // + // Every one of these keeps the #5041 envelope. The reasons are recorded with + // the cases in `cross-field-conformance-cases.ts`, which drives the same + // list against both SQL drivers; this block pins the ones whose WORDING a + // caller has to act on, plus the positions unique to this matrix. + describe('refused positions keep the INVALID_FILTER envelope', () => { + const refused: Array<[string, unknown, string]> = [ + ['a dotted relation path', { amount: { $gt: { $field: 'a.b' } } }, 'dotted path'], + ['an undeclared column', { amount: { $gt: { $field: 'nope' } } }, 'not a declared field'], + ['the tenant column as referent', { stage: { $eq: { $field: 'organization_id' } } }, 'tenant-isolation column'], + ['the tenant column as target', { organization_id: { $eq: { $field: 'stage' } } }, 'tenant-isolation column'], + ['a cross-class comparison', { amount: { $gt: { $field: 'stage' } } }, 'stored as'], + ['$in list member', { amount: { $in: [{ $field: 'budget' }, 1] } }, 'index 0'], + ['$nin list member', { amount: { $nin: [{ $field: 'budget' }] } }, 'index 0'], + ['$between lower bound', { amount: { $between: [{ $field: 'budget' }, 100] } }, 'index 0'], + ['$between upper bound', { amount: { $between: [0, { $field: 'budget' }] } }, 'index 1'], + ['$startsWith', { stage: { $startsWith: { $field: 'note' } } }, '$field'], + ['$contains', { stage: { $contains: { $field: 'note' } } }, '$field'], + ['$endsWith', { stage: { $endsWith: { $field: 'note' } } }, '$field'], + ['$notContains', { stage: { $notContains: { $field: 'note' } } }, '$field'], + ['$icontains', { stage: { $icontains: { $field: 'note' } } }, '$field'], + ]; + + for (const [name, where, fragment] of refused) { + it(`${name} → 400 INVALID_FILTER naming the reason`, async () => { + const err = await refusalOf(() => find(where)); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err).not.toBeInstanceOf(TypeError); + expect(err.message).not.toContain('can only bind'); + expect(err.message).not.toContain('[sql-driver]'); + expect(err.message).toContain(fragment); + }); + } + + it('a refused position stays refused INSIDE a combinator', async () => { + // The gate sits at the comparison emitter, which every combinator + // recurses into — so nesting is not a way around it. Worth pinning + // because a validation placed at the top-level entry instead would pass + // this and refuse only the flat spelling. + const err = await refusalOf(() => + find({ $or: [{ stage: 'won' }, { amount: { $gt: { $field: 'organization_id' } } }] }), + ); expect(err.code).toBe('INVALID_FILTER'); - expect(err.status).toBe(400); - expect(err.message).toContain('$field'); - // The member's position is named, so a long list is still actionable. - expect(err.message).toMatch(/index \d+/); + expect(err.message).toContain('tenant-isolation column'); }); - } - // The general arm the issue reported as missing: a KNOWN operator whose value - // shape cannot be bound. Measured pre-fix, every one of these was the same - // bare `TypeError` as the `$field` case. - const uncompilable: Array<[string, unknown]> = [ - ['$gt with a plain object', { amount: { $gt: { foo: 1 } } }], - ['$eq with a plain object', { amount: { $eq: { foo: 1 } } }], - ['$ne with a plain object', { amount: { $ne: { foo: 1 } } }], - ['$gt with an array', { amount: { $gt: [1, 2] } }], - ['$eq with an array', { amount: { $eq: [1, 2] } }], - ['implicit `=` with a plain object', { amount: { } }], - ]; - - for (const [name, where] of uncompilable) { - it(`${name} → 400 INVALID_FILTER instead of a bare TypeError`, async () => { - const err = await refusalOf(() => find(where)); + 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)); expect(err.code).toBe('INVALID_FILTER'); expect(err.status).toBe(400); - expect(err).not.toBeInstanceOf(TypeError); - expect(err.message).not.toContain('can only bind'); - expect(err.message).not.toContain('[sql-driver]'); - expect(err.message).toContain('amount'); + expect(err.message).toContain('$eq'); + expect(err.message).toContain('budget'); }); - } + }); + + // ── The general arm #5041 installed, untouched by the narrowing ────────── + // + // A KNOWN operator whose comparand is a shape no dialect can bind. These + // were the same bare `TypeError` before #5041; the `$field` narrowing must + // not have re-opened any of them, since `fieldReferenceOf` recognises only + // an object carrying a STRING `$field`. + describe('non-$field uncompilable comparands stay refused (#5041/#5234)', () => { + const uncompilable: Array<[string, unknown]> = [ + ['$gt with a plain object', { amount: { $gt: { foo: 1 } } }], + ['$eq with a plain object', { amount: { $eq: { foo: 1 } } }], + ['$ne with a plain object', { amount: { $ne: { foo: 1 } } }], + ['$gt with an array', { amount: { $gt: [1, 2] } }], + ['$eq with an array', { amount: { $eq: [1, 2] } }], + ['a field spec with no operators', { amount: {} }], + ['a $field whose value is not a string', { amount: { $gt: { $field: 42 } } }], + ]; + + for (const [name, where] of uncompilable) { + it(`${name} → 400 INVALID_FILTER instead of a bare TypeError`, async () => { + const err = await refusalOf(() => find(where)); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err).not.toBeInstanceOf(TypeError); + expect(err.message).not.toContain('can only bind'); + expect(err.message).not.toContain('[sql-driver]'); + }); + } + }); - // The guard must not narrow what already compiled. These are the shapes it - // sits directly in front of. + // ── The guard must not narrow what already compiled ───────────────────── describe('comparands that legitimately compile are untouched', () => { it('a scalar equality still matches', async () => { - const rows = await find({ stage: 'won' }); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids({ stage: 'won' })).toEqual(['1']); }); it('$in with a real value list still matches', async () => { - const rows = await find({ amount: { $in: [10, 20] } }); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids({ amount: { $in: [10, 20] } })).toEqual(['1']); }); it('$nin with a real value list still matches', async () => { - const rows = await find({ amount: { $nin: [99] } }); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids({ amount: { $nin: [99] } })).toEqual(['1']); }); it('$between with a real range still matches', async () => { - const rows = await find({ amount: { $between: [0, 100] } }); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids({ amount: { $between: [0, 100] } })).toEqual(['1']); }); it('a Date comparand still binds', async () => { @@ -206,13 +292,20 @@ describe('[#5041] SqlDriver refuses `$field` cross-field comparison in the ADR-0 }); it('a null comparand is still a null predicate, not a refusal', async () => { - const rows = await find({ budget: { $ne: null } }); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids({ budget: { $ne: null } })).toEqual(['1']); }); it('an authored array triple with a scalar still matches, once lowered (#5158)', async () => { - const rows = await find(parseFilterAST([['amount', '>', 1]])); - expect(rows.map((r: any) => r.id)).toEqual(['1']); + expect(await ids(parseFilterAST([['amount', '>', 1]]))).toEqual(['1']); + }); + + it('a literal string comparand is never mistaken for a column reference', async () => { + // The inverse risk of this change: `{ stage: 'note' }` must compare + // against the TEXT 'note', not against the column of that name — the + // seeded row has stage 'won' and note 'won', so a compiler that resolved + // the literal as a column would match it. + expect(await ids({ stage: 'note' })).toEqual([]); + expect(await ids({ stage: { $eq: 'note' } })).toEqual([]); }); it('the malformed-$between refusal keeps its own descriptive message', async () => { diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 80541dc6ef..aba7843922 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -933,30 +933,158 @@ function fieldReferenceOf(value: unknown): string | null { } /** - * [#5041] `{ $field }` reached a comparison this driver compiles to SQL. + * [#5041→#5222] `{ $field }` reached a position this driver does NOT compile to + * a column-to-column comparison. * * `FieldReferenceSchema` is declared in the spec and really is PRODUCED — * `compileCelToFilter` emits `{ $field: path }` for a field-to-field comparison - * in a CEL permission/RLS rule — but the only implementation in the repo is the - * in-memory evaluator. Pushed down to SQL, the reference object was handed to - * Knex as a BIND VALUE, so sqlite answered with a bare `TypeError` ("can only - * bind numbers, strings, bigints, buffers, and null") carrying no `code` and no - * `status` — outside the ADR-0112 envelope every sibling filter refusal in this - * driver speaks, and therefore served as an opaque 500-shaped body. - * - * Refusing loudly is the whole fix here (maintainer adjudication on #5041): - * column-to-column compilation is a capability tracked separately, and until it - * lands the honest answer to "this filter cannot run on this backend" is the - * catalogued `INVALID_FILTER`, not a crash and not a silent wrong answer. + * in a CEL permission/RLS rule. #5041 refused the shape everywhere (before it, + * the reference object was handed to Knex as a BIND VALUE and sqlite answered + * with a bare `TypeError`); #5222 narrowed that gate: the reference now + * COMPILES when it is the whole comparand of one of the six scalar comparison + * operators ({@link CROSS_FIELD_COMPARISON_OPERATORS} — see + * {@link SqlDriver.applyCrossFieldComparison}). This error answers every + * position still outside that boundary, deliberately: + * + * - **`$in` / `$nin` / `$between` list MEMBERS.** The memory evaluator resolves + * a member per record and `looseEq`s it, which SQL's `IN (…)` bind list has + * no slot for — compiling it would need per-member OR-expansion whose NULL + * and type-affinity behaviour was not proven conformance-equivalent, and an + * unproven push-down of a permission shape is the #3948 class. Before #5041 + * this position did not even crash: it compiled and returned ZERO ROWS. + * - **The string operators (`$contains` / `$startsWith` / … / `like`).** In + * memory the referenced value is matched as a LITERAL substring; in SQL it + * would become the TEXT of a LIKE pattern, so any `%` / `_` / `\` stored in + * the referenced column would act as WILDCARDS — the filter-bypass class + * `applyLike` escapes literal comparands to prevent. Escaping a column-side + * value portably needs a dialect REPLACE chain nobody has proven; refused. + * - **Any other operator** (`$null`, a retired spelling, …): no cross-field + * reading exists to compile. */ function crossFieldComparisonError(field: string, op: string, ref: string, index?: number): Error { const position = index === undefined ? '' : ` at index ${index} of its value list`; return unsupportedFilterError( `Operator "${op}" on field "${field}" compares against another field ` + - `({ "$field": "${ref}" })${position}. Cross-field comparison is currently supported ` + - `only on the in-memory evaluation path (matchesFilter); it cannot be compiled to SQL, ` + - `so this filter cannot be pushed down to the database. Compare against a literal value ` + - `instead, or evaluate the rule in memory.`, + `({ "$field": "${ref}" })${position}, a position SQL push-down does not compile. ` + + `Cross-field comparison compiles only as the whole comparand of a scalar comparison ` + + `operator ($eq/$ne/$gt/$gte/$lt/$lte) between two same-table declared columns. ` + + `Compare against a literal value here, or evaluate the rule in memory (matchesFilter).`, + ); +} + +/** + * [#5222] `{ field: { $field: 'other' } }` — a field reference used as a BARE + * 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. + */ +function bareFieldReferenceError(field: string, ref: string): Error { + return unsupportedFilterError( + `Field "${field}" is constrained by a bare field reference ({ "$field": "${ref}" }) with no ` + + `operator. Write the comparison explicitly — { "${field}": { "$eq": { "$field": "${ref}" } } } ` + + `— which compiles to a column-to-column comparison. The bare form is refused because the ` + + `in-memory evaluator does not read it as an equality (it matches no record), so compiling ` + + `it here would make the two execution paths answer this filter differently.`, + ); +} + +/** + * [#5222] The operators whose `{ $field }` comparand compiles to a + * column-to-column comparison — the six scalar comparisons, in the `$`-form + * spelling the emitters read (array-triple / infix authorings are lowered to + * these before a driver ever runs; see `parseFilterAST` and #5158). + * + * Deliberately NOT the whole of {@link SCALAR_COMPARAND_OPERATORS}: `like` / + * `ilike` are in that set only for the bind gate, and a cross-field LIKE is + * refused (see {@link crossFieldComparisonError}). + */ +const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet = new Set([ + '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', +]); + +/** + * [#5222] The comparison class a declared field's stored column belongs to, or + * `null` for a field no column-to-column comparison can be compiled against. + * + * Cross-field comparison is only emitted between two columns of the SAME + * class. One class = one storage shape on both sides of one row, which is what + * makes the SQL answer provably the memory evaluator's answer (the cross-path + * conformance suite pins it). + * + * **The divergence is measured, not assumed — and it is DIRECTIONAL.** With + * this check disabled, `{ stage: { $gt: { $field: 'amount' } } }` (TEXT target, + * numeric referent) returns four rows on SQLite and NONE in memory: SQLite + * orders by STORAGE CLASS first, so every TEXT sorts above every INTEGER, + * while JS relational operators coerce `'won' > 10` to a NaN comparison and + * answer false. The mirrored spelling `{ amount: { $gt: { $field: 'stage' } } }` + * happens to agree (both answer nothing), and so does date-against-text — both + * are TEXT on both paths, so they agree while answering by lexicographic + * accident rather than by any temporal reading. + * + * The rule stays symmetric and covers the agreeing cells anyway, deliberately: + * a guard that admitted exactly the pairings measured non-divergent on ONE + * fixture would be a rule about this data rather than about the types, and the + * agreeing cells are agreeing by coincidence of storage — a `boolean` column + * holds `0/1` where the record holds `true/false`, and the three temporal + * classes store three different text shapes (`YYYY-MM-DD` / canonical UTC ISO / + * `HH:MM:SS`). Refusing a shape that would have agreed costs a caller an error + * message; admitting one that diverges costs a permission rule its meaning. + * + * `null` — refused outright — for: `formula` (virtual, no column to + * reference), every JSON-stored shape (`multiple: true` and the + * {@link JSON_COLUMN_TYPES} classes — element-wise semantics SQL comparison + * operators do not have), and anything else without a scalar stored form. + */ +function crossFieldComparisonClass( + decl: Record, +): 'numeric' | 'text' | 'boolean' | 'date' | 'datetime' | 'time' | null { + if (decl.multiple) return null; + const type = String((decl as { type?: unknown }).type || 'string'); + if (type === 'formula') return null; + if (JSON_COLUMN_TYPES.has(type)) return null; + if (NUMERIC_SCALAR_TYPES.has(type)) return 'numeric'; + if (type === 'boolean' || type === 'toggle') return 'boolean'; + if (type === 'date') return 'date'; + if (type === 'datetime') return 'datetime'; + if (type === 'time') return 'time'; + // Everything else `createColumn` stores as TEXT: string/text/textarea/html/ + // markdown/email/url/phone/password, select, lookup/user (row ids), + // autonumber, and the unknown-type default. + return 'text'; +} + +/** + * [#5222] A `{ $field }` reference at a compilable operator that fails the + * v1 validation boundary — the maintainer's 2026-08-06 rulings on #5222, + * verbatim: same-table columns only (dot paths refused — no JOIN planning, no + * alias contract), declared-only enumeration (unknown columns refused at + * compile time; external/federated tables, whose column sets this driver does + * not own, refused wholesale), and the tenant-isolation column FORBIDDEN on + * either side of the comparison. `reason` carries the specific sentence; + * this wrapper keeps the envelope and the shared contract statement. + */ +function uncompilableFieldReferenceError(field: string, op: string, ref: string, reason: string): Error { + return unsupportedFilterError( + `Operator "${op}" on field "${field}" compares against another field ` + + `({ "$field": "${ref}" }), which cannot be compiled here: ${reason} ` + + `Cross-field comparison on SQL push-down supports same-table columns the object ` + + `declares, compared as the same type class, excluding the tenant-isolation column.`, ); } @@ -2089,6 +2217,15 @@ function nullValueSatisfiesOperator(op: string, value: unknown): boolean { /** [#5146] Is this operator's compiled SQL already total for a NULL column? */ function operatorIsNullTotal(op: string, value: unknown): boolean { + // [#5222] A `{ $field }` comparand on a scalar comparison compiles to a + // TOTAL column-to-column predicate — `applyCrossFieldComparison` writes both + // columns' nullness INTO the emitted SQL, so it is never UNKNOWN and `NOT` + // over it is the exact complement. It must not fall to the literal arms + // below: their 'requireValue' guard assumes a NULL target column FAILS the + // operator, which is false for `$eq: { $field }` (a both-NULL row MATCHES, + // the memory evaluator's answer), so the guard would flip `$not` on exactly + // the rows the null pins in the conformance suite exist to protect. + if (CROSS_FIELD_COMPARISON_OPERATORS.has(op) && fieldReferenceOf(value) !== null) return true; switch (op) { // Compile to `IS NULL` / `IS NOT NULL` — two-valued by construction. case '$null': @@ -8523,6 +8660,25 @@ export class SqlDriver implements IDataDriver { const columnExpr = this.filterColumnExpr(table, localField, field); for (const [rawOp, opValue] of Object.entries(value as Record)) { const method = logicalOp === 'or' ? 'orWhere' : 'where'; + // #5222 — a `{ $field }` that is the WHOLE comparand of a scalar + // comparison compiles to a same-table column-to-column comparison + // (or refuses inside the emitter, with the specific boundary named). + // Checked before the #5041 gate below, which still answers every + // other `$field` position — list members, the LIKE family — and + // before coercion/rewrites, which expect a literal comparand. + // #5222 — the bare `{ field: { $field: 'other' } }` spelling reaches + // this loop as an OPERATOR named `$field`, because the whole field + // spec is its own operator map. Answered here, ahead of the generic + // unsupported-operator arm, so the message can name the supported + // spelling instead of listing fifteen operator names. + if (rawOp === '$field' && typeof opValue === 'string') { + throw bareFieldReferenceError(field, opValue); + } + const crossFieldRef = fieldReferenceOf(opValue); + if (crossFieldRef !== null && CROSS_FIELD_COMPARISON_OPERATORS.has(rawOp)) { + this.applyCrossFieldComparison(builder, method, table, key, localField, field, rawOp, crossFieldRef); + continue; + } // #5041 — reject a comparand that cannot become a bind parameter // BEFORE any rewrite or coercion touches it, so the message names the // shape the caller actually sent. @@ -8693,6 +8849,185 @@ export class SqlDriver implements IDataDriver { } } + /** + * [#5222] Declared metadata fields for `object`, under either key the two + * registration paths use (`initObjects` keys by resolved TABLE name, + * `registerExternalObject` by object name — same double lookup as + * {@link paginationTieBreaker} / {@link resolveTenantField}). `undefined` + * means this driver does not OWN the object's column set: never registered, + * or federated/external (ADR-0015 — `registerExternalObject` deliberately + * does not populate {@link managedObjectFields}), and cross-field comparison + * refuses rather than guessing at columns it cannot enumerate. + */ + protected declaredFieldsFor(object: string): Record | undefined { + const tableName = StorageNameMapping.resolveTableName({ name: object } as any); + return this.managedObjectFields.get(tableName) ?? this.managedObjectFields.get(object); + } + + /** + * [#5222] Compile `{ : { : { $field: } } }` into a + * SAME-TABLE column-to-column comparison — the capability #5041 catalogued + * and deliberately deferred — or refuse in the same ADR-0112 envelope + * (`INVALID_FILTER`, 400) it installed. + * + * # The validation boundary (maintainer rulings, 2026-08-06, on #5222) + * + * 1. **Same-table columns only.** A dotted `$field` is refused: in memory + * `getPath` walks a dot path across related objects, and SQL has no + * equivalent short of JOIN planning (rejected as disproportionate) or an + * alias contract (rejected as nonexistent). + * 2. **Declared-only enumeration, on BOTH operands.** The `$field` value + * lands in a SQL IDENTIFIER position; only names the object declared + * ({@link managedObjectFields}) are accepted, so an unknown column is a + * compile-time refusal, not a database error — and a table this driver + * has no declaration for (external/federated, ADR-0015) refuses + * wholesale. The TARGET field must be declared too: the type-class check + * below needs both declarations, and a comparison is one surface — it + * cannot be half-validated. + * 3. **The tenant-isolation column is forbidden, on either side.** The + * ruling names the referent; both sides are closed because the operands + * of `=` commute — `{ org_id: { $eq: { $field: x } } }` is the same + * privilege-escalation comparison surface as + * `{ x: { $eq: { $field: org_id } } }` spelled backwards, and a ban only + * one swap away is not a ban. + * 4. **Same comparison class** ({@link crossFieldComparisonClass}) — the + * conformance boundary: across classes SQLite's storage-class ordering + * and the memory evaluator's JS coercion genuinely diverge, so those + * shapes go to the refusal arm rather than shipping a per-backend answer. + * + * # The emitted SQL is TOTAL — never UNKNOWN — by construction + * + * Both columns can be NULL at once, and the memory evaluator answers every + * such row two-valuedly (`matches-filter.ts` `evalOp`): the orderings need + * both sides non-null; `$eq` with a null referent matches exactly the + * null-target rows; `$ne` is `$eq`'s complement. Each arm below writes that + * truth table into the predicate itself (`IS [NOT] NULL` conjuncts), which + * buys the same property #5146 buys leaf-by-leaf for `$not`: a total + * predicate's `NOT` is its exact complement, so the negation rewrite needs + * no guard here ({@link operatorIsNullTotal}'s cross-field arm) and every + * combinator nesting composes. The cross-path conformance suite pins the + * NULL rows on every operator, both polarities. + * + * Each side reads through {@link filterColumnExpr} when the column needs the + * legacy-datetime/time storage repair (#3912/#3994), the same normalisation + * every VALUE comparison applies — `??` identifier binding otherwise, so + * quoting stays Knex's on every dialect. + */ + protected applyCrossFieldComparison( + builder: Knex.QueryBuilder, + method: 'where' | 'orWhere', + table: string | null, + targetKey: string, + targetLocalField: string, + targetColumn: string, + op: string, + ref: string, + ): void { + if (ref.includes('.')) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `"${ref}" is a dotted path, and SQL push-down compiles same-table column references ` + + `only (no relation traversal, no alias-qualified columns).`); + } + if (!table) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `the target table of this query could not be resolved, so the reference cannot be ` + + `checked against any declared column set.`); + } + const declared = this.declaredFieldsFor(table); + if (!declared) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `object "${table}" has no declared column set on this driver (an external/federated ` + + `or unregistered table), so referenced column names cannot be validated.`); + } + const tenantField = this.resolveTenantField(table); + if (tenantField !== null && (ref === tenantField || targetKey === tenantField || targetLocalField === tenantField)) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `"${tenantField}" is the tenant-isolation column of "${table}", which must not appear ` + + `on either side of a cross-field comparison.`); + } + const hasOwn = (name: string) => Object.prototype.hasOwnProperty.call(declared, name); + const refDeclaredName = hasOwn(ref) ? ref : this.mapSortField(ref); + if (!hasOwn(refDeclaredName)) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `"${ref}" is not a declared field of "${table}" — only declared fields can be referenced.`); + } + const targetDeclaredName = hasOwn(targetKey) ? targetKey : targetLocalField; + if (!hasOwn(targetDeclaredName)) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `the target field "${targetKey}" is not a declared field of "${table}", so the two ` + + `columns' types cannot be checked as comparable.`); + } + const refClass = crossFieldComparisonClass(declared[refDeclaredName] ?? {}); + const targetClass = crossFieldComparisonClass(declared[targetDeclaredName] ?? {}); + if (refClass === null) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `"${ref}" (type "${String(declared[refDeclaredName]?.type ?? 'string')}"` + + `${declared[refDeclaredName]?.multiple ? ', multiple' : ''}) has no scalar stored ` + + `column a comparison can read.`); + } + if (targetClass === null) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `the target field "${targetKey}" (type ` + + `"${String(declared[targetDeclaredName]?.type ?? 'string')}"` + + `${declared[targetDeclaredName]?.multiple ? ', multiple' : ''}) has no scalar stored ` + + `column a comparison can read.`); + } + if (refClass !== targetClass) { + throw uncompilableFieldReferenceError(targetColumn, op, ref, + `"${targetKey}" is stored as ${targetClass} but "${ref}" as ${refClass}, and a ` + + `cross-class comparison answers differently in SQL (storage-class ordering) than in ` + + `memory (JS coercion) — compare same-class columns.`); + } + + const refLocal = this.mapSortField(ref); + const refColumn = this.remoteColumn(table, ref, refLocal); + const lhs = this.filterColumnExpr(table, targetLocalField, targetColumn) + ?? { sql: '??', bindings: [targetColumn] }; + const rhs = this.filterColumnExpr(table, refLocal, refColumn) + ?? { sql: '??', bindings: [refColumn] }; + const raw = method === 'orWhere' ? 'orWhereRaw' : 'whereRaw'; + const A = lhs.sql; + const B = rhs.sql; + const ab = [...lhs.bindings, ...rhs.bindings]; + switch (op) { + case '$eq': + // Matches when both are NULL, or both have a value and the values + // agree — `evalOp`'s `$eq` over a resolved reference, made total. + (builder as any)[raw]( + `((${A} is null and ${B} is null) or (${A} is not null and ${B} is not null and ${A} = ${B}))`, + [...ab, ...ab, ...ab], + ); + break; + case '$ne': + // The exact complement of the `$eq` arm: exactly one side NULL, or + // both valued and different. + (builder as any)[raw]( + `((${A} is null and ${B} is not null) or (${A} is not null and ${B} is null) ` + + `or (${A} is not null and ${B} is not null and ${A} <> ${B}))`, + [...ab, ...ab, ...ab, ...ab], + ); + break; + case '$gt': + case '$gte': + case '$lt': + case '$lte': { + // The orderings need both sides non-null (`evalOp`: `actual != null && + // v != null && …`). A bare `A > B` already DROPS null rows via + // UNKNOWN; the explicit conjuncts are what make the predicate total, + // so `$not` of it re-admits exactly those rows — the memory answer. + const sqlOp = op === '$gt' ? '>' : op === '$gte' ? '>=' : op === '$lt' ? '<' : '<='; + (builder as any)[raw]( + `(${A} is not null and ${B} is not null and ${A} ${sqlOp} ${B})`, + [...ab, ...ab], + ); + break; + } + default: + // Unreachable: the call site gates on CROSS_FIELD_COMPARISON_OPERATORS. + throw crossFieldComparisonError(targetColumn, op, ref); + } + } + /** * [#5134] Emit the dialect FALSE constant — a predicate that matches no row. * diff --git a/packages/drivers/driver-sqlite-wasm/package.json b/packages/drivers/driver-sqlite-wasm/package.json index 5b2fc84bf2..c236e8dd38 100644 --- a/packages/drivers/driver-sqlite-wasm/package.json +++ b/packages/drivers/driver-sqlite-wasm/package.json @@ -37,6 +37,7 @@ "sql.js": "^1.14.1" }, "devDependencies": { + "@objectstack/formula": "workspace:*", "@types/node": "^26.1.2", "@types/sql.js": "^1.4.11", "typescript": "^6.0.3", 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 new file mode 100644 index 0000000000..4ffb537146 --- /dev/null +++ b/packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-cross-field-conformance.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5222] Cross-field `{ $field }` push-down conformance for the wasm driver — + * the same corpus `driver-sql` runs, through this driver's own pipeline. + * + * `SqliteWasmDriver extends SqlDriver`, so `applyCrossFieldComparison` and the + * validation gate around it are INHERITED and nothing here re-implements them. + * What this pins is the other half, the same half this driver's temporal, + * pagination and filter-logic suites pin: the compiled predicate has to + * survive a different **engine**. This driver swaps knex's transport for a + * custom sql.js dialect (`Client_WasmSqlite`) that compiles the statement, + * binds its parameters and marshals the rows back through its own path. + * + * That matters more here than for an ordinary value comparison, and it is why + * this file exists rather than a comment asserting inheritance. A cross-field + * predicate is emitted through `whereRaw` with **identifier** bindings (`??`) + * and repeats each column expression several times to stay total across NULLs + * — a dialect that mis-ordered or mis-escaped that binding list would produce + * exactly the failure the capability exists to rule out: a filter that looks + * applied and selects the wrong rows. "It inherits the compiler, therefore it + * is fine" is the assumption those sibling suites exist to disprove. + * + * The corpus is imported from `@objectstack/driver-sql` — the package this one + * already depends on — so the two drivers cannot drift into fixtures that + * differ in a way that matters. See its header for why it does not live in + * `packages/spec/src/data`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { matchesFilterCondition } from '@objectstack/formula'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { + CROSS_FIELD_CASES, + CROSS_FIELD_OBJECT_FIELDS, + CROSS_FIELD_REFUSALS, + CROSS_FIELD_ROWS, +} from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from './index.js'; + +describe('[#5222] driver-sqlite-wasm — cross-field `$field` push-down conformance', () => { + let driver: SqliteWasmDriver; + let records: Array>; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([ + { name: 'cross_field_deal', fields: CROSS_FIELD_OBJECT_FIELDS } as any, + ]); + for (const row of CROSS_FIELD_ROWS) await driver.create('cross_field_deal', { ...row }); + records = (await driver.find('cross_field_deal', {})) as Array>; + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + it('the fixture round-tripped with its NULLs intact', () => { + // The control every null case depends on — and it is a real risk on THIS + // driver rather than a copied assertion: the wasm dialect marshals values + // back through its own row mapper, so a NULL that returned as `undefined` + // or `''` here would quietly turn the null cases green. + expect(records).toHaveLength(CROSS_FIELD_ROWS.length); + const byId = new Map(records.map((r) => [r.id as string, r])); + expect(byId.get('6')!.amount).toBeNull(); + expect(byId.get('6')!.stage).toBeNull(); + expect(byId.get('4')!.amount).toBeNull(); + expect(byId.get('4')!.budget).toBe(5); + expect(byId.get('5')!.budget).toBeNull(); + }); + + const sqlIds = async (filter: unknown): Promise => { + const rows = await driver.find('cross_field_deal', { + fields: ['id'], + where: filter as FilterCondition, + }); + return rows.map((r: any) => String(r.id)).sort(); + }; + + const memoryIds = (filter: unknown): string[] => + records + .filter((r) => matchesFilterCondition(r, filter as FilterCondition)) + .map((r) => String(r.id)) + .sort(); + + for (const testCase of CROSS_FIELD_CASES) { + it(`${testCase.name} — same rows on both paths`, async () => { + const expected = [...testCase.expected].sort(); + const note = testCase.note ? `\n${testCase.note}` : ''; + expect(memoryIds(testCase.filter), `in-memory evaluator disagreed${note}`).toEqual(expected); + expect(await sqlIds(testCase.filter), `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 () => { + let error: (Error & { code?: string; status?: number }) | null = null; + try { + await sqlIds(refusal.filter); + } catch (e) { + error = e as Error & { code?: string; status?: number }; + } + expect(error, `expected a refusal${refusal.note ? `\n${refusal.note}` : ""}`).not.toBeNull(); + expect(error!.code).toBe('INVALID_FILTER'); + expect(error!.status).toBe(400); + expect(error!).not.toBeInstanceOf(TypeError); + expect(error!.message).not.toContain('can only bind'); + expect(error!.message).not.toContain('[sql-driver]'); + for (const fragment of refusal.messageIncludes) { + expect(error!.message).toContain(fragment); + } + }); + } + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 90a4398409..881145c90b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -929,6 +929,9 @@ importers: specifier: ^18.0.0 version: 18.6.2 devDependencies: + '@objectstack/formula': + specifier: workspace:* + version: link:../../formula '@types/node': specifier: ^26.1.2 version: 26.1.2 @@ -964,6 +967,9 @@ importers: specifier: ^1.14.1 version: 1.14.1 devDependencies: + '@objectstack/formula': + specifier: workspace:* + version: link:../../formula '@types/node': specifier: ^26.1.2 version: 26.1.2