From 46ed8c986edb8060f16a488917e1aa5efe58b504 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:11:43 +0000 Subject: [PATCH 1/3] feat(service-analytics): serve a `$field` RLS rule by declining native SQL (#7598) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the maintainer ruling of 2026-08-12 (Q1 = B). `NativeSQLStrategy.canHandle` now declines a query whose `where` or read scope carries a `{ $field }` reference in a scalar comparand position, so the query routes to the ObjectQL/engine path and `driver-sql` compiles the comparison under the four #5222 rulings with the metadata it owns. The capability becomes available on the analytics face and the security rules stay in exactly one place — no `StrategyContext` hook, no `packages/spec` change, no second copy of the rulings here. The ruling site carries the ruling as a comment, as it required. Refusal arms, after the routing: - `filter-normalizer`'s scalar arm (#7694's interim) is REMOVED. It sat in `fieldLeaves`, the one leaf producer for all three consumers of the tree — including the engine path — so it refused the execution the routing exists to reach. Reverse-verified: restoring it turns both suites red (75 cells). - its `$between`-endpoint arm STAYS, and is now load-bearing rather than inherited: this door splits `$between` into `gte` / `lte`, so a routed endpoint reference would reach the driver under an operator #5222 compiles and succeed here alone. It gets its own wording. - `read-scope-sql`'s arm STAYS, whole. Its remaining caller is `ObjectQLStrategy.generateSql` — the `/analytics/sql` echo, which also declines a `$field`-carrying `where` rather than half-rendering it. - the envelope is untouched: `READ_SCOPE_COMPILE_FAILED` / 500 with the message withheld, per #5367 as re-affirmed by Q2 = A. Two fixes the routing needed, both found by measurement: - `ObjectQLStrategy.convertFilter` lowered an equality comparand bare, producing `{ amount: { $field: … } }` — a field spec no backend reads as an equality (#7597's defect, other door). It now branches on the comparand. - a reference comparand no longer takes the NULL-safe `$ne` guard (#5298), which is right for a literal and wrong for a reference: measured, it admitted the both-NULL row that the shared corpus, both SQL drivers and the in-memory evaluator exclude. Tests: the shared `CROSS_FIELD_*` corpus driven end-to-end through `AnalyticsService` against a real SQLite engine — right rows for a `$field` `where` AND a `$field` read scope, the four rulings still biting on the fallback path, and the echo declining. Closes #7598 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EWcRLiMFvDoQV3zS2LgEHH --- .../analytics-cross-field-engine-decline.md | 71 +++ .../services/service-analytics/package.json | 1 + .../cross-field-engine-fallback.test.ts | 420 ++++++++++++++++++ .../cross-field-reference-refusal.test.ts | 387 ++++++++++------ .../service-analytics/src/comparand-shape.ts | 227 ++++++++-- .../service-analytics/src/read-scope-sql.ts | 82 +++- .../src/strategies/filter-normalizer.ts | 176 +++++--- .../src/strategies/native-sql-strategy.ts | 144 ++++++ .../src/strategies/objectql-strategy.ts | 87 +++- pnpm-lock.yaml | 3 + 10 files changed, 1313 insertions(+), 285 deletions(-) create mode 100644 .changeset/analytics-cross-field-engine-decline.md create mode 100644 packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts diff --git a/.changeset/analytics-cross-field-engine-decline.md b/.changeset/analytics-cross-field-engine-decline.md new file mode 100644 index 0000000000..5581f34418 --- /dev/null +++ b/.changeset/analytics-cross-field-engine-decline.md @@ -0,0 +1,71 @@ +--- +"@objectstack/service-analytics": minor +--- + +feat(service-analytics): a field-to-field (`$field`) RLS rule is served on the analytics path — native SQL declines and routes to the engine (#7598) + +A CEL permission / RLS rule that compares two columns of the same record — +`compileCelToFilter` lowers it to `{ amount: { $gt: { $field: 'budget' } } }` — +now **works** on `/analytics/query`, whether it arrives in the caller's `where` +or in the read scope the platform compiles from an admin-authored sharing rule. + +Before #7694 the two analytics SQL compilers **bound the reference object as the +comparison's value**: the statement compiled perfectly and compared a column +against the text `{"$field":"budget"}`, which no row can hold — an empty chart, +or an RLS predicate quietly answering the wrong row set, with nothing to read. +#7694 stopped that by refusing the shape. This change replaces the refusal with +the answer. + +**How.** `NativeSQLStrategy.canHandle` declines a query whose `where` or read +scope carries a reference in a scalar comparand position, so the query falls +through to the lower-priority ObjectQL/engine path — the same decline-and-route +mechanism this strategy already uses for federated objects (ADR-0062 D6) and for +date-bucketed queries. `driver-sql` then compiles the comparison and enforces the +four #5222 security rulings — same-table columns only, declared-only enumeration, +the tenant-isolation column forbidden on both sides, and a matching comparison +class — using the `initObjects` metadata it owns. Those rules stay in exactly one +place; the alternative considered was a `StrategyContext` enumeration hook plus a +second implementation of them inside this package, and a guard that exists twice +is a guard that will eventually disagree with itself. + +⚠️ **Query routing now depends on filter CONTENT, not only on query shape.** That +is new behaviour for `canHandle`, and it is deliberate: a query carrying a +cross-field comparison takes the engine path rather than raw SQL, so it is served +by `engine.aggregate` and is slower than a pushed-down statement. Every other +query is unaffected — a literal comparand, a literal read scope and a filterless +query all keep the native-SQL path exactly as before. + +**Two positions deliberately still refuse**, and both converge with what +`driver-sql` itself refuses rather than diverging from it: + +- `/analytics/sql` — the display echo declines a cross-field comparison instead + of half-rendering one. It describes an execution it does not perform, and the + predicate the engine path actually runs is written total across NULLs; what + this renderer can emit is a comparison against the reference as a bound value, + which reproduces none of the rows the query returns. `/analytics/query` still + serves those queries and returns rows — the response simply carries no `sql` + string. +- a `$field` in a `$between` **endpoint**. No backend serves it (`@objectstack/spec` + removed the position in #7596), and this compiler splits `$between` into its two + bounds — so routing it would hand the driver a `$gte` / `$lte` the author never + wrote, and the range would quietly succeed here while the identical filter is + refused everywhere else. The refusal message now names that, and points at the + scalar spelling which *is* served. + +The LIKE family and `$in` / `$nin` members keep their existing refusals and +wordings, unchanged. + +**Read-scope error envelope: unchanged.** An unsupported rule on the read-scope +lowering still answers `READ_SCOPE_COMPILE_FAILED` / 500 with the message +withheld, exactly as the #5367 ruling set it — no new error code, no move to a +4xx. A read scope is not the caller's document, so it is not the caller's 4xx. + +One further fix this needed, in the same class as #7597: `ObjectQLStrategy` +lowered an equality comparand **bare** (`{ amount: 5 }` — correct for a literal), +which for a reference produced `{ amount: { $field: 'budget' } }`, a field spec no +backend reads as an equality. It now emits an explicit `$eq` when the comparand is +a reference, branching on the comparand rather than on the operator. And a +reference comparand no longer takes this door's NULL-safe `$ne` guard (#5298), +which is right for a literal and wrong for a reference — measured, it admitted the +both-NULL row that the shared corpus, both SQL drivers and the in-memory evaluator +all exclude. diff --git a/packages/services/service-analytics/package.json b/packages/services/service-analytics/package.json index e76cf844b1..1c50048b2e 100644 --- a/packages/services/service-analytics/package.json +++ b/packages/services/service-analytics/package.json @@ -24,6 +24,7 @@ }, "devDependencies": { "@objectstack/driver-sql": "workspace:*", + "@objectstack/driver-sqlite-wasm": "workspace:*", "@types/node": "^26.1.2", "@types/sql.js": "^1.4.11", "sql.js": "^1.14.1", diff --git a/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts new file mode 100644 index 0000000000..8faeaf05b7 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts @@ -0,0 +1,420 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7598, maintainer ruling 2026-08-12 Q1 = B] A cross-field `{ $field }` + * comparison is SERVED on the analytics face — by `NativeSQLStrategy.canHandle` + * declining it, so the query routes to the ObjectQL/engine path where the driver + * compiles it. + * + * ## What this file has to prove, and why "canHandle returned false" is not it + * + * The card is explicit that a routing assertion is not the deliverable: "a test + * proving a `$field`-carrying `where` AND a `$field`-carrying read scope each + * route to the engine path and RETURN THE RIGHT ROWS — not merely that + * `canHandle` returned false. The capability working end-to-end is the point." + * + * A decline that routes to a path which then refuses, or mis-answers, or drops + * the predicate, would satisfy every plausible routing test and none of the + * card. So this suite runs the whole road with a REAL SQL engine at the end of + * it — `SqliteWasmDriver` (`driver-sqlite-wasm`, one of the two drivers #5222 + * taught to compile the comparison) seeded with the shared corpus fixture — and + * holds every answer to the corpus's own declared id lists. + * + * Three independent statements of the semantics have to agree for a case to + * pass, which is what makes agreement worth something (the discipline + * `sql-driver-cross-field-conformance.test.ts` states): the corpus's `expected`, + * the analytics face's rows, and — since these are the same filters — the driver + * suites that already run them one package away. + * + * ## The setup is the experiment + * + * `queryCapabilities` declares **both** `nativeSql` and `objectqlAggregate`, and + * `executeRawSql` is supplied. That is deliberate and load-bearing: with those + * three facts, `NativeSQLStrategy` (priority 10) wins `resolveStrategy` for + * every query unless it declines. So `executeRawSql` never being called is a + * measurement of the decline rather than of a missing capability, and the + * literal-comparand controls — which DO reach it — are what stop that + * measurement from being vacuous. + * + * ## Why a `where` arm AND a read-scope arm + * + * They are different producers with different authors, and only the second is + * the one #5041 measured. A `where` is written by the caller; a read scope is + * compiled by the platform from an ADMIN-authored CEL sharing rule, which is + * exactly what `compileCelToFilter` emits `{ $field: path }` for. They also + * travel different code: the `where` goes through `filter-normalizer` → + * `convertFilter`, the scope through `ObjectQLStrategy.withReadScope`, which + * ANDs the raw `FilterCondition` into what `engine.aggregate` receives. A test + * of one says nothing about the other. + * + * ## And the refusal arm, on the SAME road + * + * Routing is only defensible while the four #5222 rulings still bite after it — + * same-table columns only, declared-only enumeration, the tenant-isolation + * column forbidden on BOTH sides, same comparison class. The whole of + * `CROSS_FIELD_REFUSALS` is driven through the analytics face below and every + * case is asserted to be refused with an ADR-0112 envelope. What differs is + * WHICH component refuses, and the suite asserts that split rather than + * flattening it — see the block's own comment. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { + CROSS_FIELD_CASES, + CROSS_FIELD_OBJECT_FIELDS, + CROSS_FIELD_REFUSALS, + CROSS_FIELD_ROWS, +} from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery } from '@objectstack/spec/contracts'; + +import { AnalyticsService } from '../analytics-service.js'; +import { findCrossFieldComparand } from '../comparand-shape.js'; + +const OBJECT = 'cross_field_deal'; + +/** Every column of the corpus fixture, as a plain cube dimension. */ +const CUBE: Cube = { + name: 'deals', + sql: OBJECT, + measures: { n: { sql: '*', type: 'count', title: 'n' } }, + dimensions: Object.fromEntries( + ['id', 'amount', 'budget', 'stage', 'owner', 'starts_on', 'ends_on', 'organization_id'].map( + (n) => [n, { name: n, label: n, type: 'string', sql: n }], + ), + ), + public: false, +} as unknown as Cube; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#7598] cross-field `$field` on the analytics face — served via the engine fallback', () => { + let driver: SqliteWasmDriver; + let service: AnalyticsService; + /** Every `executeRawSql` call the run made — the decline's measurement. */ + let rawSqlCalls: string[]; + /** The read scope `getReadScope` answers with, swapped per test. */ + let readScope: FilterCondition | null; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.initObjects([{ name: OBJECT, fields: CROSS_FIELD_OBJECT_FIELDS } as any]); + for (const row of CROSS_FIELD_ROWS) await driver.create(OBJECT, { ...row }); + + rawSqlCalls = []; + readScope = null; + service = new AnalyticsService({ + cubes: [CUBE], + // BOTH paths available — see the header. Native SQL wins unless it declines. + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }), + executeRawSql: async (_object, sql) => { + rawSqlCalls.push(sql); + return []; + }, + // The production bridge is `engine.aggregate`; here it is the driver's own + // `aggregate`, which is what that engine call reaches. The filter is + // forwarded VERBATIM — no normalisation, no stringification — because the + // claim under test is that the reference survives this hop intact. + executeAggregate: async (objectName, options) => + (await driver.aggregate(objectName, { + where: options.filter as FilterCondition, + groupBy: options.groupBy, + // `{field, method, alias}` → `{field, function, alias}`: the analytics + // strategy speaks the contract's `method`, the Query Protocol's + // `AggregationNodeSchema` spells it `function`, and `engine.aggregate` + // is what renames it in production. Mapped here rather than worked + // around, so the bridge stays the shape a real host writes. + aggregations: options.aggregations?.map(({ field, method, alias }) => ({ + field, + function: method, + alias, + })), + } as any)) as Record[], + getReadScope: () => readScope ?? undefined, + }); + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + /** Run one analytics query and return the matching ids, ascending. */ + const idsFor = async (query: Partial): Promise => { + const result = await service.query({ + cube: 'deals', + dimensions: ['id'], + measures: ['n'], + ...query, + } as AnalyticsQuery); + return result.rows.map((r) => String(r.id)).sort(); + }; + + const errorFrom = async (run: () => Promise): Promise => { + let returned: unknown; + try { + returned = await run(); + } catch (e) { + return e as WireBearingError; + } + throw new Error( + `expected the analytics face to refuse this query, but it returned ${JSON.stringify(returned)}`, + ); + }; + + it('the fixture round-tripped with its NULLs intact', async () => { + // The control every null case depends on. Asserted through THIS driver + // rather than assumed from the corpus: rows 4-6 are the cells that decide + // `$eq` / `$ne`, and a NULL that came back as `''` would turn them green + // for the wrong reason. + const rows = (await driver.find(OBJECT, {})) as Array>; + expect(rows).toHaveLength(CROSS_FIELD_ROWS.length); + const byId = new Map(rows.map((r) => [String(r.id), r])); + expect(byId.get('6')!.amount).toBeNull(); + expect(byId.get('6')!.budget).toBeNull(); + expect(byId.get('4')!.amount).toBeNull(); + expect(byId.get('5')!.budget).toBeNull(); + }); + + // ── The capability, through the caller's `where` ──────────────────────────── + + describe('a `$field`-carrying `where` routes to the engine and returns the right rows', () => { + for (const testCase of CROSS_FIELD_CASES) { + it(`${testCase.name} — the corpus's own row set`, async () => { + rawSqlCalls = []; + readScope = null; + const note = testCase.note ? `\n${testCase.note}` : ''; + expect(await idsFor({ where: testCase.filter as FilterCondition }), `wrong rows${note}`) + .toEqual([...testCase.expected].sort()); + // The decline is what put the query here. If native SQL had taken it, + // the rows above would have come from a spy that returns nothing — so + // this assertion and the one above are each other's control. + expect(rawSqlCalls, 'NativeSQLStrategy did not decline').toEqual([]); + }); + } + }); + + // ── The capability, through an RLS read scope ────────────────────────────── + + describe('a `$field`-carrying READ SCOPE routes to the engine and returns the right rows', () => { + // The producer #5041 actually measured: `compileCelToFilter` emits this + // shape for a field-to-field comparison in an admin-authored CEL rule, and + // the scope reaches the engine down a different road than the `where` + // (`withReadScope` ANDs the raw FilterCondition in, rather than going + // through `convertFilter`). Same corpus, same expectations, other door. + for (const testCase of CROSS_FIELD_CASES) { + it(`${testCase.name} — the corpus's own row set, as a read scope`, async () => { + rawSqlCalls = []; + readScope = testCase.filter as FilterCondition; + const note = testCase.note ? `\n${testCase.note}` : ''; + expect(await idsFor({}), `wrong rows${note}`).toEqual([...testCase.expected].sort()); + expect(rawSqlCalls, 'NativeSQLStrategy did not decline').toEqual([]); + readScope = null; + }); + } + + it('a read scope INTERSECTS the caller`s own where rather than replacing it', async () => { + // `withReadScope` composes with `$and`, never by key merge — the property + // that stops caller input overwriting a security predicate. Worth pinning + // on THIS path because both operands now carry references: the scope is + // `amount >= budget` (rows 1, 3) and the where is `stage = 'won'` (rows 1, + // 5), so a composition failure shows as the wrong intersection rather than + // as an error. + rawSqlCalls = []; + readScope = { amount: { $gte: { $field: 'budget' } } } as FilterCondition; + expect(await idsFor({ where: { stage: 'won' } as FilterCondition })).toEqual(['1']); + expect(rawSqlCalls).toEqual([]); + readScope = null; + }); + }); + + // ── The four #5222 rulings still bite, one package over ──────────────────── + + describe('the #5222 refusal arm still bites after the routing', () => { + /** + * Every corpus refusal is refused on the analytics face — that is the + * invariant. WHICH component refuses is what the routing changed, and the + * split is asserted rather than flattened, because flattening it would let a + * blanket refusal pass as this suite's green. + * + * - A refusal whose reference sits in a SCALAR comparand position is + * routed by `canHandle`, so `driver-sql`'s validation gate answers it — + * the four rulings, enforced with `initObjects` metadata the analytics + * layer never sees. These are the cases the card names: same-table + * columns only, declared-only enumeration, the tenant-isolation column + * on both sides, same comparison class. + * - Every other position (`$in`/`$nin` members, `$between` endpoints, the + * LIKE family) is refused by the analytics door itself and never routes. + * Those refusals CONVERGE with `driver-sql`'s own refusal arm, so + * routing them would swap one 400 for another 400 a package away while + * losing this package's more precise wording. + */ + for (const refusal of CROSS_FIELD_REFUSALS) { + const routed = findCrossFieldComparand(refusal.filter) !== null; + it(`${refusal.name} → INVALID_FILTER / 400 (${routed ? 'routed, driver-enforced' : 'refused at the analytics door'})`, async () => { + rawSqlCalls = []; + readScope = null; + const err = await errorFrom(() => idsFor({ where: refusal.filter as FilterCondition })); + const note = refusal.note ? `\n${refusal.note}` : ''; + expect(err.code, `wrong code${note}`).toBe('INVALID_FILTER'); + expect(err.status, `wrong status${note}`).toBe(400); + // Never a bind-layer accident dressed up as a refusal. + expect(err).not.toBeInstanceOf(TypeError); + expect(err.message).not.toContain('can only bind'); + // The native-SQL emitter never saw it either way — declined, or refused + // before a strategy was chosen. + expect(rawSqlCalls).toEqual([]); + }); + } + + it('a refused read scope is refused too, and does not degrade to an unscoped read', async () => { + // The direction that matters on this door: a scope that cannot be served + // must never become "no scope". Driven with the tenant-isolation case, + // the named privilege-escalation surface of the four rulings. + rawSqlCalls = []; + readScope = { stage: { $eq: { $field: 'organization_id' } } } as FilterCondition; + const err = await errorFrom(() => idsFor({})); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(rawSqlCalls).toEqual([]); + readScope = null; + }); + + it('the four rulings are each represented, so the loop above is not vacuous', () => { + // The corpus is imported, so a case retired upstream arrives here + // silently. Without this, a corpus that lost its scalar-position refusals + // would leave the `routed` half of the loop green over an empty set + // (#5821's empty-input-set class) — and the routed half IS the card's + // acceptance criterion. + const routed = CROSS_FIELD_REFUSALS.filter((r) => findCrossFieldComparand(r.filter)); + const covers = (fragment: string) => + routed.some((r) => r.messageIncludes.some((m) => m.includes(fragment))); + expect(covers('dotted path'), 'ruling 1: same-table columns only').toBe(true); + expect(covers('not a declared field'), 'ruling 2: declared-only enumeration').toBe(true); + expect(covers('tenant-isolation column'), 'ruling 2, security half').toBe(true); + expect(covers('stored as'), 'the comparison-class rule').toBe(true); + // Both SIDES of the tenant ban — `=` commutes, so a ban a swap walks + // around is not a ban. + expect( + routed.filter((r) => r.messageIncludes.includes('tenant-isolation column')).length, + ).toBeGreaterThanOrEqual(2); + }); + }); + + // ── Routing controls: the decline is narrow ──────────────────────────────── + + describe('the decline is narrow — a query without a reference still takes native SQL', () => { + it('a literal comparand keeps the native-SQL path', async () => { + rawSqlCalls = []; + readScope = null; + await idsFor({ where: { amount: { $gt: 5 } } as FilterCondition }); + expect(rawSqlCalls, 'the literal filter should NOT have declined').toHaveLength(1); + expect(rawSqlCalls[0]).toContain('amount'); + }); + + it('a literal read scope keeps the native-SQL path', async () => { + rawSqlCalls = []; + readScope = { organization_id: 'o1' } as FilterCondition; + await idsFor({}); + expect(rawSqlCalls).toHaveLength(1); + expect(rawSqlCalls[0]).toContain('organization_id'); + readScope = null; + }); + + it('a no-filter query keeps the native-SQL path', async () => { + rawSqlCalls = []; + readScope = null; + await idsFor({}); + expect(rawSqlCalls).toHaveLength(1); + }); + + it('the AUTHORED array spelling declines too — the sugar lowers before the gate', async () => { + // `['amount', '=', { $field: 'budget' }]` is what a client actually sends, + // and `canHandle` reads the filter through `lowerAnalyticsWhere` precisely + // so it sees the reference AFTER `parseFilterAST` has lowered the triple + // to `{ amount: { $eq: … } }` (#7597). Scanning the raw array would have + // missed it, and the query would have gone to native SQL and bound the + // reference — the exact defect, entered through the front door. + rawSqlCalls = []; + readScope = null; + expect(await idsFor({ where: ['amount', '=', { $field: 'budget' }] as unknown as FilterCondition })) + .toEqual(['3', '6']); + expect(rawSqlCalls).toEqual([]); + }); + }); + + // ── `/analytics/sql` — the echo declines, loudly, with no half-rendering ─── + + describe('the `/analytics/sql` echo declines rather than half-rendering', () => { + const sqlFor = (query: Partial) => + service.generateSql({ + cube: 'deals', + dimensions: ['id'], + measures: ['n'], + ...query, + } as AnalyticsQuery); + + it('a `$field`-carrying `where` is refused — INVALID_FILTER / 400', async () => { + readScope = null; + const err = await errorFrom(() => + sqlFor({ where: { amount: { $gt: { $field: 'budget' } } } as FilterCondition }), + ); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('$field'); + expect(err.message).toContain('/analytics/query'); + }); + + it('a `$field`-carrying READ SCOPE is refused — READ_SCOPE_COMPILE_FAILED / 500', async () => { + // A different envelope on purpose, and the one the #5367 ruling fixed: + // a read scope is not the caller's document, so it is not the caller's + // 4xx. #7598 Q2 = A kept that verbatim — no new ADR-0112 code, no 4xx. + readScope = { amount: { $gt: { $field: 'budget' } } } as FilterCondition; + const err = await errorFrom(() => sqlFor({})); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status).toBe(500); + expect(err.message).toContain('read-scope-sql'); + readScope = null; + }); + + it('the refusal is TOTAL — no partial statement is returned alongside it', async () => { + // "No half-rendering" is the ruling's own wording. A renderer that emitted + // the SELECT and dropped the predicate would be the #3601 / #3602 / #3650 + // failure: an echo describing a WIDER query than the one that ran. + readScope = null; + const err = await errorFrom(() => + sqlFor({ where: { amount: { $gt: { $field: 'budget' } } } as FilterCondition }), + ); + expect(err.message).not.toContain('SELECT'); + }); + + it('…while `/analytics/query` still SERVES the same query — the echo is the only thing that declines', async () => { + // The pair that makes the point: one face returns rows, the other refuses, + // and that is deliberate rather than an inconsistency. `execute()` calls + // `generateSql` inside a try/catch precisely because the echo is a + // debugging aid that must never fail a query that already ran, so the + // response simply carries no `sql` string. + readScope = null; + rawSqlCalls = []; + const result = await service.query({ + cube: 'deals', + dimensions: ['id'], + measures: ['n'], + where: { amount: { $gt: { $field: 'budget' } } } as FilterCondition, + } as AnalyticsQuery); + expect(result.rows.map((r) => String(r.id))).toEqual(['1']); + expect(result.sql, 'the echo must be absent, not half-rendered').toBeUndefined(); + expect(rawSqlCalls).toEqual([]); + }); + + it('a literal filter still renders an echo — the decline is narrow here too', async () => { + readScope = null; + const { sql } = await sqlFor({ where: { amount: { $gt: 5 } } as FilterCondition }); + expect(sql).toContain('SELECT'); + expect(sql).toContain('amount'); + }); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts index 0286fa026d..d4d7705998 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-reference-refusal.test.ts @@ -1,15 +1,14 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#7598] Both of this package's SQL-lowering doors answer a `{ $field }` - * comparand the same way — a loud refusal — instead of binding the reference - * object as a value. + * [#7598] Where a `{ $field }` comparand is answered in this package, now that + * the routing ruling has landed — and which refusals survived it. * * ## What was actually wrong, which is NOT what the issue said * * #7598 was filed reading "`service-analytics`' compilers still REFUSE `$field`, * so a CEL field-to-field RLS rule 400s on those faces". Measured on - * `origin/main` (`5823d593d`) before any change here, nothing 400'd. For the six + * `origin/main` (`5823d593d`) before any change, nothing 400'd. For the six * scalar comparison operators — exactly the ones #5222 taught `driver-sql` to * compile — both doors COMPILED, and bound the reference object as the * comparison's value: @@ -22,86 +21,72 @@ * | analytics `where` → ObjectQL engine | `{amount:{$gt:{$field:'budget'}}}` — reached `driver-sql`, which compiles it CORRECTLY since #5222 | * * So the defect was a silent wrong answer, not a refusal: a syntactically - * perfect predicate comparing a column against a value no row can hold. On the - * read-scope door that is an ADMIN's RLS predicate quietly answering the wrong - * row set, which is why it is graded above the `where` door's empty chart. The - * gates that were assumed to be catching this — `isBindableComparand` / - * `isRenderableTextComparand` — were never ASKED about that position: both doors - * consult them for the LIKE family and for `$in`/`$nin`/`$between` MEMBERS only. - * They had not drifted from `driver-sql`; they were answering a different - * question. + * perfect predicate comparing a column against a value no row can hold. The + * gates that were assumed to be catching it — `isBindableComparand` / + * `isRenderableTextComparand` — were never ASKED about that position. They had + * not drifted from `driver-sql`; they were answering a different question. * - * ## The corpus is the shared one, driven through both faces + * ## Two changes, and this file pins the SECOND one's end state * - * `CROSS_FIELD_CASES` / `CROSS_FIELD_REFUSALS` are exported from - * `@objectstack/driver-sql` (`cross-field-conformance-cases.ts`) precisely so a - * second face can be held to the same table, and this suite is that second - * consumer. Held here in the direction the measurement supports: **every case in - * both arms is REFUSED on both analytics doors.** The supported arm's refusals - * are the asymmetry #7598 exists to record, stated as an executable fact — when - * the capability lands here, those cases flip from "refused" to row sets and - * this file is what says so. + * #7694 stopped the bind by REFUSING the shape on both doors — the shipped + * interim, while the routing question went to the maintainer. **The 2026-08-12 + * ruling (Q1 = B) replaced that with routing**: `NativeSQLStrategy.canHandle` + * declines a query whose `where` or read scope carries a scalar reference, the + * query falls through to the ObjectQL/engine path, and `driver-sql` compiles the + * comparison under the four #5222 rulings with the metadata it owns. The + * capability is AVAILABLE, and the security rules live in exactly one place. * - * ⚠️ Corpus `messageIncludes` are deliberately NOT asserted: those pin - * `driver-sql`'s wordings, and this package's refusals are its own (a driver - * message naming `initObjects` declarations would be a lie here). The envelope - * is asserted for every case; the wordings are asserted separately, per door, - * against the sentences `comparand-shape.ts` owns. + * So this file no longer asserts a `where`-door refusal for the supported arm — + * it asserts the LOWERING that carries the reference to the engine, which is the + * step the routing depends on. That the rows actually come back is a separate + * question and a separate suite: `cross-field-engine-fallback.test.ts` runs the + * whole road against a real SQLite engine. This file is the unit half — what + * each compiler in this package does with the shape, asserted where it is cheap + * and exact. * - * ## Reverse verification — direction predicted BEFORE running it - * - * Plain before-green / after-red, with one predicted asymmetry. Removing the two - * `assertNoFieldReferenceComparand` call sites must: + * ## The refusals that survived, and why each one did * - * - turn every case in the `$field`-in-a-scalar-comparand blocks RED, and red - * by RESOLVING rather than by throwing something else — which is why - * `refusalOf` reports "returned" rather than letting a bare `toThrow` count - * a differently-caused throw as a pass; - * - leave the LIKE-family and list-member blocks GREEN, because those refusals - * predate this change and come from the #5234 gates. That half is the proof - * the new gate is NARROW rather than merely present. + * Not every `{ $field }` position was routed. Three classes stayed refused HERE, + * and each converges with `driver-sql`'s own #5222 refusal arm rather than + * diverging from it: * - * Measured with both call sites disabled, over this file and - * `comparand-shape-refusal.test.ts` together: **87 failed / 47 passed**, and - * every failure reads `expected the compiler to refuse this filter, but it - * returned …` — the predicted cells, failing in the predicted MANNER rather than - * by some other throw. Both halves of the prediction held under a targeted - * re-read of the output: + * - the LIKE family (`isRenderableTextComparand`) — a column-side LIKE pattern + * cannot be metacharacter-escaped portably; + * - `$in` / `$nin` MEMBERS (`isBindableComparand`) — the memory evaluator does + * not resolve a reference inside a list either, so there is no semantics to + * be equivalent TO; + * - `$between` ENDPOINTS, which is the one that had to be argued rather than + * inherited. This door LOWERS `$between` into a `gte` leaf and an `lte` leaf, + * so a routed endpoint reference would reach the driver wearing an operator + * #5222 COMPILES — succeeding here while the identical filter is refused on + * both SQL drivers and removed from the spec by #7596. See + * `filter-normalizer.ts`'s `assertNoFieldReferenceComparand`. * - * - not one LIKE-family or `$in` / `$nin` corpus case went red, so the new - * gate is proven NARROW and those refusals are proven to come from the - * #5234 gates rather than from this one; - * - the two `$between`-endpoint cases went red on the `where` door only. On - * the read-scope door they stayed green, because `assertCompilableMembers` - * already refused them there — which is exactly why `$between` had to be - * named on the `where` door: its branch in `fieldLeaves` lowers to `gte` / - * `lte` before any shape gate is consulted, so it was the one comparand - * position on that door no gate had ever seen; - * - `comparand-shape-refusal.test.ts` stayed green in full (1 file failed, 1 - * passed), confirming nothing in the #5234 pins depends on this change. + * And the read-scope door keeps its refusal WHOLE, in its own envelope: after + * the ruling its remaining caller is `ObjectQLStrategy.generateSql`, the + * `/analytics/sql` echo, which has no faithful rendering of the total + * column-to-column predicate the engine path runs. 「一致的响亮答案,不半渲染」. * - * ## The one cell that did NOT refuse on the `where` door — CLOSED by #7693 + * ## Reverse verification — direction predicted BEFORE running it * - * This file used to end in a RECORDED GAP block: `$icontains` was absent from - * `comparand-shape.ts`'s `TEXT_PATTERN_OPERATORS`, so the analytics `where` - * door applied NO comparand-shape gate to it at all — a #5234-class hole that - * arrived with the operator itself (#6520 added `$icontains` to - * `MONGO_TO_CUBE_OP` and to `read-scope-sql`'s `assertRenderableText`, but not - * to that set). #7598 left it alone deliberately and filed it as #7693. + * The predictions, written first: * - * #7693 added the entry, so the pin FLIPPED: the block at the bottom now - * asserts the refusal it used to record the absence of, and the corpus loop - * below no longer has to filter the operator out of `CROSS_FIELD_REFUSALS`. + * 1. Restoring the scalar arm in `assertCompilableComparand` (i.e. undoing the + * ruling) must turn the `where`-door LOWERING assertions below RED **and** + * take `cross-field-engine-fallback.test.ts` down with them — because that + * gate sits in `fieldLeaves`, the one leaf producer for all three consumers + * of the tree, so it refuses the engine path it was meant to route to. + * That coupling is the whole reason the arm had to go, so it is the + * prediction worth measuring. + * 2. Removing the `$between` arm must turn ONLY the `$between` cells red here, + * and must ALSO make the engine-fallback suite's two `$between` refusal + * cells red by RESOLVING — the laundering, demonstrated rather than argued. + * 3. Removing the `#7598` arm of `operatorIsNullTotal` must turn exactly six + * corpus cases red in the fallback suite (three `$ne` class pairs, the + * self-`$ne` control, and the two `$not`-of-`$eq` cases) plus one cell + * here, and nothing else. * - * Reverse-verified by deleting the entry again and re-running the WHOLE - * package: **5 failed / 1553 passed** (green: 1558 / 0). The five are exactly - * the `where`-door cells — the three new ones here, this file's now-unfiltered - * corpus case, and `comparand-shape-refusal.test.ts`'s fifth loop member. What - * stayed GREEN is the other half of the proof: every read-scope `$icontains` - * assertion (that door's refusal comes from its own `assertRenderableText`, not - * from this entry), and every narrowness control below — a well-formed - * `$icontains` comparand compiles identically in both states, so the entry is - * shown to close a hole rather than retire the operator #6520 added. + * Measured — see the PR body for the run output. */ import { describe, it, expect } from 'vitest'; @@ -109,8 +94,7 @@ import { CROSS_FIELD_CASES, CROSS_FIELD_REFUSALS, } from '@objectstack/driver-sql'; -import type { FilterCondition } from '@objectstack/spec/data'; -import type { Cube } from '@objectstack/spec/data'; +import type { Cube, FilterCondition } from '@objectstack/spec/data'; import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; import { normalizeAnalyticsFilterTree } from '../strategies/filter-normalizer.js'; @@ -118,6 +102,7 @@ import { compileScopedFilterToSql } from '../read-scope-sql.js'; import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; import { CROSS_FIELD_COMPARISON_OPERATORS, + findCrossFieldComparand, isFieldReference, } from '../comparand-shape.js'; @@ -147,32 +132,52 @@ function refusalOf(run: () => unknown): WireBearingError { const tree = (where: unknown) => normalizeAnalyticsFilterTree({ where } as any); const scope = (where: unknown) => compileScopedFilterToSql(where as FilterCondition, 'deal'); -/** Does this filter put a reference in a position #5222 made the drivers COMPILE? */ -function usesScalarCrossField(filter: unknown): boolean { - if (!filter || typeof filter !== 'object') return false; - if (Array.isArray(filter)) return filter.some(usesScalarCrossField); - return Object.entries(filter as Record).some(([key, value]) => { - if (CROSS_FIELD_COMPARISON_OPERATORS.has(key) && isFieldReference(value)) return true; - return usesScalarCrossField(value); - }); +/** Every leaf comparand the normalizer produced, structure discarded. */ +function leafComparands(node: unknown): unknown[] { + if (!node || typeof node !== 'object') return []; + const n = node as Record; + if (n.kind === 'leaf') return [...(n.values ?? [])]; + if (n.kind === 'not') return leafComparands(n.child); + if (Array.isArray(n.children)) return n.children.flatMap(leafComparands); + return []; } -// ── The shared corpus, through both doors ──────────────────────────────────── - -describe("[#7598] the #5222 corpus's SUPPORTED arm is refused by both analytics doors", () => { - // Every case here RETURNS ROWS on `driver-sql` / `driver-sqlite-wasm`. That - // these same filters are refused two doors away IS the asymmetry #7598 - // records — pinned so it cannot be closed, or widened, without this file - // saying so. +/** Does this filter put a reference in a position #5222 made the drivers COMPILE? */ +const usesScalarCrossField = (filter: unknown): boolean => + findCrossFieldComparand(filter) !== null; + +const CUBE: Cube = { + name: 'deals', + title: 'Deals', + sql: 'deal', + measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } }, + dimensions: { + id: { name: 'id', label: 'Id', type: 'string', sql: 'id' }, + amount: { name: 'amount', label: 'Amount', type: 'number', sql: 'amount' }, + budget: { name: 'budget', label: 'Budget', type: 'number', sql: 'budget' }, + }, + public: false, +} as unknown as Cube; + +// ── The supported arm: routed, not refused ─────────────────────────────────── + +describe("[#7598] the #5222 corpus's SUPPORTED arm is ROUTED by the `where` door, not refused", () => { for (const testCase of CROSS_FIELD_CASES) { - it(`the \`where\` door refuses: ${testCase.name}`, () => { - const err = refusalOf(() => tree(testCase.filter)); - expect(err.code, testCase.name).toBe('INVALID_FILTER'); - expect(err.status, testCase.name).toBe(400); - expect(err.message, testCase.name).toContain('$field'); + it(`the \`where\` door lowers it with the reference intact: ${testCase.name}`, () => { + // The step the routing rests on. `canHandle` declines the query, so the + // tree this produces is compiled by `ObjectQLStrategy.convertFilter` into + // the `FilterCondition` `engine.aggregate` receives — and a reference that + // did not survive THIS lowering could not survive that one. Asserting the + // comparand rather than the whole tree keeps the assertion about the one + // thing this file is measuring. + const refs = leafComparands(tree(testCase.filter)).filter(isFieldReference); + expect(refs.length, testCase.name).toBeGreaterThan(0); }); - it(`the read-scope door refuses: ${testCase.name}`, () => { + it(`the read-scope door still refuses it (the /analytics/sql echo): ${testCase.name}`, () => { + // Unchanged by the ruling, and deliberately: this lowering's remaining + // caller is the display echo, which cannot render the total predicate the + // engine path runs. #7598 Q2 = A kept the #5367 envelope verbatim. const err = refusalOf(() => scope(testCase.filter)); expect(err.code, testCase.name).toBe('READ_SCOPE_COMPILE_FAILED'); expect(err.status, testCase.name).toBe(500); @@ -180,23 +185,91 @@ describe("[#7598] the #5222 corpus's SUPPORTED arm is refused by both analytics expect(err.message, testCase.name).toContain('$field'); }); } + + it('the `$eq` spelling reaches the engine as an EXPLICIT `$eq`, not a bare field spec', () => { + // The one spelling that would have broken silently. `convertFilter`'s + // `equals` arm returns the comparand BARE for a literal (`{amount: 5}` is + // implicit equality and every backend reads it), which for a reference + // produces `{amount: {$field: 'budget'}}` — a field spec whose only key is + // `$field`, which no backend reads as an equality. That is #7597's defect at + // the spec's lowering sink, arriving here through a different door; the fix + // branches on the COMPARAND, exactly as #7597's did. + expect(engineFilterFor({ amount: { $eq: { $field: 'budget' } } })) + .toEqual({ amount: { $eq: { $field: 'budget' } } }); + // The literal keeps its implicit-equality lowering — the narrowness control. + expect(engineFilterFor({ amount: { $eq: 5 } })).toEqual({ amount: 5 }); + // …and the five siblings were never affected: they emit their operator + // explicitly, so they are asserted as the control that the fix is narrow. + expect(engineFilterFor({ amount: { $gt: { $field: 'budget' } } })) + .toEqual({ amount: { $gt: { $field: 'budget' } } }); + }); + + it('a scalar reference takes NO null guard — the driver writes the predicate total', () => { + // The measured interaction that six corpus cases turned on. `$ne`'s + // negative-polarity totalisation (#5298) is right for a literal — a NULL + // column must satisfy `$ne: 5` — and WRONG for a reference, whose NULL + // semantics are decided by the referent as well. Unguarded, `$ne` excludes + // the both-NULL row, which is what the corpus, both SQL drivers and the + // memory evaluator all say. + expect(tree({ amount: { $ne: { $field: 'budget' } } })).toEqual({ + kind: 'leaf', member: 'amount', operator: 'notEquals', values: [{ $field: 'budget' }], + }); + // …and the literal keeps its guard, unchanged. This pair is the whole claim. + expect(tree({ amount: { $ne: 5 } })).toEqual({ + kind: 'or', + children: [ + { kind: 'leaf', member: 'amount', operator: 'notSet', values: [] }, + { kind: 'leaf', member: 'amount', operator: 'notEquals', values: [5] }, + ], + }); + }); }); -describe("[#7598] the #5222 corpus's REFUSAL arm stays refused on both doors", () => { - // These are refused on the drivers too, so this block asserts CONVERGENCE - // rather than asymmetry. - // - // [#7693] The corpus is driven WHOLE. It used to be filtered — `$icontains` - // was the single exception on the `where` door, because it was missing from - // `TEXT_PATTERN_OPERATORS` — and dropping that filter is half of this card's - // proof: `$icontains against a field reference is refused` is a corpus case - // that only passes here once the entry exists. +/** The `FilterCondition` `ObjectQLStrategy` would hand `engine.aggregate`. */ +function engineFilterFor(where: unknown): Record { + const strategy = new ObjectQLStrategy() as unknown as { + applyFilterNode( + node: unknown, + cube: Cube, + filter: Record, + conjuncts: unknown[], + ): void; + }; + const filter: Record = {}; + const conjuncts: Record[] = []; + strategy.applyFilterNode(tree(where), CUBE, filter, conjuncts); + if (conjuncts.length) filter.$and = [...((filter.$and as unknown[]) ?? []), ...conjuncts]; + return filter; +} + +// ── The refusal arm: split by who answers it ───────────────────────────────── + +describe("[#7598] the #5222 corpus's REFUSAL arm — routed, or refused at this door", () => { for (const testCase of CROSS_FIELD_REFUSALS) { - it(`the \`where\` door refuses: ${testCase.name}`, () => { - const err = refusalOf(() => tree(testCase.filter)); - expect(err.code, testCase.name).toBe('INVALID_FILTER'); - expect(err.status, testCase.name).toBe(400); - }); + const routed = usesScalarCrossField(testCase.filter); + + it( + routed + ? `the \`where\` door ROUTES it to the driver's gate: ${testCase.name}` + : `the \`where\` door refuses it here: ${testCase.name}`, + () => { + if (routed) { + // The four #5222 rulings — dotted paths, undeclared columns, the + // tenant-isolation column, the comparison class — are enforced by + // `driver-sql` with `initObjects` metadata this package cannot see. + // Re-implementing them here is precisely what the ruling rejected as + // option A, so this door must pass the shape THROUGH. That the driver + // then refuses it is asserted end-to-end in + // `cross-field-engine-fallback.test.ts`. + expect(leafComparands(tree(testCase.filter)).filter(isFieldReference).length) + .toBeGreaterThan(0); + return; + } + const err = refusalOf(() => tree(testCase.filter)); + expect(err.code, testCase.name).toBe('INVALID_FILTER'); + expect(err.status, testCase.name).toBe(400); + }, + ); it(`the read-scope door refuses: ${testCase.name}`, () => { const err = refusalOf(() => scope(testCase.filter)); @@ -205,23 +278,34 @@ describe("[#7598] the #5222 corpus's REFUSAL arm stays refused on both doors", ( }); } - it('the two arms are answered by DIFFERENT gates, not by one blanket refusal', () => { + it('the surviving `where`-door refusals are answered by DIFFERENT gates, each with its own wording', () => { // Otherwise every assertion above would hold for a compiler that refused // `{$field}` everywhere with one sentence — which is the thing #5240 says - // sends an operator to the wrong repair. The supported arm hits the new - // capability gate; the LIKE family and list members keep the #5234 wordings. - expect(refusalOf(() => tree({ amount: { $gt: { $field: 'budget' } } })).message) - .toContain('does not compile into a column-to-column comparison'); + // sends an operator to the wrong repair. expect(refusalOf(() => tree({ stage: { $contains: { $field: 'owner' } } })).message) .toContain('StringOperatorSchema'); expect(refusalOf(() => tree({ amount: { $in: [{ $field: 'budget' }, 1] } })).message) .toContain('cannot be bound as a SQL parameter'); - expect(refusalOf(() => scope({ amount: { $gt: { $field: 'budget' } } })).message) - .toContain('does not compile into a column-to-column comparison'); - expect(refusalOf(() => scope({ stage: { $contains: { $field: 'owner' } } })).message) - .toContain('StringOperatorSchema'); - expect(refusalOf(() => scope({ amount: { $in: [{ $field: 'budget' }, 1] } })).message) - .toContain('cannot be bound as a SQL parameter'); + expect(refusalOf(() => tree({ amount: { $between: [{ $field: 'budget' }, 100] } })).message) + .toContain('may not be a field reference on any backend'); + // …and the scalar position is not refused at all here any more. + expect(tree({ amount: { $gt: { $field: 'budget' } } })).toEqual({ + kind: 'leaf', member: 'amount', operator: 'gt', values: [{ $field: 'budget' }], + }); + }); + + it('the `$between` refusal names the laundering it prevents, not a bind failure', () => { + // The repair a `$between` author needs is different from the one a + // read-scope author needs, so the two sentences are different (#5240 in the + // direction that separates rather than merges). + const err = refusalOf(() => tree({ amount: { $between: [0, { $field: 'budget' }] } })); + expect(err.code).toBe('INVALID_FILTER'); + expect(err.status).toBe(400); + expect(err.message).toContain('index 1'); + expect(err.message).toContain('#7596'); + // It points at the spelling that IS served, rather than at "use a literal" + // alone — the capability exists one operator away. + expect(err.message).toContain('"$gte"'); }); it('every corpus case that uses a SCALAR cross-field position is covered', () => { @@ -233,9 +317,29 @@ describe("[#7598] the #5222 corpus's REFUSAL arm stays refused on both doors", ( .filter((c) => usesScalarCrossField(c.filter)); expect(covered.length).toBeGreaterThan(20); }); + + it('the routing detector agrees with the operator set it is built from', () => { + // `findCrossFieldComparand` is what `canHandle` declines on, so a drift + // between it and `CROSS_FIELD_COMPARISON_OPERATORS` would silently narrow + // the decline — and a narrowed decline is a silent bind, not an error. + for (const op of CROSS_FIELD_COMPARISON_OPERATORS) { + expect(findCrossFieldComparand({ amount: { [op]: { $field: 'budget' } } }), op) + .toEqual({ op, field: 'amount', ref: 'budget' }); + } + // …and it finds one however deeply it is nested, because a reference three + // combinators down still needs the engine path. + expect(findCrossFieldComparand({ + $or: [{ stage: 'won' }, { $not: { $and: [{ amount: { $gt: { $field: 'budget' } } }] } }], + })).toEqual({ op: '$gt', field: 'amount', ref: 'budget' }); + // …and it does NOT fire on the positions this door still refuses, which is + // what keeps those refusals reachable instead of routed past. + expect(findCrossFieldComparand({ stage: { $contains: { $field: 'owner' } } })).toBeNull(); + expect(findCrossFieldComparand({ amount: { $in: [{ $field: 'budget' }] } })).toBeNull(); + expect(findCrossFieldComparand({ amount: { $between: [{ $field: 'budget' }, 1] } })).toBeNull(); + }); }); -// ── What the refusal replaced: nothing binds any more ──────────────────────── +// ── What the read-scope refusal replaced: nothing binds any more ───────────── describe('[#7598] the reference object never reaches a bind list again', () => { // The defect was not "an error was missing" — it was a VALUE in the bind list. @@ -285,17 +389,22 @@ describe('[#7598] the reference object never reaches a bind list again', () => { describe('[#7598] the field-reference shape is read exactly as `driver-sql` reads it', () => { it('extra keys do not disqualify a reference — `formula` ignores them too', () => { - const err = refusalOf(() => tree({ amount: { $gt: { $field: 'budget', extra: 1 } } })); - expect(err.code).toBe('INVALID_FILTER'); // #5222 measured this cell driver-side and moved its own test to the // supported arm for it: a narrower reading would let the remainder be // re-bound as a literal on one face and ignored on another. + expect(findCrossFieldComparand({ amount: { $gt: { $field: 'budget', extra: 1 } } })) + .toEqual({ op: '$gt', field: 'amount', ref: 'budget' }); + const err = refusalOf(() => scope({ amount: { $gt: { $field: 'budget', extra: 1 } } })); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); expect(err.message).toContain('budget'); }); it('a NON-STRING `$field` is not a reference — it stays the object account', () => { // `driver-sql`'s `fieldReferenceOf` requires `typeof ref === 'string'`, and // this package mirrors that spelling rather than inventing a third reading. + // It is therefore NOT routed either: it binds as JSON, exactly as any other + // object comparand does, which is the account #5234 left open on purpose. + expect(findCrossFieldComparand({ amount: { $gt: { $field: 5 } } })).toBeNull(); expect(tree({ amount: { $gt: { $field: 5 } } })).toEqual({ kind: 'leaf', member: 'amount', operator: 'gt', values: [{ $field: 5 }], }); @@ -303,35 +412,23 @@ describe('[#7598] the field-reference shape is read exactly as `driver-sql` read }); it('an ordinary object comparand is untouched — #5234 left that account open', () => { + expect(findCrossFieldComparand({ amount: { $eq: { a: 1 } } })).toBeNull(); expect(tree({ amount: { $eq: { a: 1 } } })).toEqual({ kind: 'leaf', member: 'amount', operator: 'equals', values: [{ a: 1 }], }); }); }); -// ── The path this refusal deliberately does NOT touch ──────────────────────── - -describe('[#7598] a read scope keeps working on the ObjectQL engine path', () => { - const CUBE: Cube = { - name: 'deals', - title: 'Deals', - sql: 'deal', - measures: { total: { name: 'total', label: 'Total', type: 'count', sql: '*' } }, - dimensions: { - id: { name: 'id', label: 'Id', type: 'string', sql: 'id' }, - amount: { name: 'amount', label: 'Amount', type: 'number', sql: 'amount' }, - budget: { name: 'budget', label: 'Budget', type: 'number', sql: 'budget' }, - }, - public: false, - } as unknown as Cube; +// ── The path the routing lands on ──────────────────────────────────────────── +describe('[#7598] a read scope reaches the ObjectQL engine path intact', () => { it('the reference reaches `engine.aggregate` intact, never `read-scope-sql`', async () => { - // Load-bearing for `read-scope-sql`'s header claim that this change refuses - // a shape without removing the one path that serves it. `ObjectQLStrategy` - // ANDs the scope into the `FilterCondition` it hands the engine, so the - // reference travels to `driver-sql` — which compiles it under the four #5222 - // rulings, with the declared-field and tenant-column metadata it owns and - // this package does not. + // Load-bearing for `read-scope-sql`'s header claim that its refusal serves + // the display echo without removing the path that SERVES the rule. + // `ObjectQLStrategy` ANDs the scope into the `FilterCondition` it hands the + // engine, so the reference travels to `driver-sql` — which compiles it under + // the four #5222 rulings, with the declared-field and tenant-column metadata + // it owns and this package does not. let seen: unknown; const ctx = { getCube: (n: string) => (n === 'deals' ? CUBE : undefined), diff --git a/packages/services/service-analytics/src/comparand-shape.ts b/packages/services/service-analytics/src/comparand-shape.ts index 5df7c1f6db..db18f5147d 100644 --- a/packages/services/service-analytics/src/comparand-shape.ts +++ b/packages/services/service-analytics/src/comparand-shape.ts @@ -79,9 +79,28 @@ * rather than a widened answer to the first two: the defect was never a * misclassification, so tightening `isBindableComparand` would have changed * cells that were already right (and broken the mirror) while leaving the - * unasked position unasked. See {@link fieldReferenceComparandMessage} for what - * the two doors now say there, and why they say it instead of compiling the - * comparison the SQL drivers compile since #5222. + * unasked position unasked. + * + * ## …and what the answer to that question is now — maintainer ruling 2026-08-12 + * + * #7694 stopped the bind by REFUSING the shape on both doors, as the shipped + * interim while the routing question sat with the maintainer. The ruling (Q1 = + * B) replaced that with routing: `NativeSQLStrategy.canHandle` DECLINES a query + * whose `where` or read scope carries a scalar reference, the query falls + * through to the ObjectQL/engine path, and `driver-sql` compiles the comparison + * under the four #5222 rulings using the `initObjects` metadata it owns. The + * capability is therefore AVAILABLE on the analytics face, and the four security + * rulings live in exactly one place — the option-A alternative (a + * `StrategyContext` enumeration hook plus a second copy of those rulings here) + * was rejected precisely because a guard that exists twice is a guard that will + * disagree with itself. + * + * What this file contributes to that is {@link findCrossFieldComparand}, the + * routing predicate, alongside the two refusal sentences that survive it: + * {@link fieldReferenceComparandMessage} for the `/analytics/sql` echo, which + * cannot honestly RENDER a predicate it does not emit, and + * {@link fieldReferenceBetweenBoundMessage} for a `$between` endpoint, which no + * backend serves and which #7596 removed from the spec. */ /** @@ -180,6 +199,75 @@ export const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet = new Set([ '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', ]); +/** + * [#7598, maintainer ruling 2026-08-12 Q1 = B] The first `{ $field }` reference + * sitting in a position `driver-sql` COMPILES since #5222 — or `null`. + * + * ## What this is FOR, which is not what the two predicates above are for + * + * {@link isBindableComparand} and {@link isRenderableTextComparand} answer + * "may this ONE value reach that ONE position". This walks a WHOLE filter — + * an analytics `where` (already lowered by `lowerAnalyticsWhere`, so the + * authored array sugar arrives here as a `FilterCondition`) or an RLS read + * scope — and answers a routing question instead: **does serving this query + * require the cross-field capability?** `NativeSQLStrategy.canHandle` reads it + * to DECLINE, so the query falls through to the ObjectQL/engine path, where + * `driver-sql` compiles the comparison and enforces the four #5222 rulings + * with the `initObjects` metadata it owns. See that method for the ruling. + * + * ## Why only the six scalar operators, when the ruling says "carries `$field`" + * + * Every OTHER position a reference can occupy is refused IDENTICALLY on both + * sides of the routing decision — the LIKE family and `$in` / `$nin` members + * through this file's two predicates here and through `driver-sql`'s own #5222 + * refusal arm, a `$between` endpoint through + * `filter-normalizer.ts`'s surviving gate, a bare `{ field: { $field: … } }` as + * an unsupported operator. Declining for those would swap one refusal for + * another refusal a package further away, trading this package's precise + * wording for the driver's without changing a single outcome. The scalar + * comparands are the whole of what routing BUYS, so they are the whole of what + * it tests. + * + * The walk is structural and total: it descends into `$and` / `$or` arrays, + * `$not` operands, nested relation objects and any other nesting, because a + * reference three combinators deep still needs the engine path. It is + * deliberately blind to whether the referenced column is DECLARED, is the + * tenant column, or has a comparable type — those are the four rulings, they + * live in exactly one place (`driver-sql`), and re-asking them here is the + * duplicated-guard the ruling rejected as option A. + */ +export function findCrossFieldComparand( + filter: unknown, +): { op: string; field: string; ref: string } | null { + return findIn(filter, ''); +} + +function findIn( + node: unknown, + field: string, +): { op: string; field: string; ref: string } | null { + if (!node || typeof node !== 'object') return null; + if (Array.isArray(node)) { + for (const child of node) { + const hit = findIn(child, field); + if (hit) return hit; + } + return null; + } + if (node instanceof Date || ArrayBuffer.isView(node)) return null; + for (const [key, value] of Object.entries(node as Record)) { + if (CROSS_FIELD_COMPARISON_OPERATORS.has(key) && isFieldReference(value)) { + return { op: key, field, ref: value.$field }; + } + // A `$`-prefixed key is an operator or a combinator, so the FIELD in scope + // does not change; anything else names a field (or a nested relation + // member) and becomes the new scope. Only used for the message. + const hit = findIn(value, key.startsWith('$') ? field : key); + if (hit) return hit; + } + return null; +} + /** * The Filter Protocol operators whose comparand becomes the text of a `LIKE` * pattern — the ones every compiler in this package routes through @@ -244,39 +332,42 @@ export function unrenderableTextComparandMessage(op: string, field: string, valu } /** - * [#7598] The sentence both doors say about a `{ $field }` comparand they do not - * compile — shared for the same reason {@link unrenderableTextComparandMessage} - * is, and with the same split: one diagnosis, two envelopes. - * - * ## What it has to say that the other two do not - * - * The other refusals in this file answer a shape that is wrong everywhere. This - * one answers a shape that is RIGHT somewhere: since #5222 `driver-sql` and - * `driver-sqlite-wasm` compile exactly this comparand into a same-table - * column-to-column comparison, and `@objectstack/formula`'s - * `matchesFilterCondition` has always resolved it in memory. So the author is - * not told "this is nonsense" — they are told WHICH face declined and why, which - * is the difference between an authoring mistake and a platform boundary. - * - * It also names what used to happen, because that is the part a reader cannot + * [#7598] What `read-scope-sql` says about a `{ $field }` comparand it cannot + * lower — the ONE surviving caller of this sentence after the 2026-08-12 ruling, + * and the reason it now reads as a RENDERING boundary rather than a capability + * one. + * + * ## What changed under the ruling, and why the wording had to follow + * + * Until that ruling this sentence was said by both doors and meant "the platform + * will not serve this here". It no longer means that. Q1 = B routes a query + * whose `where` or read scope carries a reference to the ObjectQL/engine path, + * where `driver-sql` compiles the comparison and enforces the four #5222 rulings + * with metadata it owns — so `/analytics/query` SERVES these queries and returns + * rows. What is left is `compileScopedFilterToSql`, and the caller that still + * reaches it with such a scope is `ObjectQLStrategy.generateSql`: the + * `/analytics/sql` ECHO, a display string for an execution it does not perform. + * There is no honest rendering of a total column-to-column predicate available + * to that renderer, and the ruling's answer for the echo was explicit — + * 「一致的响亮答案,不半渲染」 (one consistent, loud answer; no half-rendering). + * + * So the message tells a reader three things it could not tell them before: the + * query itself is fine, the ECHO is what declined, and the rows are one call + * away on `/analytics/query`. Telling them instead to "compare against a literal" + * would send them to repair a rule that works. + * + * It still names what USED to happen, because that is the part a reader cannot * reconstruct: the reference was BOUND. The predicate was syntactically perfect, * the query ran, and a column was compared against a value no row can hold — no - * error, no log line, an empty chart or a read scope quietly answering the wrong - * row set. That is the #3650 / #5234 class, and naming it is what stops the next - * reader from "restoring" the old tolerance as a convenience. - * - * ## Why the reason is a MISSING ENUMERATION and not a missing emitter - * - * The SQL is trivial — two identifiers and an operator. What these two compilers - * do not have is the four things #5222's maintainer rulings (2026-08-06) require - * before a name may enter a SQL identifier position: the object's DECLARED field - * set, its declared TYPES (for the same-comparison-class rule), its - * tenant-isolation column (forbidden on both sides), and whether the table is - * federated. `driver-sql` reads all four out of its own `initObjects` capture; - * `StrategyContext` (`@objectstack/spec/contracts`) exposes none of them, so - * these compilers cannot enforce the rulings and refuse rather than ship a - * weaker port of them. Implementing it here is therefore a `packages/spec` - * surface question first — tracked on #7598. + * error, no log line, an admin's read scope quietly answering the wrong row set. + * That is the #3650 / #5234 class, and naming it is what stops the next reader + * from "restoring" the old tolerance as a convenience. + * + * ⛔ `position` is no longer passed by any caller for a `$between` endpoint — + * that arm says {@link fieldReferenceBetweenBoundMessage} instead, because it is + * refused permanently and everywhere rather than declined by one renderer. The + * parameter stays for a caller that needs to locate a reference inside a nested + * scope. */ export function fieldReferenceComparandMessage( op: string, @@ -286,19 +377,65 @@ export function fieldReferenceComparandMessage( ): string { return ( `"${op}" on "${field}"${position ? ` (${position})` : ''} compares against the field reference ` + - `{ "$field": "${ref}" }, which this compiler does not compile into a column-to-column ` + + `{ "$field": "${ref}" }, which this compiler does not lower into a column-to-column ` + `comparison. Refusing rather than binding it: the reference object used to become the BOUND ` + `VALUE of the comparison, so the emitted predicate compared "${field}" against the reference ` + - `itself — a value no row can hold — and returned a wrong row set with nothing to read. ` + - `@objectstack/spec declares this shape (FieldReferenceSchema) and it IS executed elsewhere: ` + - `@objectstack/formula resolves it per record in memory, and driver-sql / driver-sqlite-wasm ` + - `compile it to a same-table column comparison for the six scalar operators since #5222. It is ` + - `refused HERE because the #5222 rulings admit a referenced column name into SQL only after ` + - `checking it against the object's declared fields, their declared types, and its ` + - `tenant-isolation column — none of which this compiler can see (StrategyContext exposes no ` + - `such hook), so enforcing them is impossible and skipping them would open a comparison ` + - `surface onto the tenant boundary. Compare against a literal value here, or route the query ` + - `through the ObjectQL engine path, whose driver does the enforcing (#7598).` + `itself — a value no row can hold — and a read scope built from it answered the wrong row set ` + + `with nothing to read. ⚠️ This is NOT the platform declining the rule. @objectstack/spec ` + + `declares this shape (FieldReferenceSchema), @objectstack/formula resolves it per record in ` + + `memory, driver-sql / driver-sqlite-wasm compile it to a same-table column comparison for the ` + + `six scalar operators since #5222, and since the 2026-08-12 ruling on #7598 the analytics ` + + `native-SQL strategy DECLINES such a query so it routes to the ObjectQL engine path and runs ` + + `there — the driver enforcing declared-only enumeration, the tenant-isolation ban and the ` + + `comparison class with metadata it owns. What refuses here is this SQL lowering, whose only ` + + `remaining caller is the /analytics/sql display echo; it has no faithful rendering of the ` + + `predicate the engine path actually runs, and half-rendering one would describe a query that ` + + `returns different rows. Run the query itself (/analytics/query) to get its rows (#7598).` + ); +} + +/** + * [#7598] A `{ $field }` in a `$between` ENDPOINT — a separate sentence from + * {@link fieldReferenceComparandMessage} because it is a separate condition, + * and #5240's rule cuts the other way here: two shapes with two repairs must + * not share one wording. + * + * The scalar comparands above are SERVED, one path over. A `$between` endpoint + * is not served anywhere and is not going to be: `driver-sql` and + * `driver-sqlite-wasm` refuse it (`CROSS_FIELD_REFUSALS` pins both endpoints), + * the memory evaluator has no reading of it either — `resolveValue` returns an + * array unchanged, so the bounds are ordered against the raw reference OBJECT — + * and #7596 removed the position from `FieldReferenceSchema` outright under + * ADR-0049 declared = enforced (maintainer ruling 2026-08-11). Pointing that + * author at the engine path would point them at another refusal. + * + * It matters most on THIS door, which is why the arm exists here at all: the + * analytics `where` lowering splits `$between` into a `gte` leaf and an `lte` + * leaf, so an endpoint reference would reach the driver wearing an operator + * #5222 COMPILES — succeeding on the analytics face alone, in defiance of both + * the driver corpus and the schema. See `filter-normalizer.ts`'s + * `assertNoFieldReferenceComparand`. + */ +export function fieldReferenceBetweenBoundMessage( + op: string, + field: string, + ref: string, + index: number, +): string { + return ( + `"${op}" on "${field}" has the field reference { "$field": "${ref}" } at index ${index} of its ` + + `[min, max] bounds. A range BOUND may not be a field reference on any backend: driver-sql and ` + + `driver-sqlite-wasm refuse both endpoints (#5222), @objectstack/formula does not resolve a ` + + `reference inside a list either — it orders the bounds against the raw reference object, which ` + + `no value compares meaningfully to — and @objectstack/spec no longer declares the position at ` + + `all (#7596 removed FieldReferenceSchema from the $between endpoint union, ADR-0049 declared = ` + + `enforced). Refusing rather than lowering it: this compiler splits $between into its two ` + + `bounds, so the reference would arrive at the driver under a "$gte" / "$lte" the author never ` + + `wrote — a position the SQL drivers DO compile — and the range would quietly succeed here ` + + `while the identical filter is refused everywhere else. Use a literal bound, or spell the ` + + `comparison you meant as a scalar one ({ "${field}": { "$gte": { "$field": "${ref}" } } }), ` + + `which IS served — on the ObjectQL engine path, where the driver enforces the #5222 rulings ` + + `(#7598).` ); } diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 18a1c2cf86..d78f370889 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -5,6 +5,7 @@ import type { RegisteredErrorCode } from '@objectstack/spec/api'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr } from './like-pattern.js'; import { CROSS_FIELD_COMPARISON_OPERATORS, + fieldReferenceBetweenBoundMessage, fieldReferenceComparandMessage, isBindableComparand, isFieldReference, @@ -223,14 +224,35 @@ import { * tenant-isolation column, so the enumeration the rulings turn on does not * exist on this side). * - * ⚠️ Note what this does NOT do: it does not make the capability available. - * A read scope carrying a field-to-field comparison still cannot be served by - * the raw-SQL analytics path — it is now REFUSED there instead of silently - * mis-answered, which is the whole of the change. The same scope continues to - * work on the ObjectQL engine path, where the driver compiles it and enforces - * the rulings with the metadata it owns (measured: `ObjectQLStrategy` ANDs the - * scope into the `FilterCondition` it hands `engine.aggregate`, so the reference - * reaches `driver-sql` intact and never passes through this file). + * ⚠️ [UPDATED — maintainer ruling 2026-08-12, #7598 Q1 = B] When this section + * was written the refusal was the whole answer, and it said so: "it does not + * make the capability available". **It does now, by getting out of the way.** + * `NativeSQLStrategy.canHandle` DECLINES a query whose read scope carries a + * reference, so the query falls through to the ObjectQL/engine path — where + * `ObjectQLStrategy` ANDs the scope into the `FilterCondition` it hands + * `engine.aggregate`, the reference reaches `driver-sql` intact, and the driver + * compiles it under all four #5222 rulings using its own `initObjects` + * metadata. A field-to-field RLS rule is therefore SERVED on the analytics + * face, and the security rules live in exactly one place rather than two. + * + * What that leaves for the gate below is a narrower but still live job: + * `applyReadScope` (`native-sql-strategy.ts`) no longer reaches it — the + * decline runs first — but `ObjectQLStrategy.generateSql` does. That is the + * `/analytics/sql` ECHO, a display string for an execution it does not perform, + * and it has no faithful rendering of the total column-to-column predicate the + * engine path runs. The ruling answered that face explicitly — + * 「一致的响亮答案,不半渲染」 (one consistent, loud answer; no half-rendering) — + * so the refusal here IS the echo's decline. `compileScopedFilterToSql` is also + * a public export of this package (`index.ts`), so the gate additionally holds + * for any consumer outside these two. + * + * ⛔ The ENVELOPE is untouched, and deliberately: Q2 = A kept the #5367 ruling + * verbatim — `READ_SCOPE_COMPILE_FAILED` / 500 with the message withheld. No new + * ADR-0112 code for the unsupported-rule class (option C was declined for zero + * measured pull; #5367 recorded that no consumer reads a code on this path), and + * no 4xx (option B reintroduces both defects #5367 closed). The paragraph above + * beginning "⚠️ Deliberately NOT a 4xx of any flavour" is that ruling's own text + * and is not to be rewritten. */ const IDENT = /^[a-z_][a-z0-9_]*$/i; @@ -837,21 +859,37 @@ function assertBooleanFlagComparands(field: string, spec: unknown): void { * * `$between` is in the covered set even though {@link assertCompilableMembers} * would also refuse its endpoints, because this gate runs FIRST and the truer - * diagnosis wins: "a field reference is not compiled here" tells the policy - * author what to write, where "cannot be bound as a SQL parameter" describes a - * consequence of the shape rather than the shape. - * - * ## Envelope: unchanged, deliberately (#5367 ruling, 2026-08-06) + * diagnosis wins: "a range bound may not be a field reference on any backend" + * tells the policy author what to write, where "cannot be bound as a SQL + * parameter" describes a consequence of the shape rather than the shape. The two + * covered positions now say DIFFERENT sentences, because the 2026-08-12 ruling + * made them different conditions — see {@link fieldReferenceComparandMessage} + * (a rendering boundary on a rule the platform SERVES) versus + * {@link fieldReferenceBetweenBoundMessage} (a position refused everywhere, and + * removed from the spec by #7596). + * + * ## What reaches this gate after the 2026-08-12 ruling + * + * Not `applyReadScope`. `NativeSQLStrategy.canHandle` declines a query whose + * read scope carries a scalar reference before that method runs, so the scope + * is served on the engine path instead (module header). What DOES reach it is + * `ObjectQLStrategy.generateSql` — the `/analytics/sql` echo — plus any external + * consumer of the `compileScopedFilterToSql` export. The gate is therefore the + * echo's decline, which is what the ruling asked that face for. + * + * ## Envelope: unchanged, deliberately (#5367 ruling 2026-08-06, re-affirmed as + * #7598 Q2 = A on 2026-08-12) * * `READ_SCOPE_COMPILE_FAILED` / 500, like the other twelve. The two arguments - * that ruling gave apply to this shape verbatim rather than by analogy: the - * producer is an ADMIN-authored sharing rule / permission set and its CEL - * lowering — `compileCelToFilter` is exactly what emits `{ $field: path }` — so - * a 4xx would bill the caller for a document they cannot author, and a 4xx - * echoes the message, which here names the POLICY's field names. Whether an - * unsupported-capability refusal on this path should nevertheless get a code and - * status of its own is a contract-face question raised on #7598 and left to the - * maintainer; it is not decided as a rider by the change that stops the bind. + * #5367 gave apply to this shape verbatim rather than by analogy: the producer + * is an ADMIN-authored sharing rule / permission set and its CEL lowering — + * `compileCelToFilter` is exactly what emits `{ $field: path }` — so a 4xx would + * bill the caller for a document they cannot author, and a 4xx echoes the + * message, which here names the POLICY's field names. #7598 put the question to + * the maintainer and it was answered A: keep #5367 as it stands, add no new + * ADR-0112 code for the unsupported-rule class (option C had zero measured pull + * — #5367 recorded that no consumer reads a code on this path — and a zero-pull + * vocabulary is recorded, not built), and do not move to 4xx. */ function assertNoFieldReferenceComparand(field: string, spec: unknown): void { if (!isFilterNode(spec)) return; @@ -865,7 +903,7 @@ function assertNoFieldReferenceComparand(field: string, spec: unknown): void { opValue.forEach((member, index) => { if (!isFieldReference(member)) return; throw readScopeCompileError( - `[read-scope-sql] ${fieldReferenceComparandMessage(op, field, member.$field, `index ${index}`)}`, + `[read-scope-sql] ${fieldReferenceBetweenBoundMessage(op, field, member.$field, index)}`, ); }); } diff --git a/packages/services/service-analytics/src/strategies/filter-normalizer.ts b/packages/services/service-analytics/src/strategies/filter-normalizer.ts index ef5a6c39a6..196b087844 100644 --- a/packages/services/service-analytics/src/strategies/filter-normalizer.ts +++ b/packages/services/service-analytics/src/strategies/filter-normalizer.ts @@ -351,7 +351,7 @@ import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/s import { StandardErrorCode } from '@objectstack/spec/api'; import { CROSS_FIELD_COMPARISON_OPERATORS, - fieldReferenceComparandMessage, + fieldReferenceBetweenBoundMessage, isBindableComparand, isFieldReference, isRenderableTextComparand, @@ -388,8 +388,19 @@ export interface NormalizedAnalyticsFilter { * * It carries no `#5352`-specific wording on purpose — the envelope is the * contract, the message stays whatever the refusing site says. + * + * [#7598] EXPORTED, and the "only way this module refuses" invariant is + * unchanged by it: that rule is about this file's own sites, and the export + * exists so a sibling in this directory cannot invent a SECOND spelling of the + * same envelope. `ObjectQLStrategy.generateSql` refuses a cross-field + * comparison it cannot honestly render, and that refusal is a `where`-door + * refusal in every respect that matters — the caller authored the input, the + * repair is theirs — so it takes the `where` door's envelope rather than a + * hand-rolled twin. (`read-scope-sql.ts` keeps its OWN local error factory + * because its envelope genuinely differs: a read scope is not caller-authored, + * hence `READ_SCOPE_COMPILE_FAILED` / 500 by the #5367 ruling.) */ -function invalidFilterError(message: string): Error { +export function invalidFilterError(message: string): Error { const err = new Error(message) as Error & { code?: string; status?: number }; err.code = StandardErrorCode.enum.INVALID_FILTER; err.status = 400; @@ -562,18 +573,26 @@ function andOf(children: NormalizedFilterNode[]): NormalizedFilterNode | null { * `FilterCondition` that never passes through here — and carries the same two * checks in its own fail-closed envelope. * - * Three shapes are refused: the two #5234 measured, and — since #7598 — a - * `{$field}` reference in the comparand of a scalar comparison, the position - * both of #5234's checks are simply never asked about. `$eq` and friends keep + * Two shapes are refused, the two #5234 measured. `$eq` and friends keep * binding any OTHER object as JSON (`toSqlBindValue`), which remains a separate * account. + * + * ⚠️ [#7598, maintainer ruling 2026-08-12 Q1 = B] A THIRD arm briefly lived + * here — a `{$field}` reference in the comparand of the six scalar comparison + * operators, added by #7694 as the shipped interim while the routing question + * was with the maintainer. It is GONE, and its removal is the point of the + * ruling rather than a cleanup: this function runs inside {@link fieldLeaves}, + * which is the one producer of leaf nodes for ALL THREE consumers of this tree + * — including `ObjectQLStrategy.convertFilter`, the ENGINE path. Refusing here + * therefore refused the very execution B routes such a query to, so the arm and + * the ruling cannot both stand. `NativeSQLStrategy.canHandle` now declines + * instead (see the ruling recorded there), and `driver-sql` compiles the + * comparison under the four #5222 rulings with the metadata it owns. + * + * What did NOT move is the `$between` arm — see + * {@link assertNoFieldReferenceComparand}, which is now that arm alone. */ function assertCompilableComparand(opKey: string, field: string, value: unknown): void { - // [#7598] The field-reference arm runs FIRST, and only over the positions that - // were BOUND — see {@link assertNoFieldReferenceComparand} for the measured - // table, for why the LIKE / list positions keep their own (converging) wording - // instead, and for why refusing is the answer rather than compiling. - assertNoFieldReferenceComparand(opKey, field, value); if (TEXT_PATTERN_OPERATORS.has(opKey)) { // An array reaches this door as `values[0]` — i.e. every member after the // first is silently DROPPED — while `read-scope-sql` and `driver-sql` @@ -594,74 +613,52 @@ function assertCompilableComparand(opKey: string, field: string, value: unknown) } /** - * [#7598] A `{ $field: 'col' }` reference in a comparand position this door - * BOUND instead of refusing. - * - * ## The measured cell, on `origin/main` (`5823d593d`) - * - * `{ amount: { $gt: { $field: 'budget' } } }` produced the leaf - * `{member: 'amount', operator: 'gt', values: [{$field: 'budget'}]}` — and then - * the THREE consumers of that leaf answered three different ways, which is the - * split this module's header spends its length removing: - * - * | consumer | answer | - * |---|---| - * | `NativeSQLStrategy` (the statement that executes) | `WHERE amount > $1`, bound to the JSON TEXT `{"$field":"budget"}` | - * | `ObjectQLStrategy.generateSql` (the `/analytics/sql` echo) | `WHERE amount > $1`, bound to the reference OBJECT | - * | `ObjectQLStrategy.convertFilter` (the engine path) | `{amount: {$gt: {$field: 'budget'}}}` — reaches `driver-sql`, which COMPILES it correctly since #5222 | - * - * Two wrong answers and one right one, chosen by which strategy the datasource - * routed to. The two wrong ones are wrong in the silent way (#3650 / #5234): a - * valid statement comparing a column against a value no row can hold, so a - * widget draws an empty chart with nothing to read. - * - * ## Why this refuses instead of compiling — and what it costs - * - * ⚠️ Refusing at the door NARROWS the engine path, which today passes the - * reference through to a driver that handles it properly. That cost is taken - * deliberately and is the part to re-open if the maintainer rules otherwise: - * - * - the pass-through is an ACCIDENT of {@link convertFilter} forwarding an - * unrecognised comparand, not a capability this package implements — nothing - * here validates it, and no test pinned it; - * - leaving it makes one authored `where` mean two things depending on the - * backend behind the cube, which is the exact "whichever face took the query - * is the answer you get" split #5146 / #5332 / #5567 / #5298 each spent a - * round removing — and the loud half of it would still be missing, since the - * other two emitters cannot be made to agree; - * - the four #5222 rulings that make a referenced column name safe in a SQL - * identifier position (declared-only enumeration, declared types for the - * comparison class, the tenant-isolation column, federation) turn on - * metadata `StrategyContext` does not expose, so this door cannot enforce - * them for the two emitters that would need it. - * - * So the package answers ONE way, loudly, exactly as it already does for - * `{$contains: {$field: …}}` — a refusal that CONVERGES with `driver-sql`'s own - * #5222 refusal arm. Whether the capability should instead be IMPLEMENTED here - * (which needs a `StrategyContext` hook, i.e. a `packages/spec` change) is the - * open question on #7598, and is deliberately not decided by this gate. - * - * ## Positions - * - * Only the ones that were bound: the six scalar comparison operators' - * whole comparand ({@link CROSS_FIELD_COMPARISON_OPERATORS}) and the two - * `$between` endpoints. `$between` needs naming because its branch in - * {@link fieldLeaves} lowers to `gte` / `lte` leaves BEFORE this gate is - * consulted, so its endpoints were the one position no shape gate on this door - * ever saw. The LIKE family and `$in` / `$nin` members keep their existing - * wording — they were already refused here AND on `driver-sql`. + * [#7598] A `{ $field: 'col' }` reference in a `$between` ENDPOINT — the one + * position on this door where the gate is still load-bearing after the + * 2026-08-12 ruling, and the one place its removal would silently CREATE a + * capability rather than remove a refusal. + * + * ## Why this arm survived when the scalar-comparand arm did not + * + * The ruling (Q1 = B) moved the six scalar comparison operators OUT of this + * door's business entirely: `NativeSQLStrategy.canHandle` declines a `where` + * carrying one, the query routes to the engine, and `driver-sql` compiles the + * comparison under the four #5222 rulings. Refusing them here would refuse the + * execution the routing exists to reach, so that arm is gone. + * + * `$between` is the opposite case, because of a LOWERING this door performs and + * the driver never sees. {@link fieldLeaves}'s `$between` branch splits + * `{ $between: [a, b] }` into a `gte` leaf and an `lte` leaf, and + * `ObjectQLStrategy.convertFilter` hands those to the engine as `{ $gte: a }` / + * `{ $lte: b }`. So a reference in an endpoint would arrive at `driver-sql` + * wearing a `$gte` it was never authored with — and `$gte` is a position #5222 + * COMPILES. The result would be that `{ amount: { $between: [{ $field: + * 'budget' }, 100] } }` quietly SUCCEEDS on the analytics face while + * `CROSS_FIELD_REFUSALS` pins it as refused on both SQL drivers, and while + * #7596 has removed the position from `FieldReferenceSchema` altogether + * (maintainer ruling 2026-08-11, ADR-0049 declared = enforced). One shape, two + * answers, created by a laundering this module does on the way past — exactly + * the class #7598 was filed about, spelled backwards. + * + * Refusing here therefore CONVERGES with `driver-sql`'s own #5222 refusal arm, + * which is what every surviving `{$field}` refusal in this package now does: + * the LIKE family through {@link assertCompilableComparand}'s + * `isRenderableTextComparand` call, `$in` / `$nin` members through its + * `isBindableComparand` one, a bare `{ field: { $field: … } }` as an unsupported + * operator, and this. The scalar comparands are the only positions where the + * two faces now differ, and they differ by the analytics face DECLINING to + * serve them itself rather than by refusing them. + * + * Asserted under the `$between` name, not under the `gte` / `lte` the bounds + * lower to, because the author wrote `$between` and that is the key they have + * to repair. */ function assertNoFieldReferenceComparand(opKey: string, field: string, value: unknown): void { - if (CROSS_FIELD_COMPARISON_OPERATORS.has(opKey) && isFieldReference(value)) { - throw invalidFilterError( - `[analytics] ${fieldReferenceComparandMessage(opKey, field, value.$field)}`, - ); - } if (opKey !== '$between' || !Array.isArray(value)) return; value.forEach((member, index) => { if (!isFieldReference(member)) return; throw invalidFilterError( - `[analytics] ${fieldReferenceComparandMessage(opKey, field, member.$field, `index ${index}`)}`, + `[analytics] ${fieldReferenceBetweenBoundMessage(opKey, field, member.$field, index)}`, ); }); } @@ -1279,6 +1276,41 @@ function nullValueSatisfiesOperator(op: string, value: unknown): boolean { /** Is this operator's compiled leaf already total for a NULL column? */ function operatorIsNullTotal(op: string, value: unknown): boolean { + // [#7598, maintainer ruling 2026-08-12] A `{ $field }` comparand on any of the + // six scalar comparison operators is TOTAL AT THE BACKEND, so this module must + // add no guard of its own — and MEASURED, adding one changes the answer. + // + // Every other entry in this switch is total because THIS module compiles the + // operator into a null predicate. This one is total because of where the leaf + // ends up: since the ruling, a `where` carrying a reference is declined by + // `NativeSQLStrategy.canHandle` and served on the engine path, where + // `driver-sql`'s `applyCrossFieldComparison` emits a predicate written total + // across NULLs by construction (it repeats both column expressions for exactly + // that reason — see `cross-field-conformance-cases.ts`, whose rows 4-6 carry + // every NULL arrangement a pair of columns can be in). `@objectstack/formula` + // resolves the reference and then compares in two-valued JS. The two agree, + // and the corpus's declared id lists are the third statement of it. + // + // ## What the guard did before this arm existed — measured on the wasm driver + // + // The `$ne` arm of {@link nullValueSatisfiesOperator} answers `true` for any + // non-null comparand, so a reference took the negative-polarity totalisation + // in {@link fieldLeaves} and `{ amount: { $ne: { $field: 'budget' } } }` + // lowered to `{$or: [{amount: null}, {amount: {$ne: ref}}]}`. That admitted + // fixture row 6 — BOTH columns NULL — where the corpus, both SQL drivers and + // the memory evaluator all EXCLUDE it, because row 6 satisfies the inner + // `$eq` and `$ne` is its exact complement. Six corpus cases moved: the three + // `$ne` class-pair cases, `a column differs from itself on no row`, and the + // two `$not`-of-`$eq` cases (which reach the same guard through + // {@link nullGuardForFieldSpec}). Widening a `$ne`, on a shape whose producer + // is an RLS rule, is the direction that matters. + // + // The guard is right for a LITERAL comparand and is untouched there: `{amount: + // {$ne: 5}}` must still admit a NULL `amount`, which is #5298's ruling and the + // JS backends' answer. What differs is only that a reference's NULL semantics + // are already decided by the referent, not by the target column alone — so + // there is nothing left for a guard to decide. + if (CROSS_FIELD_COMPARISON_OPERATORS.has(op) && isFieldReference(value)) return true; switch (op) { // Compile to `set` / `notSet` — `IS NULL` / `IS NOT NULL`, two-valued by // construction, on every strategy that compiles this tree. diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 1e5afffd40..0ece11e08c 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -4,12 +4,14 @@ import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contract import type { Cube } from '@objectstack/spec/data'; import type { AnalyticsStrategy, StrategyContext } from './types.js'; import { + lowerAnalyticsWhere, normalizeAnalyticsFilterTree, toSqlBindValue, SQL_CONST_FALSE, SQL_CONST_TRUE, type NormalizedFilterNode, } from './filter-normalizer.js'; +import { findCrossFieldComparand } from '../comparand-shape.js'; import { compileScopedFilterToSql } from '../read-scope-sql.js'; import { datasetInvalidError, invalidMemberError } from '../dataset-refusal.js'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr, type LikeShape } from '../like-pattern.js'; @@ -117,10 +119,148 @@ export class NativeSQLStrategy implements AnalyticsStrategy { } } } + // ── [#7598] DECLINE a `{ $field }` cross-field comparison ─────────────── + // + // ## The maintainer ruling this implements (2026-08-12, Q1 = B) + // + // 「`NativeSQLStrategy.canHandle` 对携带 `$field` 的 `where` / read scope + // **decline**,落回 ObjectQL/engine 路径,由 driver 用它自有的 metadata 强制 + // 全部四条 #5222 裁定 —— 安全规则只存在一处,不复制、不新增 + // `StrategyContext` 钩子、不动 `packages/spec`。⚠️ canHandle 依据 filter + // 内容路由是新行为 —— 认可并接受,实现时在 canHandle 处注释记录本裁定。」 + // + // (Q1 = B; option A — `StrategyContext.getDeclaredFields` / `getTenantColumn` + // hooks plus a SECOND implementation of the four rulings inside this package + // — was explicitly rejected: it builds an enumeration surface with no + // measured consumer, and its fallback when a host omits a hook is either + // "refuse" or "skip the check", and skipping the check is the defect #7598 + // exists to close. Q2 = A: `read-scope-sql`'s envelope is untouched.) + // + // ## What is new here, and why it is sound + // + // Every other decline above turns on the query's SHAPE (a granularity, a + // federated object). This one turns on filter CONTENT, which is new + // behaviour for `canHandle` — named as such in the ruling and accepted + // there. It is the same mechanism ADR-0062 D6 already uses one branch up: + // when this strategy cannot compile something CORRECTLY, routing to the + // lower-priority ObjectQL path is better than compiling it anyway. What it + // cannot compile correctly here is a column-to-column comparison, because + // the four #5222 rulings (same-table columns only, declared-only + // enumeration, tenant-isolation column forbidden on BOTH sides, same + // comparison class) each turn on metadata `StrategyContext` does not expose + // — an object's declared field set, its declared types, its + // tenant-isolation column. `driver-sql` reads all four out of its own + // `initObjects` capture, so declining puts the query in front of the one + // component that can enforce them, instead of enforcing them twice. + // + // ## Both inputs, because a read scope is not the caller's `where` + // + // The caller's `where` and the RLS read scope are separate producers and + // either can carry a reference — `compileCelToFilter` emits `{ $field }` + // for a field-to-field comparison in an ADMIN-authored CEL rule, which is + // the read-scope half and the one #5041 measured. The scopes of the JOINED + // objects are read too, for the same reason `generateSql` injects them: + // `applyReadScope` would compile each of them through `read-scope-sql`. + // + // `lowerAnalyticsWhere` rather than `query.where` raw, so the authored + // ARRAY sugar (`['amount', '=', { $field: 'budget' }]`) is seen after + // `parseFilterAST` has lowered it (#7597). A THROW from that lowering is + // not this gate's to answer — the filter is malformed either way and + // `normalizeAnalyticsFilterTree` refuses it a moment later with the message + // and envelope it has always had — so it is caught and read as "no + // reference found". + if (this.carriesCrossFieldComparison(query, ctx)) return false; const caps = ctx.queryCapabilities(query.cube); return caps.nativeSql && typeof ctx.executeRawSql === 'function'; } + /** + * [#7598] Does serving this query require the cross-field capability this + * strategy declines? See the ruling recorded at {@link canHandle}. + * + * ⚠️ This and {@link assertNoCrossFieldComparison} read the SAME inputs + * through the SAME detector, which is what makes the decline and the + * fail-closed backstop unable to drift: a shape one of them recognises is a + * shape the other recognises. + */ + private carriesCrossFieldComparison(query: AnalyticsQuery, ctx: StrategyContext): boolean { + return this.crossFieldComparisonIn(query, ctx) !== null; + } + + private crossFieldComparisonIn( + query: AnalyticsQuery, + ctx: StrategyContext, + ): { source: string; op: string; field: string; ref: string } | null { + let where: unknown = null; + try { + where = lowerAnalyticsWhere(query); + } catch { + // A `where` this compiler cannot even lower is refused downstream, with + // its own message. Nothing to route. + return null; + } + const inWhere = findCrossFieldComparand(where); + if (inWhere) return { source: 'the query\'s `where`', ...inWhere }; + + if (typeof ctx.getReadScope !== 'function') return null; + const cube = query.cube ? ctx.getCube(query.cube) : undefined; + if (!cube) return null; + const objects = [this.extractObjectName(cube)]; + for (const alias of Object.keys(cube.joins ?? {})) { + objects.push(cube.joins?.[alias]?.name ?? alias); + } + for (const objectName of objects) { + const scope = ctx.getReadScope(objectName); + if (scope === undefined || scope === null) continue; + const inScope = findCrossFieldComparand(scope); + if (inScope) return { source: `the read scope of "${objectName}"`, ...inScope }; + } + return null; + } + + /** + * [#7598] The fail-closed backstop at the door that BINDS. + * + * ⚠️ **Unreachable by construction, and kept deliberately** — saying so + * because #7598's brief asks that a refusal arm which has become unreachable + * be named rather than left to be re-discovered. {@link canHandle} declines + * every query this would fire on, and it declines using + * {@link crossFieldComparisonIn} — the same walk over the same two inputs — + * so `resolveStrategy` cannot hand this strategy a query carrying one. + * + * It is kept because of what the failure mode is if that ever stops being + * true. The defect #7598 measured was not a missing error: it was a SILENT + * BIND — `toSqlBindValue` JSON-stringifies the reference object, so the + * statement compiled perfectly and compared a column against the text + * `{"$field":"budget"}`, a value no row can hold. A routing gate that misses + * a shape therefore degrades to a wrong ANSWER rather than to an error, and + * that is the one class this package refuses to leave to a single guard + * (Prime Directive #12 — refuse at the door, do not tolerate at the + * consumer). One line, no measurable cost, and it turns a routing regression + * into a loud refusal instead of an empty chart. + * + * Deliberately BARE — an undeclared 500, not `INVALID_FILTER` / 400 — for the + * reason `buildFilterClauseSql`'s #5333 exit in `objectql-strategy.ts` gives + * for the same class: the caller's filter is legal and is served on the + * engine path, so an arrival here is drift between our own routing gate and + * our own emitter. Billing the caller 400 for that would hide a platform bug + * from 5xx alerting and tell a dashboard user to fix a filter that is fine. + * Same tier as `resolveMeasureSql`'s unrecognised-`Metric.type` throw below. + */ + private assertNoCrossFieldComparison(query: AnalyticsQuery, ctx: StrategyContext): void { + const hit = this.crossFieldComparisonIn(query, ctx); + if (!hit) return; + throw new Error( + `[native-sql-strategy] ${hit.source} carries a field reference ` + + `{ "$field": "${hit.ref}" } under "${hit.op}" on "${hit.field}", which this strategy does not ` + + `compile into a column-to-column comparison — it would BIND the reference object as the ` + + `comparison's value and answer a wrong row set silently (#7598). \`canHandle\` declines such a ` + + `query so it routes to the ObjectQL/engine path, whose driver compiles it and enforces the ` + + `#5222 rulings with metadata it owns; reaching this throw means the decline and this emitter ` + + `stopped agreeing, which is our bug and must never degrade to a silent answer.`, + ); + } + async execute(query: AnalyticsQuery, ctx: StrategyContext): Promise { const { sql, params } = await this.generateSql(query, ctx); const cube = ctx.getCube(query.cube!)!; @@ -140,6 +280,10 @@ export class NativeSQLStrategy implements AnalyticsStrategy { throw new Error(`Cube not found: ${query.cube}`); } + // [#7598] Unreachable by construction — `canHandle` declined this query. + // See {@link assertNoCrossFieldComparison} for why it is asserted anyway. + this.assertNoCrossFieldComparison(query, ctx); + const params: unknown[] = []; const selectClauses: string[] = []; const groupByClauses: string[] = []; diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 475111713b..edb293ffba 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -4,12 +4,15 @@ import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contract import type { Cube } from '@objectstack/spec/data'; import type { AnalyticsStrategy, StrategyContext } from './types.js'; import { + invalidFilterError, + lowerAnalyticsWhere, normalizeAnalyticsFilterTree, collectFilterLeaves, SQL_CONST_FALSE, SQL_CONST_TRUE, type NormalizedFilterNode, } from './filter-normalizer.js'; +import { findCrossFieldComparand, isFieldReference } from '../comparand-shape.js'; import { compileScopedFilterToSql } from '../read-scope-sql.js'; import { invalidMemberError } from '../dataset-refusal.js'; import { likePattern, LIKE_ESCAPE_CHAR, asciiLowerSqlExpr, type LikeShape } from '../like-pattern.js'; @@ -240,6 +243,50 @@ export class ObjectQLStrategy implements AnalyticsStrategy { throw new Error(`Cube not found: ${query.cube}`); } + // [#7598, maintainer ruling 2026-08-12] The echo DECLINES a cross-field + // comparison — 「`/analytics/sql` 的 echo 同样 decline(一致的响亮答案, + // 不半渲染)」. + // + // This renderer describes an execution it does not perform, and there is no + // honest description of a cross-field comparison available to it. The + // reference reaches `engine.aggregate` intact and `driver-sql` compiles it + // into a TOTAL column-to-column predicate — several repetitions of both + // column expressions, so the answer matches the memory evaluator across + // NULLs. What this file's `buildFilterClauseSql` can render is `amount > + // $1` with the reference OBJECT in `params`: not a simplification of that + // predicate but a different one, comparing a column against a value no row + // can hold. Rendering it would hand a debugger SQL that reproduces NONE of + // the rows the query returned — the #3601 / #3602 / #3650 failure this + // whole render block exists to prevent, in its worst direction. + // + // Note what this does NOT affect: `execute()` calls `generateSql` inside a + // `try`/`catch` precisely because the echo is a debugging aid that must + // never fail a query that already ran, so `/analytics/query` still serves + // these queries and returns rows — the response simply carries no `sql` + // string. Only the dry-run face (`/analytics/sql`) refuses, which is the + // "one consistent, loud answer" the ruling asked for. + // + // The READ SCOPE half needs no arm of its own: `compileScopedFilterToSql` + // below still refuses a reference in its own fail-closed envelope + // (`READ_SCOPE_COMPILE_FAILED` / 500, #5367 ruling kept verbatim by Q2 = A), + // and that refusal is now reached from HERE rather than from + // `NativeSQLStrategy.applyReadScope` — see `read-scope-sql.ts`'s header. + const crossField = findCrossFieldComparand(this.loweredWhere(query)); + if (crossField) { + throw invalidFilterError( + `[analytics] cannot render display SQL for the field reference ` + + `{ "$field": "${crossField.ref}" } under "${crossField.op}" on "${crossField.field}". ` + + `The query itself is SERVED — \`NativeSQLStrategy.canHandle\` declines a cross-field ` + + `comparison so it routes to the ObjectQL engine path, where driver-sql compiles it into a ` + + `column-to-column predicate written TOTAL across NULLs and enforces the #5222 rulings ` + + `(#7598, maintainer ruling 2026-08-12). This renderer has no faithful rendering of that ` + + `predicate: what it can emit is a comparison against the reference object as a bound VALUE, ` + + `which reproduces none of the rows the query returns. Refusing rather than half-rendering — ` + + `an echo that contradicts execution is worse than no echo (#3601 / #3602 / #3650). Run the ` + + `query itself (/analytics/query) to get its rows.`, + ); + } + const selectParts: string[] = []; const groupByParts: string[] = []; const params: unknown[] = []; @@ -1101,7 +1148,26 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // side effect of converting; dropping the conversion must not drop the copy. const all = [...values]; switch (operator) { - case 'equals': return v0; + // [#7598] IMPLICIT equality for a literal, EXPLICIT `$eq` for a field + // reference — the branch is on the COMPARAND, not on the operator, which + // is the same fix and the same reasoning #7597 applied to + // `parseFilterAST`, the spec's own lowering sink. + // + // `{ amount: 5 }` is implicit equality and every backend reads it that + // way. `{ amount: { $field: 'budget' } }` is NOT: it is a field-spec + // object whose only key is `$field`, which no backend reads as an + // equality — `driver-sql` sees an unrecognised operator key and the + // memory evaluator sees a comparand it never resolves. So the bare return + // was correct for four years' worth of literals and silently wrong for + // the one comparand the 2026-08-12 ruling routes HERE on purpose: with it, + // `{ amount: { $eq: { $field: 'budget' } } }` — the shape + // `compileCelToFilter` emits for a field-to-field CEL rule, and the shape + // `canHandle` now declines native SQL for — would arrive at the driver as + // something the driver cannot read, so the capability B exists to serve + // would fail on its single most important spelling. Its five siblings + // (`$ne`/`$gt`/`$gte`/`$lt`/`$lte`) were never affected: they emit their + // operator explicitly two lines down. + case 'equals': return isFieldReference(v0) ? { $eq: v0 } : v0; case 'notEquals': return { $ne: v0 }; case 'gt': return { $gt: v0 }; case 'gte': return { $gte: v0 }; @@ -1157,6 +1223,25 @@ export class ObjectQLStrategy implements AnalyticsStrategy { return cube.sql.trim(); } + /** + * [#7598] The query's `where`, lowered — the same input + * `NativeSQLStrategy.canHandle` scans, so the strategy that DECLINED and the + * echo that refuses read one shape rather than two. + * + * A throw from the lowering is swallowed for the same reason it is there: the + * `where` is malformed either way and `normalizeAnalyticsFilterTree` below + * refuses it with the message and envelope it has always had. This helper's + * only job is finding a reference, and there is none to find in a filter that + * does not lower. + */ + private loweredWhere(query: AnalyticsQuery): unknown { + try { + return lowerAnalyticsWhere(query); + } catch { + return null; + } + } + /** * The dimensions this query PROJECTS, in the order the result carries them: * every `dimensions` entry, then every granular `timeDimensions` entry that diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e94514aa56..72b18ad9c2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2103,6 +2103,9 @@ importers: '@objectstack/driver-sql': specifier: workspace:* version: link:../../drivers/driver-sql + '@objectstack/driver-sqlite-wasm': + specifier: workspace:* + version: link:../../drivers/driver-sqlite-wasm '@types/node': specifier: ^26.1.2 version: 26.1.2 From 6d1d747deec7db59d81f6c42f7f8d8d04ce3422f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:21:20 +0000 Subject: [PATCH 2/3] fix(service-analytics): name the cross-field decline when no engine path exists (#7598) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decline creates one configuration that previously (wrongly) compiled: a host advertising `nativeSql` WITHOUT an aggregate bridge now reaches `resolveStrategy`'s dead end for a `$field`-carrying filter, and the bare "no strategy can handle this cube" reads like a broken driver when every other query on that cube still works. Measured: not reachable from `AnalyticsServicePlugin`, whose default `queryCapabilities` derives both flags from the bridges it wired and auto-wires the aggregate bridge from the engine — so only a host that overrides `queryCapabilities` by hand can hit it. Named anyway, with the narrowness control (a literal filter on the same deployment still runs). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EWcRLiMFvDoQV3zS2LgEHH --- .../cross-field-engine-fallback.test.ts | 32 ++++++++++++++ .../src/analytics-service.ts | 43 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts index 8faeaf05b7..6242ac2a2b 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts @@ -346,6 +346,38 @@ describe('[#7598] cross-field `$field` on the analytics face — served via the }); }); + // ── The one deployment where declining leaves nowhere to go ─────────────── + + it('a host with NO aggregate bridge gets a diagnostic naming the reference, not a driver hunt', async () => { + // The configuration this change creates: `nativeSql` advertised WITHOUT + // `objectqlAggregate`, so the decline has no lower-priority strategy to + // fall to. Not reachable from `AnalyticsServicePlugin` — its default + // capabilities derive both flags from the bridges it wired, and it + // auto-wires the aggregate bridge from the engine — so this is a host that + // overrode `queryCapabilities` by hand. It still deserves to be told which + // of its queries is affected and why, rather than a bare "no strategy can + // handle this cube" that reads like a broken driver. + const nativeOnly = new AnalyticsService({ + cubes: [CUBE], + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [], + }); + const run = (where?: unknown) => + nativeOnly.query({ + cube: 'deals', dimensions: ['id'], measures: ['n'], + ...(where ? { where } : {}), + } as AnalyticsQuery); + + const err = await errorFrom(() => run({ amount: { $gt: { $field: 'budget' } } })); + expect(err.message).toContain('budget'); + expect(err.message).toContain('executeAggregate'); + expect(err.message).toContain('#7598'); + + // …and the narrowness control, which is the half that makes the sentence + // trustworthy: a literal filter on the SAME deployment still runs. + await expect(run({ amount: { $gt: 5 } })).resolves.toBeDefined(); + }); + // ── `/analytics/sql` — the echo declines, loudly, with no half-rendering ─── describe('the `/analytics/sql` echo declines rather than half-rendering', () => { diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index bc4644893f..369992e432 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -34,6 +34,7 @@ import { lowerAnalyticsWhere, conjunctFieldKeys, } from './strategies/filter-normalizer.js'; +import { findCrossFieldComparand } from './comparand-shape.js'; import { compileDataset, type CompiledDataset, type RelationshipResolver } from './dataset-compiler.js'; import { DatasetExecutor, resolveDimensionGranularity, type DateGranularityValue } from './dataset-executor.js'; import { @@ -1944,14 +1945,56 @@ export class AnalyticsService implements IAnalyticsService { return strategy; } } + // [#7598] Name the one decline that is about the QUERY rather than about + // the deployment. Since the 2026-08-12 ruling `NativeSQLStrategy` declines + // a cross-field `{ $field }` comparison so it routes to the engine path — + // so a host advertising `nativeSql` WITHOUT an aggregate bridge now reaches + // this exit for a filter it used to (wrongly) compile. The bare message + // below would send that operator off to check their driver configuration, + // which is not the problem: every other query on that cube still works. + // + // Reachability, measured: NOT from `AnalyticsServicePlugin`, whose default + // `queryCapabilities` derives BOTH flags from the bridges it wired + // (`objectqlAggregate: !!executeAggregate`) and auto-wires the aggregate + // bridge from the engine — so a real deployment that has `nativeSql` has + // `objectqlAggregate` too. Reachable only from a host that overrides + // `queryCapabilities` by hand. Cheap to say, and the alternative is a dead + // end that reads like a misconfiguration. + const crossField = findCrossFieldComparand(lowerAnalyticsWhereQuietly(query)); throw new Error( `[Analytics] No strategy can handle query for cube "${query.cube}". ` + `Checked: ${this.strategies.map(s => s.name).join(', ')}${skip?.size ? ` (skipped at runtime: ${[...skip].map((s) => s.name).join(', ')})` : ''}. ` + + (crossField + ? `This query's filter compares against the field reference ` + + `{ "$field": "${crossField.ref}" } under "${crossField.op}" on "${crossField.field}", and ` + + `NativeSQLStrategy DECLINES a cross-field comparison so that it routes to the ObjectQL ` + + `engine path — whose driver compiles it and enforces the #5222 rulings with metadata it ` + + `owns (#7598). No such path is configured here, so the capability is unavailable on this ` + + `deployment: supply an \`executeAggregate\` bridge (the plugin auto-wires one from the ` + + `engine), or compare against a literal value. Every other query on this cube is ` + + `unaffected. ` + : '') + 'Ensure a compatible driver is configured or a fallback service is registered.', ); } } +/** + * [#7598] A query's `where`, lowered — the same input the strategies scan, so + * the diagnostic above describes the filter the decline actually saw. + * + * Quiet by construction: a `where` that cannot even be lowered is refused + * downstream with its own message and envelope, and there is no reference to + * find in one that does not lower. + */ +function lowerAnalyticsWhereQuietly(query: AnalyticsQuery): unknown { + try { + return lowerAnalyticsWhere(query); + } catch { + return null; + } +} + /** * [#5918] The `cube.measures` KEY a request's `measures` entry may be MINTED * under — or a loud refusal when the entry is a dotted member. From 94c5580c03dce6564c70f93744cc55939ffad41d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:29:24 +0000 Subject: [PATCH 3/3] test(service-analytics): type the aggregate bridge instead of casting it away (#7598) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine-fallback suite's `executeAggregate` bridge passed its query as `as any` to work around one key: the analytics contract types a measure's `method` as a plain string while `AggregationNode.function` is the closed `AggregationFunction` enum. That erased the whole options object and moved `check:query-options-erasure`'s test surface 242 -> 243. The query is now typed `DriverQuery` and only `method` is cast, to the one type it has to be. Every other key stays checked — which is the point of the ratchet, and better for a bridge whose whole job is to stand in for the real one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EWcRLiMFvDoQV3zS2LgEHH --- .../cross-field-engine-fallback.test.ts | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts index 6242ac2a2b..d62040da63 100644 --- a/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts +++ b/packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts @@ -66,8 +66,8 @@ import { CROSS_FIELD_ROWS, } from '@objectstack/driver-sql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; -import type { Cube, FilterCondition } from '@objectstack/spec/data'; -import type { AnalyticsQuery } from '@objectstack/spec/contracts'; +import type { AggregationNode, Cube, FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery, DriverQuery } from '@objectstack/spec/contracts'; import { AnalyticsService } from '../analytics-service.js'; import { findCrossFieldComparand } from '../comparand-shape.js'; @@ -119,21 +119,29 @@ describe('[#7598] cross-field `$field` on the analytics face — served via the // `aggregate`, which is what that engine call reaches. The filter is // forwarded VERBATIM — no normalisation, no stringification — because the // claim under test is that the reference survives this hop intact. - executeAggregate: async (objectName, options) => - (await driver.aggregate(objectName, { + executeAggregate: async (objectName, options) => { + // `{field, method, alias}` → `{field, function, alias}`: the analytics + // strategy speaks the contract's `method`, the Query Protocol's + // `AggregationNodeSchema` spells it `function`, and `engine.aggregate` + // is what renames it in production. Mapped here rather than worked + // around, so the bridge stays the shape a real host writes. + // + // The query is typed `DriverQuery` — not `as any` — so this bridge stays + // inside the contract it is standing in for. Only `method` needs a cast, + // because the analytics contract types it a plain `string` while + // `AggregationNode.function` is the closed `AggregationFunction` enum; + // narrowing the cast to that one field keeps every other key checked. + const query: DriverQuery = { where: options.filter as FilterCondition, groupBy: options.groupBy, - // `{field, method, alias}` → `{field, function, alias}`: the analytics - // strategy speaks the contract's `method`, the Query Protocol's - // `AggregationNodeSchema` spells it `function`, and `engine.aggregate` - // is what renames it in production. Mapped here rather than worked - // around, so the bridge stays the shape a real host writes. aggregations: options.aggregations?.map(({ field, method, alias }) => ({ field, - function: method, + function: method as AggregationNode['function'], alias, })), - } as any)) as Record[], + }; + return (await driver.aggregate(objectName, query)) as Record[]; + }, getReadScope: () => readScope ?? undefined, }); });