Skip to content

Commit 13a3dca

Browse files
os-warrenclaude
andauthored
fix(service-analytics): refuse a cross-object dataset-level filter on both ObjectQL doors (#10861) (#11066)
PR #10758 gave a compiled dataset's definition-level `filter` a route onto the ObjectQL door for the first time. That route was outside the member view `planCrossObject` judges, so a dataset declaring `filter: { 'account.region': 'West' }` was accepted by BOTH doors and `engine.aggregate` received `{"$and":[{"account.region":"West"}]}` — a predicate it cannot join. `account.region` is not a column of `opportunity`, so on any driver that evaluates it honestly the predicate matches nothing and the widget answered a number that was neither the scoped number nor the unscoped one, with no error anywhere. #10759 was not this card: the two doors AGREED in accepting it, so there was no preview/execution divergence to restore, and refusing it is a new decision about the refusal set. Ruled by the maintainer on 2026-08-22 (decision-inbox digest, accepted verbatim 「接受所有」): Option A — refuse at query time. `filterMemberView` now folds the dataset scope's leaves into the one view both doors are judged on, so the refusal fires where driver capability is known. Compile-time rejection in `dataset-compiler.ts` was not taken: the compiler cannot see which driver will serve the dataset, and the same definition is legal on a native-SQL deployment. The refusal keeps the `invalidMemberError` family (INVALID_FIELD / 400) — the same physical verdict as its neighbour, so a caller need not branch on two wire shapes for one capability limit — and carries `member` + `cube`. `param` is deliberately absent: `AnalyticsRequestKey` is the request vocabulary, `AnalyticsQuerySchema` is strict and has no `filter` key, and the request may carry no `where` at all, so both `'where'` and a widened `'filter'` would name a key the caller cannot go and edit. The as-is pin #10759 left in `crossobject-conjunct-refusal.test.ts` went red as designed; it is flipped to the new behaviour and its explanatory paragraph is rewritten, so the next reader sees the refusal is intended rather than a leftover explanation of a defect that no longer exists. Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 07bd1ca commit 13a3dca

3 files changed

Lines changed: 376 additions & 61 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
**BREAKING**: on the ObjectQL path, a compiled dataset whose definition-level
6+
`filter` is itself cross-object is now refused by both analytics doors instead
7+
of reaching `engine.aggregate` with a predicate it cannot join (#10861).
8+
9+
PR #10758 gave the dataset's own definition-level `filter` a route onto this
10+
door for the first time. That route was outside the member view the cross-object
11+
envelope check judges, so nothing ever saw it:
12+
13+
```
14+
dataset: object 'opportunity', include: ['account'],
15+
filter: { 'account.region': 'West' }
16+
17+
before /analytics/query 200, rows -> engine.aggregate received
18+
{"$and":[{"account.region":"West"}]}
19+
/analytics/sql 200, SQL
20+
after both 400 INVALID_FIELD, member "account.region",
21+
cube "<dataset>"; the engine is never reached
22+
```
23+
24+
`engine.aggregate` cannot join. `account.region` is not a column of
25+
`opportunity`, so on any driver that evaluates the predicate honestly it matches
26+
nothing, and the widget answered a number that was neither the scoped number nor
27+
the unscoped one — with no error anywhere. That is the silent mis-bucket #3654's
28+
loud refusal exists to prevent, arriving through a producer #3654 predates.
29+
30+
**Breaking, and argued rather than assumed.** A query that returns `200` with
31+
rows today starts answering `400`, on a *saved* dataset rather than on anything
32+
in the request — a dashboard that renders today can start showing an error. That
33+
is the strongest reading of "breaking" and it is why this is called out here
34+
rather than filed as a quiet fix. What is *not* lost is any correct answer: the
35+
rows that stop being served were already wrong, and wrong in the way that hides
36+
itself. The refusal names the member, names the dataset, and says the same
37+
definition is valid on a native-SQL deployment, so the operator has somewhere to
38+
go; the previous behaviour gave them a plausible number and nothing to notice.
39+
Rejecting the dataset at compile time in `dataset-compiler.ts` was considered and
40+
not taken (maintainer ruling, 2026-08-22): the compiler cannot see which driver
41+
will serve the dataset, and the same definition is legal on a native-SQL one.
42+
43+
Who is affected: a deployment whose driver reports `objectqlAggregate` but not
44+
`nativeSql` (Mongo, the memory driver), serving a dataset whose definition-level
45+
`filter` names a field on a related object. Nothing an author writes changes
46+
shape, no stored document is rewritten, and an **ordinary** dataset scope
47+
(`filter: { is_deleted: false }`) still passes both doors and still reaches the
48+
engine carrying its predicate — that direction is pinned one character away from
49+
the new refusal in `crossobject-conjunct-refusal.test.ts`, because an
50+
implementation that refused *every* dataset scope would look identical from the
51+
refusal side alone and would break every scoped dataset shipping today.
52+
53+
<!-- adr-0087: not-required (no-migration-prescription) No authorable surface is
54+
retired, renamed or re-shaped: `DatasetSchema`'s `filter` key stays exactly as it
55+
is, every stored dataset document stays valid as written, and the very same
56+
document remains correct on a native-SQL deployment. There is therefore nothing
57+
`objectstack migrate meta` could rewrite — a mechanical rewrite would have to
58+
know which driver will serve the dataset, which is precisely the capability the
59+
2026-08-22 ruling records as invisible to the compile-time placement. This is a
60+
query-time refusal on one driver family, not a surface retirement, so the ledger
61+
has no entry to carry and the upgrade guide has no prescription to print. -->

packages/services/service-analytics/src/__tests__/crossobject-conjunct-refusal.test.ts

Lines changed: 192 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,27 @@
3131
* stated the invariant it was breaking: *"`generateSql()` calls this too, so the
3232
* preview accepts/rejects the same set."*
3333
*
34-
* ## Why this file pins FOUR directions, not one
34+
* ## [#10861] The second producer, and why this file now covers both
3535
*
36-
* Pinning only the new refusal would go green on an implementation that refuses
37-
* every combinator — which would break every legitimate `$or` query shipping
38-
* today. So the accepting neighbours are pinned in the same file, one character
39-
* away from the refused ones:
36+
* The caller's `where` is not the only thing that puts a predicate in front of
37+
* `engine.aggregate` here. PR #10758 gave the compiled dataset's OWN
38+
* definition-level `filter` a route onto this door, and that route was outside
39+
* both member views too — so a dataset declaring
40+
* `filter: { 'account.region': 'West' }` was accepted by BOTH doors and the
41+
* engine received a predicate it cannot join. #10759 was not that card: the two
42+
* doors AGREED there, so there was no divergence to restore, and refusing it
43+
* was a new decision about the refusal set. It was taken (maintainer,
44+
* 2026-08-22: Option A, refuse at query time) and #10861 folds the scope's
45+
* leaves into the same one member view. Both producers are now judged by one
46+
* check, which is why they are pinned in one file.
47+
*
48+
* ## Why this file pins SIX directions, not one
49+
*
50+
* Pinning only the new refusals would go green on an implementation that
51+
* refuses every combinator, or every dataset scope — which would break every
52+
* legitimate `$or` query and every scoped dataset shipping today. So the
53+
* accepting neighbours are pinned in the same file, one character away from the
54+
* refused ones:
4055
*
4156
* ① a cross-object member nested in `$or` / `$not` is REFUSED on both doors
4257
* ② a combinator with NO cross-object member still passes both doors, and
@@ -47,14 +62,13 @@
4762
* misread as a cross-object reference — an ordinary definition-level scope
4863
* travels in `$and` exactly like a combinator does, and reads as a member
4964
* of nothing
65+
* ⑤ a CROSS-OBJECT dataset-level `filter` is REFUSED on both doors, in the
66+
* ADR-0112 envelope, before `engine.aggregate` is reached (#10861)
67+
* ⑥ an ORDINARY dataset-level `filter` still reaches the engine CARRYING its
68+
* predicate — the load-bearing half of ⑤, and the pin a
69+
* "refuse every dataset scope" implementation fails
5070
*
51-
* ## The scope line this file also draws
52-
*
53-
* A dataset whose DEFINITION-LEVEL filter is itself cross-object is accepted by
54-
* BOTH doors, before and after this change — neither call site's view contains
55-
* the dataset scope. The doors AGREE there, so it is not the invariant this card
56-
* restores; it is a separate defect and is pinned here as measured-and-known
57-
* rather than left to be rediscovered. See the last block.
71+
* ①–④ are #10759's and are re-run unchanged here; ⑤–⑥ are #10861's.
5872
*/
5973

6074
import { describe, it, expect } from 'vitest';
@@ -65,7 +79,7 @@ import { AnalyticsService } from '../analytics-service.js';
6579

6680
const ctxA = { tenantId: 'org_A', userId: 'u_a' } as ExecutionContext;
6781

68-
interface Refusal extends Error { code?: string; status?: number; member?: string; param?: string }
82+
interface Refusal extends Error { code?: string; status?: number; member?: string; param?: string; cube?: string }
6983
interface AggCall { object: string; filter?: unknown }
7084

7185
/** A cube with one base dimension, one cross-object dimension, one base measure. */
@@ -108,6 +122,21 @@ const XOBJ_SCOPED_SALES: Dataset = DatasetSchema.parse({
108122
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
109123
}) as Dataset;
110124

125+
/**
126+
* [#10861] A dataset scope with a base leaf AND a cross-object leaf. Serving is
127+
* not a property of a scope containing a base leaf — the cross-object one
128+
* decides, wherever it sits.
129+
*/
130+
const MIXED_SCOPED_SALES: Dataset = DatasetSchema.parse({
131+
name: 'mixed_scoped_sales',
132+
label: 'Mixed scoped sales',
133+
object: 'opportunity',
134+
include: ['account'],
135+
filter: { is_deleted: false, 'account.region': 'West' },
136+
dimensions: [{ name: 'stage', field: 'stage', type: 'string' }],
137+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
138+
}) as Dataset;
139+
111140
/**
112141
* `nativeSql: false` makes `NativeSQLStrategy` decline, so every query below
113142
* routes to `ObjectQLStrategy` — the door this card is about.
@@ -150,6 +179,15 @@ async function bothDoors(cube: string, query: Omit<AnalyticsQuery, 'cube'>, defs
150179
}
151180

152181
const CROSS_OBJECT_MESSAGE = /cannot evaluate a cross-object filter \("account\.region"\)/;
182+
/**
183+
* [#10861] A DISTINCT message, deliberately: the four refusals that predate
184+
* this card keep their wording (#5923's tests read it), and this one has to say
185+
* a different thing — the member is in a saved document, not in the request.
186+
* Matching on the substring that carries that distinction, so a rewording that
187+
* dropped the provenance would go red.
188+
*/
189+
const DATASET_SCOPE_MESSAGE =
190+
/cannot evaluate the cross-object filter \("account\.region"\) that dataset "[^"]+" declares at its definition level/;
153191

154192
// ─────────────────────────────────────────────────────────────────────────────
155193
// ① the refusal that was missing on the execution door
@@ -281,29 +319,148 @@ describe('[#10759] the #10413-phase-1 dataset filter conjunct is not misread', (
281319
expect(calls).toEqual([]);
282320
});
283321

284-
/**
285-
* MEASURED AND DELIBERATELY LEFT OPEN — not a latent pass.
286-
*
287-
* A cross-object DEFINITION-LEVEL filter is accepted by both doors, and
288-
* `engine.aggregate` receives `{"$and":[{"account.region":"West"}]}`, which it
289-
* cannot join. PR #10758 created this instance by giving the dataset scope a
290-
* route onto the ObjectQL door at all; #10759 is not it, because the two doors
291-
* AGREE here — neither call site's member view contains the dataset scope, so
292-
* there is no preview/execution divergence to restore.
293-
*
294-
* Filed separately rather than widened into this PR: refusing it is a real
295-
* decision (query-time refusal versus a compile-time rejection in
296-
* `dataset-compiler.ts`, which is the contract-first placement), and it is not
297-
* the invariant this file restores. The expectation below is written to the
298-
* behaviour as it IS, so the day that decision lands this pin goes red and
299-
* points at the paragraph explaining why.
300-
*/
301-
it('a CROSS-OBJECT definition-level filter is still accepted by both doors (filed separately)', async () => {
302-
const { execute, generateSql, calls } = await bothDoors('xobj_scoped_sales', {
322+
});
323+
324+
// ────────────────────────────────────────────────────────────────────────────
325+
// ⑤ [#10861] the CROSS-OBJECT dataset scope — the second producer
326+
// ────────────────────────────────────────────────────────────────────────────
327+
328+
/**
329+
* THE REFUSAL IS INTENDED. This block replaces the as-is pin #10759 left here.
330+
*
331+
* What that pin recorded, and what it was for: a dataset whose DEFINITION-LEVEL
332+
* `filter` is itself cross-object was accepted by BOTH doors, and
333+
* `engine.aggregate` received `{"$and":[{"account.region":"West"}]}` — a
334+
* predicate it cannot join, so it matches nothing on any driver that evaluates
335+
* it honestly and the widget answers neither the scoped number nor the unscoped
336+
* one. #10759 was not that card: the two doors AGREED in accepting it, so there
337+
* was no preview/execution divergence to restore. It was written to the
338+
* behaviour as it WAS, so that the day the decision landed it would go red and
339+
* point at its own explanation. It did exactly that; this is the rewrite it
340+
* asked for.
341+
*
342+
* The decision (maintainer, 2026-08-22 decision-inbox digest, accepted verbatim
343+
* 「接受所有」): **Option A — refuse at query time.** The dataset scope's
344+
* leaves are folded into the one member view `planCrossObject` judges, so both
345+
* doors refuse at the moment the engine would otherwise be misled. Compile-time
346+
* rejection in `dataset-compiler.ts` (Option B) was NOT taken — the compiler
347+
* cannot see which driver will serve the dataset, and this same definition is
348+
* legal on a native-SQL deployment — and serving the shape (Option C) is
349+
* deferred until a real customer dataset is found to depend on it.
350+
*
351+
* Measured on the merged tree, one fixture, both doors in one run:
352+
*
353+
* ```
354+
* BEFORE execute() ACCEPTED -> engine.aggregate got
355+
* {"$and":[{"account.region":"West"}]}
356+
* generateSql() ACCEPTED
357+
* AFTER execute() REFUSED INVALID_FIELD / 400, member "account.region"
358+
* generateSql() REFUSED same
359+
* engine.aggregate never reached (0 calls)
360+
* ```
361+
*
362+
* ## Why the still-served neighbour below is load-bearing
363+
*
364+
* "Refuse the dataset scope" has a trivially green wrong implementation:
365+
* refuse EVERY dataset scope. It would pass a refusal-only suite and break
366+
* every scoped dataset shipping today. The ordinary-scope case is therefore
367+
* pinned one character away from the refused one, reaching the engine and
368+
* CARRYING its predicate — dropping the scope silently widens the answer just
369+
* as badly as refusing it wrongly narrows the product.
370+
*/
371+
describe('[#10861] a CROSS-OBJECT definition-level filter is refused on BOTH doors', () => {
372+
it('execute() refuses with the ADR-0112 envelope, before the engine is asked', async () => {
373+
const { execute, calls } = await bothDoors('xobj_scoped_sales', {
374+
dimensions: ['stage'], measures: ['revenue'],
375+
}, [XOBJ_SCOPED_SALES]);
376+
377+
expect(execute, 'accepted — the dataset scope was invisible to the envelope check')
378+
.toBeInstanceOf(Error);
379+
// Read exactly as `rest-server.ts`'s catch reads them: a 4xx status AND a
380+
// code, or the route falls through to 500 ANALYTICS_QUERY_FAILED. Asserting
381+
// only that it throws would pass on a bare `Error` and report the platform
382+
// broken for what is a dataset-authoring mistake on this deployment.
383+
expect(execute?.code, 'no `code` ⇒ 500 ANALYTICS_QUERY_FAILED').toBe('INVALID_FIELD');
384+
expect(execute?.status, 'no `status` ⇒ 500 ANALYTICS_QUERY_FAILED').toBe(400);
385+
// The member as the DATASET spelled it, and the dataset named as the
386+
// document to go and edit.
387+
expect(execute?.member).toBe('account.region');
388+
expect(execute?.cube).toBe('xobj_scoped_sales');
389+
expect(String(execute?.message)).toMatch(DATASET_SCOPE_MESSAGE);
390+
// `param` is ABSENT on purpose, and this assertion is the pin on that
391+
// choice rather than an omission nobody noticed. `AnalyticsRequestKey` is
392+
// the analytics REQUEST vocabulary; `AnalyticsQuerySchema` is strict and has
393+
// no `filter` key, and this request carries no `where` at all. Both
394+
// `param: 'where'` and a widened `param: 'filter'` would send the caller to
395+
// a key that does not exist on what they sent. `cube` is the locator that
396+
// does.
397+
expect(execute?.param, '`param` must not name a key the request has no room for')
398+
.toBeUndefined();
399+
// Refused BEFORE the engine was asked, not after it mis-bucketed. This is
400+
// the assertion the whole card is about: the BEFORE run reached it once,
401+
// carrying `{"$and":[{"account.region":"West"}]}`.
402+
expect(calls, 'engine.aggregate was reached — it cannot join').toEqual([]);
403+
});
404+
405+
it('both doors agree', async () => {
406+
const { execute, generateSql } = await bothDoors('xobj_scoped_sales', {
303407
dimensions: ['stage'], measures: ['revenue'],
304408
}, [XOBJ_SCOPED_SALES]);
305-
expect(execute).toBeUndefined();
306-
expect(generateSql).toBeUndefined();
307-
expect(JSON.stringify(calls[0]?.filter)).toContain('account.region');
409+
// One fact about one query, not two independent expectations — the same
410+
// shape ① uses. Two expectations that happen to coincide do not pin an
411+
// invariant BETWEEN two call sites; this does.
412+
expect(
413+
[execute === undefined, generateSql === undefined],
414+
'the preview and the execution door accept/reject the same set',
415+
).toEqual([false, false]);
416+
expect(generateSql?.code).toBe('INVALID_FIELD');
417+
expect(generateSql?.status).toBe(400);
418+
expect(String(generateSql?.message)).toMatch(DATASET_SCOPE_MESSAGE);
419+
});
420+
421+
it('the still-served neighbour: an ORDINARY dataset scope still reaches the engine carrying its predicate', async () => {
422+
// LOAD-BEARING. An implementation that refused every dataset scope would go
423+
// green on the two tests above and break every scoped dataset shipping
424+
// today. `is_deleted: false` is one character away from `account.region`
425+
// in the fixture and travels the identical `$and` conjunct route.
426+
const { execute, generateSql, calls } = await bothDoors('scoped_sales', {
427+
dimensions: ['stage'], measures: ['revenue'],
428+
}, [SCOPED_SALES]);
429+
expect(
430+
[execute === undefined, generateSql === undefined],
431+
'both doors must still SERVE an ordinary dataset scope',
432+
).toEqual([true, true]);
433+
expect(calls, 'the engine was not reached at all — the scope was refused, not served')
434+
.toHaveLength(1);
435+
expect(
436+
JSON.stringify(calls[0]?.filter),
437+
'reached the engine with the scope DROPPED — silently wider, not refused',
438+
).toContain('is_deleted');
439+
});
440+
441+
it('a base-object leaf beside a cross-object one in the SAME dataset scope is still refused', async () => {
442+
// The counter-shape for the test above: refusing is not a property of the
443+
// scope having more than one leaf, and serving is not a property of it
444+
// having a base leaf anywhere in it. The cross-object leaf decides.
445+
const { execute, generateSql, calls } = await bothDoors('mixed_scoped_sales', {
446+
dimensions: ['stage'], measures: ['revenue'],
447+
}, [MIXED_SCOPED_SALES]);
448+
expect([execute === undefined, generateSql === undefined]).toEqual([false, false]);
449+
expect(execute?.member).toBe('account.region');
450+
expect(calls).toEqual([]);
451+
});
452+
453+
it('the KNOWN-PRESENT control: a cross-object member in the CALLER\u2019s where keeps its own diagnostic', async () => {
454+
// The counter-check for every "refused" above, and the pin that #10861 did
455+
// not repaint the refusal #10759 restored. This shape was refused before
456+
// this card and is refused after it, with the OTHER message and with
457+
// `param: 'where'` — which is exactly what makes the absent `param` above a
458+
// deliberate distinction rather than a field this file forgot to set.
459+
const { execute, generateSql } = await bothDoors('sales_by_account', {
460+
dimensions: ['stage'], measures: ['revenue'], where: { 'account.region': 'West' },
461+
}, [SALES_BY_ACCOUNT]);
462+
expect(String(execute?.message)).toMatch(CROSS_OBJECT_MESSAGE);
463+
expect(String(generateSql?.message)).toMatch(CROSS_OBJECT_MESSAGE);
464+
expect(execute?.param).toBe('where');
308465
});
309466
});

0 commit comments

Comments
 (0)