Skip to content

Commit edff010

Browse files
os-zhuangclaude
andauthored
fix(filter): refuse a where on a virtual formula field at both doors, instead of answering 200 with zero rows (#8296) (#8369)
* fix(filter): refuse a where on a virtual formula field at both doors (#8296) The FILTER axis was the last of the three query axes with no unmaterializable verdict: a `where` on a `formula` field cleared `assertFilterFieldsExist` because the field IS known, reached a driver that materialises no column for it, and answered 200 with zero rows in BOTH directions — while SORT (#6994/#7095) and SEARCH (#6674) refuse the same field by name. Ingress: `assertFilterFieldsExist` grows a second verdict, judged by the same `@objectstack/spec/data` predicate the search axis uses (`isVirtualSearchField`), so gate and drivers cannot disagree about which types have a column. `summary`/`autonumber` keep filtering — both have real stored columns. Engine: `assertFilterIsMaterializable` closes the door the REST ingress cannot reach — a saved report forwards `query.filter` straight into `engine.find` — at `lowerWhereFilterArray`, the one seam every caller-supplied `where` passes through (find/findOne/count/aggregate/ update/delete). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH * chore: changeset for the filter-axis unmaterializable verdict (#8296) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH * test(app-todo): the derive-route reverse test now pins the refusal envelope (#8296) `derived-flag-removal.test.ts` registers a test-local formula-shaped object (`derived_task`, invented by that file) to record why #7226 removed two inert flags rather than deriving them, and it pinned the exact behaviour #8296 abolishes: filtering a formula answering 0 rows with no error. Its three filtering assertions now assert the rejection envelope (status 400, INVALID_FIELD, field, object) instead of an empty array, and the `it` title no longer claims "0 rows, no error". #7226's decision is unchanged and its reasoning is stronger: a formula field still materialises no column and still cannot carry a predicate, so the eight app filters that named those flags still could not have worked. Only the failure mode changed, from an invisible zero to a named 400 -- which is the exception this very docblock had named as the safe design. Both docblocks are rewritten to state that. The read/projection half (a formula COMPUTES both flags correctly) and the stored-column CONTROL assertions are untouched; nothing under examples/app-todo/src/ or objectstack.config.ts is touched, and that app declares no formula field at all. The changeset's blast-radius sentence is corrected in the same commit: the original sweep covered app source and missed test files, which is where current behaviour is pinned and therefore where a behaviour change lands first. No app metadata filters a formula field -- that half held. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e04899 commit edff010

6 files changed

Lines changed: 654 additions & 41 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
"@objectstack/objectql": minor
4+
---
5+
6+
fix(filter): a `where` on a virtual `formula` field is refused, not answered with zero rows (#8296)
7+
8+
`formula` is the one field type no driver materialises a column for. Three query
9+
axes can name a field; until now only two of them said so.
10+
11+
| axis | verdict for a `formula` field |
12+
|:---|:---|
13+
| SORT | `400 INVALID_SORT`, ingress (#6994) and engine (#7095) |
14+
| SEARCH | `400 INVALID_FIELD`, refused by name (#6674) |
15+
| **FILTER** | **accepted — 200, 0 rows, no error** |
16+
17+
`assertFilterFieldsExist` computed exactly one verdict — is this name a field of
18+
the object — and a `formula` field IS one, so the predicate cleared the door and
19+
reached a driver with no column behind it. Measured on a real `ObjectQL`, with
20+
`is_open` a `formula` over the stored `status` column:
21+
22+
```
23+
where { is_open: true } -> 0 rows, no error
24+
where { is_open: false } -> 0 rows, no error
25+
CONTROL where { status: 'open' } -> 4 rows
26+
CONTROL where { subtask_total: 5 } -> 1 row (`summary` HAS a column)
27+
```
28+
29+
Both directions are wrong and the `false` one is the dangerous one: the same
30+
predicate against a STORED boolean returns every row, so a filter meaning "not
31+
yet done" silently became "no records at all" — a row SET changed under a 200,
32+
which no amount of inspecting the response can reveal. The formula READS
33+
correctly in that very same response, so the field is visibly populated and
34+
simultaneously unfilterable.
35+
36+
Both doors now refuse it with `400 INVALID_FIELD`, naming the offending key path
37+
and prescribing the remedy the sort and search axes already share:
38+
39+
- **ingress**`assertFilterFieldsExist` grows a second verdict, after
40+
`unknown`, covering everything that reaches `findData`: the list route,
41+
`POST /data/:object/query`, the export route and the RPC dispatcher, in every
42+
filter spelling (`where` / `filter` / `filters` / `$filter`, the array sugar,
43+
and nested `$and` / `$or`);
44+
- **engine**`assertFilterIsMaterializable` closes the half the ingress cannot
45+
reach. It is author-reachable, not merely internal: a saved report's
46+
`query.filter` is forwarded verbatim into `engine.find`, exactly as #7095
47+
measured for `query.orderBy`. It runs at the engine's one filter-lowering
48+
seam, so `find`, `findOne`, `count`, `aggregate`, `update` and `delete` all
49+
answer alike, and it judges the CALLER's `where` only — a middleware-injected
50+
RLS or sharing predicate is the platform's own and is never refused.
51+
52+
Both doors judge the field by the same `@objectstack/spec/data` predicate the
53+
search axis uses (`isVirtualSearchField` / `SEARCH_VIRTUAL_TYPES`) rather than a
54+
locally minted type list, so a gate and the drivers cannot disagree about which
55+
types have a column.
56+
57+
**`summary` and `autonumber` are unaffected and still filter** — both get real
58+
stored columns; the set is exactly `formula`. Reading, projecting and computing a
59+
formula field are untouched; only the predicate is refused.
60+
61+
**What to change if this refuses one of your queries:** denormalise the value
62+
onto the object (a stored field, written when the source changes) and filter
63+
that. There is no mechanical rewrite in either direction — the platform cannot
64+
invent the stored column, and it must not filter post-hoc after the formulas are
65+
evaluated, because the driver has already applied `limit` / `offset`, so a
66+
post-hoc predicate would filter an arbitrary PAGE. Grep your saved reports,
67+
flows, dashboards and view filters for a filtered field whose object declares it
68+
as a `formula`.
69+
70+
**In-tree sweep — source AND tests.** No shipped example app's *metadata* filters
71+
a formula field: the ones the examples declare (`crm_contact.full_name`,
72+
`crm_opportunity.expected_revenue` / `days_to_close`, `crm_lead.is_closed`,
73+
`showcase_project.budget_remaining`, `showcase_field_zoo.f_formula`) appear only
74+
as view columns, form fields, permission entries and record-level CEL
75+
predicates — never in a `where` / `filter`. One in-tree TEST did filter one and
76+
is updated in this change: `examples/app-todo/test/derived-flag-removal.test.ts`
77+
registers a test-local formula-shaped object to record *why* two inert flags were
78+
removed rather than derived, and pinned the behaviour this refusal abolishes —
79+
filtering a formula answering 0 rows with no error. It now asserts the
80+
`400 INVALID_FIELD` envelope instead; its conclusion is unchanged, because a
81+
formula still cannot be filtered. The first sweep read app source only, which is
82+
the wrong half: current behaviour is pinned in tests, so a behaviour change lands
83+
there first.

examples/app-todo/test/derived-flag-removal.test.ts

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,24 @@
2121
* A formula computes both correctly — including the temporal one — so the
2222
* obvious repair looks available. It is not, and the reason is a STORAGE fact
2323
* rather than a taste judgment: a `formula` field is virtual, no driver
24-
* materialises a column for it, and so a FILTER naming one matches nothing.
25-
* That is measured here, not asserted — {@link REVERSE} registers the
26-
* formula-shaped object and shows `where { is_completed: false }` answering
27-
* **0 rows with no error** where the stored column answers every row. Deriving
28-
* would have silently emptied the "Due Today" view, the daily reminder flow and
29-
* both open-task reports: a wrong answer traded for an invisible one.
24+
* materialises a column for it, and so a FILTER naming one cannot be applied
25+
* as written. That is measured here, not asserted — {@link REVERSE} registers
26+
* the formula-shaped object and shows `where { is_completed: false }` failing
27+
* where the stored column answers every row. Deriving would have emptied the
28+
* "Due Today" view, the daily reminder flow and both open-task reports.
29+
*
30+
* Since **#8296** that failure is VISIBLE. The engine's filter seam refuses a
31+
* `where` naming a virtual `formula` field with `400 INVALID_FIELD` instead of
32+
* handing the predicate to a driver with no column behind it and answering
33+
* **0 rows with no error** — which is what this test measured when #7226 was
34+
* decided, and the invisible zero was the danger: a wrong answer traded for an
35+
* unobservable one.
36+
*
37+
* The storage fact that decided #7226 is unchanged, so the decision stands and
38+
* its reasoning is stronger, not weaker: a formula field still carries no
39+
* column, a filter naming one still could not have worked, and the eight app
40+
* filters that read these flags still had to move to stored columns. Only the
41+
* failure mode changed — a silent zero became a named 400.
3042
*
3143
* `status` and `due_date` are stored, indexed columns that already carry the
3244
* information, and both are declared dimensions on the `task_metrics` dataset,
@@ -229,10 +241,23 @@ describe('#7226 — the replacement filters really select, on BOTH sides of the
229241
* REVERSE VERIFICATION — the measurement that chose removal over derivation.
230242
*
231243
* Predicted direction, recorded BEFORE running it: the formula field READS
232-
* correctly (so "just derive it" looks right) but is UNFILTERABLE, and the
233-
* failure is silent — 0 rows, no error — rather than an exception. That
234-
* asymmetry is the whole argument: an exception would have been safe, because
235-
* someone would have seen it.
244+
* correctly (so "just derive it" looks right) but is UNFILTERABLE. When #7226
245+
* ran it the failure was silent — 0 rows, no error — rather than an exception,
246+
* and this docblock named that asymmetry as the whole argument: **an exception
247+
* would have been safe, because someone would have seen it.**
248+
*
249+
* **#8296 supplied that exception**, and the second `it` below therefore
250+
* asserts a rejection envelope (`400 INVALID_FIELD`, naming the field and the
251+
* object) where it used to assert an empty array. That is this file's own
252+
* argument being adopted platform-wide — the safe design it asked for is now
253+
* the shipped one — not a correction of it.
254+
*
255+
* The verdict on the derive route is UNCHANGED. A formula field still
256+
* materialises no column and still cannot carry a predicate, so the eight app
257+
* filters that named these flags still could not have worked; removal in
258+
* favour of the stored `status` / `due_date` columns remains the only repair.
259+
* What #8296 changed is that choosing the derive route now fails where someone
260+
* can see it, instead of quietly answering an empty set.
236261
*/
237262
describe('REVERSE — why the derive route was rejected, measured', () => {
238263
/** `todo_task` as it would look on the derive route. */
@@ -276,26 +301,39 @@ describe('REVERSE — why the derive route was rejected, measured', () => {
276301
expect(byId.d.is_overdue).toBe(false); // no due date at all
277302
});
278303

279-
it('...and is UNFILTERABLE: 0 rows, no error — which is why deriving was refused', async () => {
304+
it('...and is UNFILTERABLE: a `where` naming one is REFUSED, 400 INVALID_FIELD (#8296)', async () => {
280305
const ql = await bootEngine(DERIVED);
281306
await ql.insert('derived_task', { id: 'a', subject: 'done', status: 'completed', due_date: '2020-01-01' });
282307
await ql.insert('derived_task', { id: 'b', subject: 'late', status: 'in_progress', due_date: '2020-01-01' });
283308

284309
// A formula field materialises no column on any driver, so the predicate
285-
// matches nothing — and returns cleanly rather than throwing.
286-
expect(await ql.find('derived_task', { where: { is_completed: true } })).toEqual([]);
287-
expect(await ql.find('derived_task', { where: { is_overdue: true } })).toEqual([]);
310+
// cannot be applied as written. When #7226 measured this the engine handed
311+
// it to the driver anyway and answered 0 rows with no error; since #8296
312+
// the engine's filter seam refuses it by name. The full envelope is pinned,
313+
// not merely "it throws": a driver that happened to throw a bare `Error`
314+
// would satisfy a bare `.rejects` while proving nothing about the verdict.
315+
await expect(ql.find('derived_task', { where: { is_completed: true } })).rejects.toMatchObject({
316+
status: 400, code: 'INVALID_FIELD', field: 'is_completed', object: 'derived_task',
317+
});
318+
await expect(ql.find('derived_task', { where: { is_overdue: true } })).rejects.toMatchObject({
319+
status: 400, code: 'INVALID_FIELD', field: 'is_overdue', object: 'derived_task',
320+
});
288321

289322
// THE decisive one. On the old stored boolean this returned EVERY row; as a
290-
// formula it returns NONE. Eight filters in this app relied on exactly this
291-
// predicate ("Due Today", the reminder flow, both open-task reports, three
292-
// distribution charts), so the derive route would have silently emptied
293-
// every one of them.
294-
expect(await ql.find('derived_task', { where: { is_completed: false } })).toEqual([]);
323+
// formula it is not answerable at all. Eight filters in this app relied on
324+
// exactly this predicate ("Due Today", the reminder flow, both open-task
325+
// reports, three distribution charts), so the derive route would have
326+
// broken every one of them — before #8296 by silently emptying them, after
327+
// #8296 by failing loudly on the first query. Neither is a working app,
328+
// which is why these flags were removed rather than derived.
329+
await expect(ql.find('derived_task', { where: { is_completed: false } })).rejects.toMatchObject({
330+
status: 400, code: 'INVALID_FIELD', field: 'is_completed', object: 'derived_task',
331+
});
295332

296333
// CONTROL — the stored column answers correctly on the same rows and the
297-
// same engine, so the emptiness above is about the field being virtual, not
298-
// about the fixture or the driver.
334+
// same engine, so the refusal above is about the field being virtual, not
335+
// about the fixture or the driver. (Assertions unchanged from #7226: the
336+
// anti-vacuity arm never depended on the formula's failure mode.)
299337
expect((await ql.find('derived_task', { where: { status: 'completed' } })).map((r: any) => r.id)).toEqual(['a']);
300338
expect((await ql.find('derived_task', { where: { status: { $ne: 'completed' } } })).map((r: any) => r.id)).toEqual(['b']);
301339
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 105 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5866,9 +5866,29 @@ export class ObjectStackProtocolImplementation implements
58665866
*
58675867
* Value shapes are NOT judged here: a wrong-typed or unrunnable filter is
58685868
* `INVALID_FILTER`'s job (#4121 / #4181), already answered upstream in this
5869-
* same block. This gate answers exactly one question — does this field
5870-
* exist — with exactly the envelope the write path and the bare-key door
5871-
* already give it.
5869+
* same block. This gate answers questions about the NAME, with exactly the
5870+
* envelope the write path and the bare-key door already give it.
5871+
*
5872+
* [#8296] It answers TWO of them now — "does this field exist" and, second,
5873+
* "does this field's TYPE materialise a column to filter on". A `formula`
5874+
* field is known, undotted and unfilterable: it cleared this gate precisely
5875+
* BECAUSE the object declares it, reached a driver that has no column for
5876+
* it, and answered 200 with zero rows in BOTH directions. That was the last
5877+
* axis in this family still fail-open — SORT refuses the same field
5878+
* (#6994/#7095) and SEARCH refuses it (#6674) — and it is the shape the
5879+
* standing ruling of 2026-08-12 names: a declaration the platform cannot
5880+
* honour is refused at the latest checkpoint that can see the whole
5881+
* picture, naming the offending key path, never answered 200.
5882+
*
5883+
* SCOPE: this is an INGRESS gate, so it covers what reaches {@link
5884+
* findData}. The half it cannot reach — a caller handing a `where` straight
5885+
* to `engine.find` / `findOne` / `count` / `aggregate` / `update` /
5886+
* `delete`, which is how a saved report's `query.filter` travels
5887+
* (`plugin-reports` forwards it verbatim) — is closed at the engine's own
5888+
* filter seam by `assertFilterIsMaterializable` (`@objectstack/objectql`,
5889+
* `filter-comparand-shape.ts`), with the same `400 INVALID_FIELD` and the
5890+
* same remedy sentence. Same two-door shape, and same reason, as the sort
5891+
* axis' #7095.
58725892
*/
58735893
private assertFilterFieldsExist(object: string, where: unknown, param: string): void {
58745894
if (!where || typeof where !== 'object') return;
@@ -5878,20 +5898,92 @@ export class ObjectStackProtocolImplementation implements
58785898
if (!gate) return;
58795899
// Head segment only, exactly as the bare-key door judges `owner_id.name`.
58805900
const unknown = names.filter((f) => !gate.known.has(f.split('.')[0]));
5881-
if (unknown.length === 0) return;
5882-
const first = unknown[0];
5901+
if (unknown.length > 0) {
5902+
const first = unknown[0];
5903+
const err: any = new Error(
5904+
`Query parameter '${param}' filters on '${first}', which is not a field on object `
5905+
+ `'${object}'`
5906+
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
5907+
+ '. A filter on a field that does not exist can only match zero records, so the '
5908+
+ 'query was refused instead of answered with an empty list.'
5909+
+ suggestFieldName(first, gate.declared),
5910+
);
5911+
err.code = 'INVALID_FIELD';
5912+
err.status = 400;
5913+
err.field = first;
5914+
err.fields = unknown;
5915+
err.object = object;
5916+
err.param = param;
5917+
throw err;
5918+
}
5919+
5920+
// [#8296] The SECOND verdict on this axis: a name that is a REAL field
5921+
// of this object and still cannot be filtered on, because its TYPE
5922+
// materialises no column. It is the FILTER axis finally growing the
5923+
// verdict its two neighbours already have — {@link
5924+
// assertSortFieldsExist} splits `unknown` from unmaterializable
5925+
// (#6994) and {@link assertSearchFieldsAreSearchable} splits `unknown`
5926+
// from `virtual` (#6674) — and it was the last axis on which a
5927+
// declaration the platform cannot honour still answered 200.
5928+
//
5929+
// Measured on a real `ObjectQL` + this protocol, base cb43296ef
5930+
// (`is_open` a `formula` over the stored `status` column):
5931+
//
5932+
// ```
5933+
// where { is_open: true } -> 0 rows, NO ERROR
5934+
// where { is_open: false } -> 0 rows, NO ERROR
5935+
// CONTROL where { status: 'open' } -> 4 rows
5936+
// CONTROL where { subtask_total: 5 } -> 1 row (`summary` HAS a column)
5937+
// ```
5938+
//
5939+
// BOTH directions are wrong and the `false` one is the dangerous one:
5940+
// the same predicate against a STORED boolean returns every row, so a
5941+
// filter meaning "not yet done" silently becomes "no records at all".
5942+
// The response is indistinguishable from an empty table, and the
5943+
// formula READS correctly in that very same response (`applyFormulaPlan`
5944+
// hydrates it), so the field is visibly populated and simultaneously
5945+
// unfilterable.
5946+
//
5947+
// Judged by the same `@objectstack/spec/data` predicate the SEARCH axis
5948+
// uses ({@link isVirtualSearchField} / `SEARCH_VIRTUAL_TYPES`) rather
5949+
// than a list minted here, so this gate and the drivers cannot disagree
5950+
// about which types have a column. `summary` and `autonumber` are NOT
5951+
// in it and must not be: both get real stored columns and filter
5952+
// correctly — a gate widened to the spec's `COMPUTED_VALUE_TYPES` (the
5953+
// WRITE contract) would refuse two working types.
5954+
//
5955+
// PRECEDENCE — `unknown` first, then this, mirroring the sort axis'
5956+
// `unknown` > `dotted` > unmaterializable: identity errors before type
5957+
// errors. DOTTED names are deliberately NOT judged here: a dotted
5958+
// filter path has no verdict on this axis at all (its head being a real
5959+
// field is what carries it through the check above), and inventing one
5960+
// for the formula-headed case alone would answer two spellings of one
5961+
// unjudged shape differently.
5962+
const virtual = names.filter((f) => !f.includes('.') && isVirtualSearchField(gate.fields[f]));
5963+
if (virtual.length === 0) return;
5964+
const virtualFirst = virtual[0];
5965+
const virtualType = String(gate.fields[virtualFirst]?.type ?? 'formula');
58835966
const err: any = new Error(
5884-
`Query parameter '${param}' filters on '${first}', which is not a field on object `
5885-
+ `'${object}'`
5886-
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
5887-
+ '. A filter on a field that does not exist can only match zero records, so the '
5888-
+ 'query was refused instead of answered with an empty list.'
5889-
+ suggestFieldName(first, gate.declared),
5967+
`Query parameter '${param}' filters on '${virtualFirst}', a virtual '${virtualType}' `
5968+
+ `field on object '${object}'`
5969+
+ (virtual.length > 1 ? ` (also: ${virtual.slice(1).join(', ')})` : '')
5970+
+ '. Its value is computed on read and never stored, so no driver materializes a '
5971+
+ 'column to filter on: the predicate reaches the driver, matches nothing, and the '
5972+
+ 'query answers an empty list under a 200 — in BOTH directions, so a false test '
5973+
+ 'returns no records where the same test against a stored boolean returns every '
5974+
+ 'record.'
5975+
// Deliberately the same remedy, in the same words, as the SORT
5976+
// axis' formula refusal (#6994) and #6673's SEARCH-axis
5977+
// correction, with only the verb changed to name this axis. One
5978+
// vocabulary across the doors: an author refused on two axes must
5979+
// not be sent two different ways.
5980+
+ ` Denormalise the value onto '${object}' (a stored field, written when the source`
5981+
+ ' changes) and filter that.',
58905982
);
58915983
err.code = 'INVALID_FIELD';
58925984
err.status = 400;
5893-
err.field = first;
5894-
err.fields = unknown;
5985+
err.field = virtualFirst;
5986+
err.fields = virtual;
58955987
err.object = object;
58965988
err.param = param;
58975989
throw err;

0 commit comments

Comments
 (0)