Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .changeset/filter-formula-field-refusal.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
---
"@objectstack/metadata-protocol": minor
"@objectstack/objectql": minor
---

fix(filter): a `where` on a virtual `formula` field is refused, not answered with zero rows (#8296)

`formula` is the one field type no driver materialises a column for. Three query
axes can name a field; until now only two of them said so.

| axis | verdict for a `formula` field |
|:---|:---|
| SORT | `400 INVALID_SORT`, ingress (#6994) and engine (#7095) |
| SEARCH | `400 INVALID_FIELD`, refused by name (#6674) |
| **FILTER** | **accepted — 200, 0 rows, no error** |

`assertFilterFieldsExist` computed exactly one verdict — is this name a field of
the object — and a `formula` field IS one, so the predicate cleared the door and
reached a driver with no column behind it. Measured on a real `ObjectQL`, with
`is_open` a `formula` over the stored `status` column:

```
where { is_open: true } -> 0 rows, no error
where { is_open: false } -> 0 rows, no error
CONTROL where { status: 'open' } -> 4 rows
CONTROL where { subtask_total: 5 } -> 1 row (`summary` HAS a column)
```

Both directions are wrong and the `false` one is the dangerous one: the same
predicate against a STORED boolean returns every row, so a filter meaning "not
yet done" silently became "no records at all" — a row SET changed under a 200,
which no amount of inspecting the response can reveal. The formula READS
correctly in that very same response, so the field is visibly populated and
simultaneously unfilterable.

Both doors now refuse it with `400 INVALID_FIELD`, naming the offending key path
and prescribing the remedy the sort and search axes already share:

- **ingress** — `assertFilterFieldsExist` grows a second verdict, after
`unknown`, covering everything that reaches `findData`: the list route,
`POST /data/:object/query`, the export route and the RPC dispatcher, in every
filter spelling (`where` / `filter` / `filters` / `$filter`, the array sugar,
and nested `$and` / `$or`);
- **engine** — `assertFilterIsMaterializable` closes the half the ingress cannot
reach. It is author-reachable, not merely internal: a saved report's
`query.filter` is forwarded verbatim into `engine.find`, exactly as #7095
measured for `query.orderBy`. It runs at the engine's one filter-lowering
seam, so `find`, `findOne`, `count`, `aggregate`, `update` and `delete` all
answer alike, and it judges the CALLER's `where` only — a middleware-injected
RLS or sharing predicate is the platform's own and is never refused.

Both doors judge the field by the same `@objectstack/spec/data` predicate the
search axis uses (`isVirtualSearchField` / `SEARCH_VIRTUAL_TYPES`) rather than a
locally minted type list, so a gate and the drivers cannot disagree about which
types have a column.

**`summary` and `autonumber` are unaffected and still filter** — both get real
stored columns; the set is exactly `formula`. Reading, projecting and computing a
formula field are untouched; only the predicate is refused.

**What to change if this refuses one of your queries:** denormalise the value
onto the object (a stored field, written when the source changes) and filter
that. There is no mechanical rewrite in either direction — the platform cannot
invent the stored column, and it must not filter post-hoc after the formulas are
evaluated, because the driver has already applied `limit` / `offset`, so a
post-hoc predicate would filter an arbitrary PAGE. Grep your saved reports,
flows, dashboards and view filters for a filtered field whose object declares it
as a `formula`.

**In-tree sweep — source AND tests.** No shipped example app's *metadata* filters
a formula field: the ones the examples declare (`crm_contact.full_name`,
`crm_opportunity.expected_revenue` / `days_to_close`, `crm_lead.is_closed`,
`showcase_project.budget_remaining`, `showcase_field_zoo.f_formula`) appear only
as view columns, form fields, permission entries and record-level CEL
predicates — never in a `where` / `filter`. One in-tree TEST did filter one and
is updated in this change: `examples/app-todo/test/derived-flag-removal.test.ts`
registers a test-local formula-shaped object to record *why* two inert flags were
removed rather than derived, and pinned the behaviour this refusal abolishes —
filtering a formula answering 0 rows with no error. It now asserts the
`400 INVALID_FIELD` envelope instead; its conclusion is unchanged, because a
formula still cannot be filtered. The first sweep read app source only, which is
the wrong half: current behaviour is pinned in tests, so a behaviour change lands
there first.
80 changes: 59 additions & 21 deletions examples/app-todo/test/derived-flag-removal.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,12 +21,24 @@
* A formula computes both correctly — including the temporal one — so the
* obvious repair looks available. It is not, and the reason is a STORAGE fact
* rather than a taste judgment: a `formula` field is virtual, no driver
* materialises a column for it, and so a FILTER naming one matches nothing.
* That is measured here, not asserted — {@link REVERSE} registers the
* formula-shaped object and shows `where { is_completed: false }` answering
* **0 rows with no error** where the stored column answers every row. Deriving
* would have silently emptied the "Due Today" view, the daily reminder flow and
* both open-task reports: a wrong answer traded for an invisible one.
* materialises a column for it, and so a FILTER naming one cannot be applied
* as written. That is measured here, not asserted — {@link REVERSE} registers
* the formula-shaped object and shows `where { is_completed: false }` failing
* where the stored column answers every row. Deriving would have emptied the
* "Due Today" view, the daily reminder flow and both open-task reports.
*
* Since **#8296** that failure is VISIBLE. The engine's filter seam refuses a
* `where` naming a virtual `formula` field with `400 INVALID_FIELD` instead of
* handing the predicate to a driver with no column behind it and answering
* **0 rows with no error** — which is what this test measured when #7226 was
* decided, and the invisible zero was the danger: a wrong answer traded for an
* unobservable one.
*
* The storage fact that decided #7226 is unchanged, so the decision stands and
* its reasoning is stronger, not weaker: a formula field still carries no
* column, a filter naming one still could not have worked, and the eight app
* filters that read these flags still had to move to stored columns. Only the
* failure mode changed — a silent zero became a named 400.
*
* `status` and `due_date` are stored, indexed columns that already carry the
* information, and both are declared dimensions on the `task_metrics` dataset,
Expand DownExpand Up@@ -229,10 +241,23 @@ describe('#7226 — the replacement filters really select, on BOTH sides of the
* REVERSE VERIFICATION — the measurement that chose removal over derivation.
*
* Predicted direction, recorded BEFORE running it: the formula field READS
* correctly (so "just derive it" looks right) but is UNFILTERABLE, and the
* failure is silent — 0 rows, no error — rather than an exception. That
* asymmetry is the whole argument: an exception would have been safe, because
* someone would have seen it.
* correctly (so "just derive it" looks right) but is UNFILTERABLE. When #7226
* ran it the failure was silent — 0 rows, no error — rather than an exception,
* and this docblock named that asymmetry as the whole argument: **an exception
* would have been safe, because someone would have seen it.**
*
* **#8296 supplied that exception**, and the second `it` below therefore
* asserts a rejection envelope (`400 INVALID_FIELD`, naming the field and the
* object) where it used to assert an empty array. That is this file's own
* argument being adopted platform-wide — the safe design it asked for is now
* the shipped one — not a correction of it.
*
* The verdict on the derive route is UNCHANGED. A formula field still
* materialises no column and still cannot carry a predicate, so the eight app
* filters that named these flags still could not have worked; removal in
* favour of the stored `status` / `due_date` columns remains the only repair.
* What #8296 changed is that choosing the derive route now fails where someone
* can see it, instead of quietly answering an empty set.
*/
describe('REVERSE — why the derive route was rejected, measured', () => {
/** `todo_task` as it would look on the derive route. */
Expand DownExpand Up@@ -276,26 +301,39 @@ describe('REVERSE — why the derive route was rejected, measured', () => {
expect(byId.d.is_overdue).toBe(false); // no due date at all
});

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

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

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

// CONTROL — the stored column answers correctly on the same rows and the
// same engine, so the emptiness above is about the field being virtual, not
// about the fixture or the driver.
// same engine, so the refusal above is about the field being virtual, not
// about the fixture or the driver. (Assertions unchanged from #7226: the
// anti-vacuity arm never depended on the formula's failure mode.)
expect((await ql.find('derived_task', { where: { status: 'completed' } })).map((r: any) => r.id)).toEqual(['a']);
expect((await ql.find('derived_task', { where: { status: { $ne: 'completed' } } })).map((r: any) => r.id)).toEqual(['b']);
});
Expand Down
118 changes: 105 additions & 13 deletions packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5866,9 +5866,29 @@ export class ObjectStackProtocolImplementation implements
*
* Value shapes are NOT judged here: a wrong-typed or unrunnable filter is
* `INVALID_FILTER`'s job (#4121 / #4181), already answered upstream in this
* same block. This gate answers exactly one question — does this field
* exist — with exactly the envelope the write path and the bare-key door
* already give it.
* same block. This gate answers questions about the NAME, with exactly the
* envelope the write path and the bare-key door already give it.
*
* [#8296] It answers TWO of them now — "does this field exist" and, second,
* "does this field's TYPE materialise a column to filter on". A `formula`
* field is known, undotted and unfilterable: it cleared this gate precisely
* BECAUSE the object declares it, reached a driver that has no column for
* it, and answered 200 with zero rows in BOTH directions. That was the last
* axis in this family still fail-open — SORT refuses the same field
* (#6994/#7095) and SEARCH refuses it (#6674) — and it is the shape the
* standing ruling of 2026-08-12 names: a declaration the platform cannot
* honour is refused at the latest checkpoint that can see the whole
* picture, naming the offending key path, never answered 200.
*
* SCOPE: this is an INGRESS gate, so it covers what reaches {@link
* findData}. The half it cannot reach — a caller handing a `where` straight
* to `engine.find` / `findOne` / `count` / `aggregate` / `update` /
* `delete`, which is how a saved report's `query.filter` travels
* (`plugin-reports` forwards it verbatim) — is closed at the engine's own
* filter seam by `assertFilterIsMaterializable` (`@objectstack/objectql`,
* `filter-comparand-shape.ts`), with the same `400 INVALID_FIELD` and the
* same remedy sentence. Same two-door shape, and same reason, as the sort
* axis' #7095.
*/
private assertFilterFieldsExist(object: string, where: unknown, param: string): void {
if (!where || typeof where !== 'object') return;
Expand All@@ -5878,20 +5898,92 @@ export class ObjectStackProtocolImplementation implements
if (!gate) return;
// Head segment only, exactly as the bare-key door judges `owner_id.name`.
const unknown = names.filter((f) => !gate.known.has(f.split('.')[0]));
if (unknown.length === 0) return;
const first = unknown[0];
if (unknown.length > 0) {
const first = unknown[0];
const err: any = new Error(
`Query parameter '${param}' filters on '${first}', which is not a field on object `
+ `'${object}'`
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
+ '. A filter on a field that does not exist can only match zero records, so the '
+ 'query was refused instead of answered with an empty list.'
+ suggestFieldName(first, gate.declared),
);
err.code = 'INVALID_FIELD';
err.status = 400;
err.field = first;
err.fields = unknown;
err.object = object;
err.param = param;
throw err;
}

// [#8296] The SECOND verdict on this axis: a name that is a REAL field
// of this object and still cannot be filtered on, because its TYPE
// materialises no column. It is the FILTER axis finally growing the
// verdict its two neighbours already have — {@link
// assertSortFieldsExist} splits `unknown` from unmaterializable
// (#6994) and {@link assertSearchFieldsAreSearchable} splits `unknown`
// from `virtual` (#6674) — and it was the last axis on which a
// declaration the platform cannot honour still answered 200.
//
// Measured on a real `ObjectQL` + this protocol, base cb43296ef
// (`is_open` a `formula` over the stored `status` column):
//
// ```
// where { is_open: true } -> 0 rows, NO ERROR
// where { is_open: false } -> 0 rows, NO ERROR
// CONTROL where { status: 'open' } -> 4 rows
// CONTROL where { subtask_total: 5 } -> 1 row (`summary` HAS a column)
// ```
//
// BOTH directions are wrong and the `false` one is the dangerous one:
// the same predicate against a STORED boolean returns every row, so a
// filter meaning "not yet done" silently becomes "no records at all".
// The response is indistinguishable from an empty table, and the
// formula READS correctly in that very same response (`applyFormulaPlan`
// hydrates it), so the field is visibly populated and simultaneously
// unfilterable.
//
// Judged by the same `@objectstack/spec/data` predicate the SEARCH axis
// uses ({@link isVirtualSearchField} / `SEARCH_VIRTUAL_TYPES`) rather
// than a list minted here, so this gate and the drivers cannot disagree
// about which types have a column. `summary` and `autonumber` are NOT
// in it and must not be: both get real stored columns and filter
// correctly — a gate widened to the spec's `COMPUTED_VALUE_TYPES` (the
// WRITE contract) would refuse two working types.
//
// PRECEDENCE — `unknown` first, then this, mirroring the sort axis'
// `unknown` > `dotted` > unmaterializable: identity errors before type
// errors. DOTTED names are deliberately NOT judged here: a dotted
// filter path has no verdict on this axis at all (its head being a real
// field is what carries it through the check above), and inventing one
// for the formula-headed case alone would answer two spellings of one
// unjudged shape differently.
const virtual = names.filter((f) => !f.includes('.') && isVirtualSearchField(gate.fields[f]));
if (virtual.length === 0) return;
const virtualFirst = virtual[0];
const virtualType = String(gate.fields[virtualFirst]?.type ?? 'formula');
const err: any = new Error(
`Query parameter '${param}' filters on '${first}', which is not a field on object `
+ `'${object}'`
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
+ '. A filter on a field that does not exist can only match zero records, so the '
+ 'query was refused instead of answered with an empty list.'
+ suggestFieldName(first, gate.declared),
`Query parameter '${param}' filters on '${virtualFirst}', a virtual '${virtualType}' `
+ `field on object '${object}'`
+ (virtual.length > 1 ? ` (also: ${virtual.slice(1).join(', ')})` : '')
+ '. Its value is computed on read and never stored, so no driver materializes a '
+ 'column to filter on: the predicate reaches the driver, matches nothing, and the '
+ 'query answers an empty list under a 200 — in BOTH directions, so a false test '
+ 'returns no records where the same test against a stored boolean returns every '
+ 'record.'
// Deliberately the same remedy, in the same words, as the SORT
// axis' formula refusal (#6994) and #6673's SEARCH-axis
// correction, with only the verb changed to name this axis. One
// vocabulary across the doors: an author refused on two axes must
// not be sent two different ways.
+ ` Denormalise the value onto '${object}' (a stored field, written when the source`
+ ' changes) and filter that.',
);
err.code = 'INVALID_FIELD';
err.status = 400;
err.field = first;
err.fields = unknown;
err.field = virtualFirst;
err.fields = virtual;
err.object = object;
err.param = param;
throw err;
Expand Down
Loading
Loading