Skip to content

Commit cfe2387

Browse files
fix(objectql,metadata-protocol)!: the per-aggregation filter takes where's remaining doors — a bad date, an addDays numeric pair, an undeclared { $field } and an unknown key are refused; a Date bound compares as an instant (#20174)
Fixes #20148 Clause-②: no (narrowing) A per-aggregation `filter` (`aggregations[i].filter`) now meets the doors `where` meets. A bad date, an `addDays` numeric pair, an undeclared `{ $field }` and an unknown key each answered `200` with the filtered count `0` (every row under a negation). Now each is refused `400`, identically on an empty and on a populated table, before any driver call. A `Date` bound is compared as an instant, as the same bound in a `where` is. Head `4da90165cd`: a prose-only patch round (contract review 5853139988, seat amendment 5853144038 on #20148) over the reviewed head `f22f0e875f`. Every code and test blob is unchanged: `engine.ts` `4e1ebbf2a4`, `having-filter.ts` `ab6ba1df00`, `protocol.ts` `a9a77c8f6d`, and the three test files. Base `49144fccc8`. The branch merged `main` at `369bcbeda7`; the merge touched none of these files. Written by session `session_01Bvd69VPa6puiNzzPUroDBx`, branch `claude/issue-20148-aggregation-filter-doors`. | commit | change | |:--|:--| | `3129311d33` | engine loop: temporal door and declared-reference rules; `having-filter.ts`: reference walk, `Date` instant comparison; `protocol.ts`: filter keys through `where`'s gate | | `d831680d7c` | pins in `objectql`, `metadata-protocol` and `rest`; two changesets | | `f22f0e875f` | merge of `main` at `369bcbeda7` | | `4da90165cd` | changeset prose only: the `20148-*` wording (see Patch round), and a DELIBERATE CORRECTION of one clause in each of two pending changesets (see that section) | ## 1. Measured first (H1) A scratch harness (not committed) ran every shape through `ObjectQL.aggregate` and through `POST /api/v1/data/:object/query` (`RestServer`, then `findData`, then `ObjectQL.aggregate`). It used a real `InMemoryDriver` and a real `SqlDriver` (better-sqlite3 `:memory:`), both schema-synced, on a populated table (6 rows, groups c1 / c2 / c3) and an empty one, with driver reads counted. The REST body is JSON round-tripped, as the wire carries it. Every per-aggregation shape ran grouped and ungrouped. Beside each ran the same condition as the call's `where` (the twin), on the aggregate verb and on `find`. 2896 cells per tree. Legend: `m` is the filtered count, over `n` rows. "Base" and "head" are the per-aggregation filter's answer; the twin is identical at base and head. | # | `aggregations[i].filter` | base, memory and sql, engine and REST | head | the `where` twin | |:--|:--|:--|:--|:--| | 1 | `{ placed_on: { $gt: 'not-a-date' } }` (`date`) | empty `m0`, populated `m0` of 6 | `INVALID_FILTER` / 400, 0 reads, all 8 cells | `INVALID_FILTER` / 400, 0 reads, all 8 cells | | 1+ | the same as an `$in` member / a `$between` endpoint / implicit / behind a held `$or` / under `$not`; `'noon'` on a `time` | populated `m1` / `m6` / `m0` / `m6` / `m6` / `m0` | `INVALID_FILTER` / 400, 0 reads, all 8 cells | `INVALID_FILTER` / 400 | | 1+ | `'last_30_days'` on a `datetime` | engine `m0`; REST `VALIDATION_FAILED` (ingress) | engine `INVALID_FILTER`; REST unchanged | the same split | | 2 | `{ amount: { $gt: { $field: 'cap', addDays: 1 } } }` | populated `m0` | `INVALID_FILTER` / 400, 0 reads, all 8 cells | sql `INVALID_FILTER` / 400; memory `n0` | | 2+ | `addDays` date-to-numeric, date-to-datetime, text-to-date, time pair, text or date offset column, a numeric pair behind a held `$or` | `m0` (date/datetime `m4`, held `$or` `m6`) | `INVALID_FILTER` / 400, 0 reads | sql `INVALID_FILTER`; memory a count | | 3 | `{ amount: { $gt: { $field: 'nope' } } }` | populated `m0` | `INVALID_FILTER` / 400, 0 reads, all 8 cells | sql `INVALID_FILTER` / 400; memory `n0` | | 3+ | under `$ne` / `$not` / a held `$or`; a dotted referent; an undeclared `addDays` offset column | `m6` / `m6` / `m6` / `m0` / `m3` | `INVALID_FILTER` / 400, 0 reads | sql `INVALID_FILTER`; memory a count | | 4 | `{ nope: 1 }` over REST | `200`, `m0` | REST `INVALID_FIELD` / 400, 0 reads; the engine door is unchanged (a count) | REST `INVALID_FIELD` / 400; engine: sql `INVALID_FILTER`, memory `n0` | | 4+ | under `$and` / `$ne` / `$not` / a held `$or` | `m0` / `m6` / `m6` / `m6` | REST `INVALID_FIELD` / 400 | REST `INVALID_FIELD` / 400 | | 4+ | `{ nope: { $median: 1 } }` | `INVALID_FILTER` everywhere | REST `INVALID_FIELD` (the field gate runs first, as for `where`); engine `INVALID_FILTER` | the same split | | 5 | `{ opened_at: { $gt: new Date('2026-02-01') } }`, in-process | `m0` | `m4` | `n4` on both drivers | Found beyond the card: the `$between` and held-`$or` rows counted every row, not zero. The card's row 5 reads "4 rows on both drivers" for the twin, and the #20122 dev report reads memory `0`. Both are right about their harness: `driver-memory` coerces a `Date` comparand by the column's storage rule only after `syncSchema` has indexed the object's temporal fields. Schema-synced, as a booted runtime is, the twin answers 4 on both drivers. ## 2. What changed - **`packages/objectql/src/engine.ts`, the per-aggregation loop of `ObjectQL.aggregate`.** - (a) It runs `assertTemporalComparandsInterpretable`, the function `where` runs fourth on its seam, fourth here too: after the text-operator door, before the type door. - (b) It hands `assertAggregationFilterIsEvaluable` the object's declared field map, and a sink that logs the withheld diagnostic at `warn`. - **`packages/objectql/src/having-filter.ts`.** - `assertAggregationFilterIsEvaluable(filter, index, declared?)`: after the walker's own refusals, `assertAggregationFilterReferencesAreDeclared` walks each scalar-comparison `{ $field }`. The referent, and an `addDays` offset's nested `$field`, must name a declared field (the field map plus `id`, `created_at`, `updated_at`: the set the REST field gate reads). An `addDays` pair must pass the class rule. - The class rule is #20127's, factored out of `assertOffsetPairIsTemporal` into `offsetPairViolation`, so `having` and this position judge one rule. `having`'s words are byte-identical. - The words withhold the fields, the operator and the reason, as `driver-sql` withholds them for the same comparison in `where`. They name the aggregation (`aggregations[1].filter`) and the rule, and say the diagnostic is in the server log. The message is sized under the REST envelope's 500-character bound. - `checkCondition`: when a `Date` sits on either side of an equality, ordering, `$between` or list arm, and both sides denote an instant (`utcInstantMs`, `@objectstack/spec/data`), it compares instants. That is the lift `@objectstack/formula`'s evaluator applies. Every other pair compares exactly as before. - **`packages/metadata-protocol/src/protocol.ts`, `findData`'s `assertAggregationFieldsExist` only.** After its entry and field checks, each entry's `filter` goes through `assertFilterFieldsExist`, the gate the explicit `where` meets. It uses the same field set and the same unknown, dotted and virtual ladder, answers `INVALID_FIELD` / 400, and passes `aggregations[i].filter` as the parameter. ## 3. The hypotheses - **H1, held.** The measured rows are in §1. - **H2, half-falsified, and no copy was needed.** - Row 1 calls the same function `where` calls. - Row 4 calls `where`'s own gate from inside `assertAggregationFieldsExist`. - Rows 2 and 3 have no engine function on `where`'s side. `where`'s refusal is `driver-sql`'s cross-field compiler (`applyCrossFieldComparison`). `objectql` cannot import it, and the claim excludes `driver-sql`. On `driver-memory` the twin is not refused at all: a count. The per-aggregation filter never reaches a driver, because it forces the in-memory fallback. So these rules reuse the `having` walker's own rule functions (#20099 / #20127) against the object's declared fields, not the aggregated row's. Nothing is copied, and nothing new needed exporting. - **H3, held.** - Base to head over 2896 cells: 2392 identical, 504 moved. Every moved cell is a per-aggregation cell of rows 1 to 5 or of the added rows, or a `having` cell with an in-process `Date` bound. - Every `where` twin (aggregate and `find`), every `groupBy` probe, every per-aggregation control, and every `having` cell without a `Date` is byte-identical, including #20123's key refusals and #20127's pair refusals. - The 36 `having` cells that moved are the engine-door `Date` cells (9 shapes, both `having` doors, both drivers, populated table). Each now keeps the groups the same bound's ISO spelling keeps (the REST cell), except the two UTC-midnight-`Date` shapes on `max(date)`, a `date`-class column. `$gte` keeps `c2, c3` where the ISO-instant spelling keeps `c2`, and `$eq` keeps `c3` where the ISO-instant spelling keeps none. Both follow the calendar-day reading a `where` gives, and the ISO-instant text, compared as text, does not. See the out-of-scope note in Acceptance notes. - The reverse state (the three source files at the base blobs) reproduced the base table exactly: 2704 of 2704. - **H4, held.** Two changesets, each `minor`, BREAKING, `Clause-②: no (narrowing)`, disposition `not-required (no-migration-prescription)` (the #20123 / #20127 precedent). One is `@objectstack/objectql`, one is `@objectstack/metadata-protocol`. At head `4da90165cd`, `check-adr-0087-registration --base 49144fc` reports 4 declared-breaking changesets, each with a disposition: this PR's two `20148-*`, and `main`'s `19856-*` (registered) and `20055-*` (not-required), which arrived with the merge. Against `origin/main` it reports this PR's two. `check-changeset-no-major` reports no `major`, and its level axis reads this body's `Clause-②: no (narrowing)`. - **H5, decided.** - `where` gives 4 of 6 for the row-5 bound on both drivers (schema-synced). Each driver reads a `Date` by the column's storage rule: canonical UTC ISO for `datetime`, the UTC calendar day for `date`, the UTC time of day for `time`. - `checkCondition` holds a flat row and no declaration, so it reads the pair the way the spec defines for a type-blind evaluator (`utcInstantMs`). Every `datetime` shape (`$gt`, `$gte`, `$lt`, `$lte`, `$eq`, `$ne`, implicit, `$in`, `$nin`, `$between`) and a UTC-midnight `Date` on a `date` field now count what the twin counts. - `having`'s answers move, toward the twin (H3). - Two in-process readings still differ from the twin: a `Date` carrying a time of day against a `date` field (`$gte` 1 vs 3, `$lt` 5 vs 3, `$eq` 0 vs 2), and a `Date` against a `time` field (0 vs 3). Matching them needs the column's class in the per-row walker. See the open question in the report. ## 4. Tests All at code head `f22f0e875f` unless stated. - `pnpm --filter @objectstack/objectql exec vitest run` (whole suite, before the merge, at `d831680d7c`): `Test Files 317 passed (317) · Tests 5633 passed (5633)`. - `pnpm --filter @objectstack/metadata-protocol exec vitest run` (whole suite, at `d831680d7c`): `Test Files 189 passed | 3 skipped (192) · Tests 2699 passed | 19 skipped (2718)`. - `typecheck` for `@objectstack/objectql`, `@objectstack/metadata-protocol` and `@objectstack/rest`: each exit 0. `--listFiles` shows each new test file in its package's program. `check:test-typecheck` for `objectql`: `40 file(s) / 234 error(s) / 65 pinned`, unchanged. - REST aggregate tests (`aggregation-filter-where-doors`, `list-view-grouping-query-door`, `request-schema-gate.conformance`, `rest-server-canonical-query-ast`): `96 passed | 1 skipped`. - `driver-sql` aggregate suites (10 files): `125 passed | 10 skipped` (live PG / MySQL not run). `driver-memory` aggregate suites (3 files): `75 passed`. - `@objectstack/service-analytics`, the consumer that lowers measure filters here (whole suite): `128 files, 3017 passed`. - After the merge: the objectql aggregate / having files, the two protocol files and the four REST files were green again. - **Reverse verification.** The fix was committed first. The three source files were restored to their base blobs (`bc4f337220`, `876293a1e9`, `ba5fbccde6`) under an EXIT / INT / TERM trap. On-disk hashes and marker counts (0 / 0 / 0) were checked. `objectql` and `metadata-protocol` were force-rebuilt, and `ablation-dist-preflight --absent` passed for both. - The new pins then read: `engine-aggregate-filter.test.ts` 34 failed of 107, `protocol.aggregation-filter-fields.test.ts` 7 of 13, `aggregation-filter-where-doors.test.ts` 10 of 15. - Those are exactly the new refusal and `Date` rows. The controls, the walker-first row and the "not refused by the engine" control stayed green. - Restored with `git checkout HEAD --`. Proven by HEAD-blob equality for all three and an empty whole-tree `git status`. `dist/` was rebuilt from HEAD and the preflight found the markers present. ## 5. Gates - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `f22f0e875f`: 65 commands, 65 run. - `--ran` with the exit codes: `65 derived famil(ies) accounted for — 65 run, 0 NOT-MEASURED`. - `check:dual-build-cjs-loads` and `check:type-check-debt` first answered `PREREQUISITE NOT MET` (3). After `turbo run build --filter='./packages/*' --filter='./packages/*/*'`, both exit 0. - `node scripts/check-issue-citations.mjs --base 369bcbe`: 20 citations, all resolve. - `pnpm check:nul-bytes`: exit 0. A control-byte self-scan of the 8 changed files found none. - Lint, a declared narrowing: - `eslint --no-inline-config --format json` on the 6 changed `.ts` files: 6 files, 0 errors, 0 warnings, 0 fatal. - `--print-config` returns a config for each. - `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`), so this diff cannot move a verdict on an untouched file. ## Patch round (contract review 5853139988) Prose only; every code and test blob is identical to `f22f0e875f` (`git diff --name-only f22f0e8 4da9016` lists three `.changeset/` files and nothing else). - `.changeset/20148-aggregation-filter-where-doors.md` (`git diff --numstat`: `2 2`): - The `having` `Date` sentence is qualified. It now reads that a `Date` bound keeps the groups its ISO spelling keeps on a `datetime` column. On a `date`-class column (`min` / `max` of a `date` field), a UTC-midnight `Date` follows the calendar-day reading a `where` gives (`{ $gte: new Date('2026-02-01') }` keeps the group whose max is `2026-02-01`, and so does `$eq`), which the ISO-instant text, compared as text, did not. - "refused in the words `driver-sql` uses" now reads "refused in the withholding posture `driver-sql` applies". The sentence on the wire is new; only the posture is borrowed. - Gates at `4da90165cd`: - `check-empty-changeset --base origin/main`: exit 1. It names exactly `.changeset/20099-having-where-doors.md` and `.changeset/20127-having-adddays-temporal-pair.md`, each "present on the merge base and CHANGED by this PR". This is by design; see the next section. - `check-adr-0087-registration`: "4 declared-breaking changeset(s), each carrying an ADR-0087 disposition" with `--base 49144fc`, and 2 with `--base origin/main`. Exit 0. - `check-changeset-no-major --base 49144fc --event` (this body, as a `pull_request` payload): exit 0. - `check-issue-citations --base 369bcbe`: 20 citations, all resolve. Exit 0. ## DELIBERATE CORRECTION Two PENDING changesets carried one clause each that is false at this head. Each file's `git diff --numstat` is `1 1`, and every other line is byte-identical. `check-empty-changeset` refuses both by name, which is the gate's own path for this class: the correction is said here, for confirmation. - `.changeset/20127-having-adddays-temporal-pair.md`, its last sentence. This PR made it false: the per-aggregation filter is now judged by the class rule, through `offsetPairViolation`, against the object's declared fields. - Before: "A per-aggregation `filter` (`aggregations[i].filter`) is not judged by this rule: it reads the object's raw columns, and this change classifies only the aggregated row's." - After: "A per-aggregation `filter` (`aggregations[i].filter`) is not judged by this change: it reads the object's raw columns, and this change classifies only the aggregated row's. Since #20148 it is judged by the same class rule, against the object's declared fields." - `.changeset/20099-having-where-doors.md`, one clause of its "Not changed" paragraph. This has been false since PR #20147: #20123 refuses a `having` key that names no column of the aggregated row. - Before: "A `having` key that names no column still keeps no group rather than being refused." - After: "A `having` key that names no column is not judged by this change; since #20123 it is refused (`INVALID_FILTER` / 400) rather than keeping no group." ## Acceptance notes - **The card's two observations.** - The `formula` `min` / `max` `addDays` pair is still unclassified in `having` (#20127's rule answers `undefined` for a `formula`), and a `formula` referent here is judged by name only. - The REST envelope's 500-character truncation (`truncateClientMessage`) still applies to #20123's column list. It bit this PR's first draft of the withheld message, which is now 440 characters at a two-digit index. Neither is changed. - **Position label.** The temporal door is `where`'s function, run unchanged, so its message reads `at where.placed_on.$gt` for a per-aggregation filter. The #15661 text-operator door already reads `where.…` at this position. The pins assert the field and the value, not the path. A `path` parameter on `assertTemporalComparandsInterpretable` would fix both; `temporal-comparand-door.ts` is outside the claim. - **Withholding at this position.** Rows 2 and 3 withhold their names, per the ruling. The #20122 reference refusals at the same position (bare reference, list-member reference, malformed `addDays`) name the fields. Nothing reaches a per-aggregation filter from a read-scope merge: RLS and sharing compose into `ast.where` only, and analytics merges its scope into `where`. See the report's open question. - **Two named deltas from `driver-sql`'s `where` rules, both deliberate.** - `id` is accepted as a referent. `driver-sql` refuses it (not in its declared map), but every fallback row carries it, and the base counted it correctly. - The tenant-isolation column is not forbidden as a referent. That rule is `driver-sql`'s guard for policy compilation, and no policy reaches this position. - **Held, not endorsed.** A cross-class `{ $field }` pair without `addDays` (`{ amount: { $gt: { $field: 'placed_on' } } }`) still counts by coercion, as #20122's notes record for `having`. `FieldReferenceSchema` declares a class rule for `addDays` only. - **Out of scope, reported to the seat.** An interpretable temporal STRING in a per-aggregation filter is compared as text, not read by the column's storage rule as `where` reads it. This is reachable over REST today and unchanged by this PR (base = head, 192 cells). - `{ placed_on: { $gte: '2026-02-01T00:00:00.000Z' } }` counts 1 where the twin counts 3. - `{ opened_at: { $lte: '2026-02-01' } }` counts 2 where the twin counts 3, because `where` reads a bare day as the whole day. - An epoch-ms bound on a `datetime` counts 0 where the twin counts 3. ## Deviations - The claim names `assertAggregationFilterIsEvaluable` and `checkCondition` in `having-filter.ts`. This PR also adds module-level helpers there: the reference walk, the withheld error, the instant helpers, and the `AggregationFilterDeclaration` interface. That interface is `export`ed at module level but package-private, because `having-filter.ts` is not re-exported by `packages/objectql/src/index.ts`. The PR also refactors `assertOffsetPairIsTemporal` into `offsetPairViolation` plus a wrapper so both positions share one rule, adds one import, and adds a paragraph to the header comment. `having`'s cells are byte-identical (H3). --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2dfe070 commit cfe2387

10 files changed

Lines changed: 974 additions & 31 deletions

‎.changeset/20099-having-where-doors.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,4 @@ Not refused, but answering differently:
3030

3131
Who is affected: `having` is a request-only key (`QuerySchema.having`, `EngineAggregateOptions.having`), and no metadata type stores it. Every `having` in this repository's docs and published skills is a scalar comparison against an aggregation alias (`{ order_count: { $gt: 5 } }` and the like), which answers exactly as before. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
3232

33-
Not changed: scalars, `null` in the equality slot, `$in` / `$nin` lists, a two-bound `$between`, scalar ordering bounds, `{}`, `null` and an omitted `having`, on both paths. The `$like` / `$ilike` operators are still refused on `having` (they are staged out of `FILTER_OPERATORS`), and `$ne` with a list is still answered until the shared face judges it. A `having` key that names no column still keeps no group rather than being refused.
33+
Not changed: scalars, `null` in the equality slot, `$in` / `$nin` lists, a two-bound `$between`, scalar ordering bounds, `{}`, `null` and an omitted `having`, on both paths. The `$like` / `$ilike` operators are still refused on `having` (they are staged out of `FILTER_OPERATORS`), and `$ne` with a list is still answered until the shared face judges it. A `having` key that names no column is not judged by this change; since #20123 it is refused (`INVALID_FILTER` / 400) rather than keeping no group.

‎.changeset/20127-having-adddays-temporal-pair.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,4 @@ A column whose class the declaration cannot tell is not judged: an object with n
3131

3232
Who is affected: `having` is a request-only key (`QuerySchema.having`, `EngineAggregateOptions.having`), and no metadata type stores it. No `having` in this repository's docs and published skills carries a `{ $field }` reference. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
3333

34-
Not changed, measured identical before and after on both paths: a `date` / `date` pair and a `datetime` / `datetime` pair, with a positive or negative whole-day literal or with an offset read from a numeric column (`max` of a number, or a `count`); a `day` date bucket against a `date` column; and any `{ $field }` comparison WITHOUT `addDays`, including a numeric pair and a numeric column against a `date` one. A per-aggregation `filter` (`aggregations[i].filter`) is not judged by this rule: it reads the object's raw columns, and this change classifies only the aggregated row's.
34+
Not changed, measured identical before and after on both paths: a `date` / `date` pair and a `datetime` / `datetime` pair, with a positive or negative whole-day literal or with an offset read from a numeric column (`max` of a number, or a `count`); a `day` date bucket against a `date` column; and any `{ $field }` comparison WITHOUT `addDays`, including a numeric pair and a numeric column against a `date` one. A per-aggregation `filter` (`aggregations[i].filter`) is not judged by this change: it reads the object's raw columns, and this change classifies only the aggregated row's. Since #20148 it is judged by the same class rule, against the object's declared fields.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
---
4+
5+
fix(metadata-protocol)!: the read door judges the keys inside each per-aggregation `filter` with the gate an explicit `where` meets — an unknown key is `INVALID_FIELD` / 400, not a count of zero (#20148)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. A per-aggregation filter key that names no field of the object never had a meaning: no row carries it, so the aggregation read "no value" in every row and counted zero, or every row under a negation, while the same key in `where` was already refused by this gate. There is no accepted spelling it can be mechanically rewritten to: which field the author meant is an authoring decision, and the refusal names the key and suggests the nearest declared field. The gate is the one `where` already meets, with the same verdicts. The request is not a stored document, so there is nothing for `objectstack migrate meta` to rewrite. The table below records the answer each shape had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what `POST /data/:object/query` (`findData`) accepts in `aggregations[i].filter`. Each entry's filter now goes through the gate the explicit `where` meets, `assertFilterFieldsExist`: the same field set, the same verdicts, `INVALID_FIELD` / 400, with the entry's position as the parameter (`aggregations[1].filter`). The refusal is raised before the engine is called. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured on the base through `POST /data/:object/query` on `driver-memory` and `driver-sql`, over six rows in three groups:
14+
15+
| in `aggregations[i].filter` | before | now |
16+
|:--|:--|:--|
17+
| a key naming no field of the object (`{ nope: 1 }`), also under `$and` | `200`, that count 0 | `INVALID_FIELD` / 400, naming the key |
18+
| the same key under `$ne`, `$not`, or behind a `$or` branch that holds | `200`, every row counted | `INVALID_FIELD` / 400 |
19+
| an unknown key carrying an unknown operator (`{ nope: { $median: 1 } }`) | `INVALID_FILTER` / 400 from the engine | `INVALID_FIELD` / 400: the field gate runs first, as it does for the same key in `where` |
20+
| a dotted key on a scalar head, or a key naming a `formula` field | `INVALID_FIELD` / 400 from the engine's own filter seam | `INVALID_FIELD` / 400 from this gate, in the words `where` gets |
21+
22+
Run after the entry checks and the aggregated-field check, so an entry the spec cannot read keeps its shape refusal and an unknown aggregated `field` keeps its own. A filter that is not a plain object names no key here and is left to the engine's shape gate.
23+
24+
Not changed: a filter on declared keys, on `id`, `created_at` or `updated_at`, and a relation head in the nested-object form reach the engine with the filter untouched.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: a per-aggregation `filter` (`aggregations[i].filter`) on `engine.aggregate` takes the doors `where` takes — the temporal-comparand door, a `{ $field }` referent that must be a declared field, and the `addDays` class rule — and a `Date` bound is compared as an instant, as it is in `where` (#20148)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. Each refused input is one this position never evaluated as written: it answered a count by coercion (usually zero, every row under a negation), while the same condition as a `where` was already refused, by the engine's temporal-comparand door or by driver-sql's cross-field compiler. There is no accepted spelling a refused input can be mechanically rewritten to: which date, which declared field, or which pair of temporal fields the author meant is an authoring decision, and the refusal says what the position takes. A stored measure filter reaches this position through the analytics service, which lowers it; there is no stored document for `objectstack migrate meta` to rewrite. The table below records the answer each shape had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what a per-aggregation `filter` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query` with `aggregations`) that forwards it there. Every refusal below is `INVALID_FILTER` / 400, raised in the engine's per-aggregation loop before any driver is asked for a row, identically on an empty and on a populated table. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured on the base through `engine.aggregate` and through `POST /data/:object/query`, on `driver-memory` and `driver-sql`, over six rows in three groups, with the same condition written as the call's `where` beside each:
14+
15+
| in `aggregations[i].filter` | before | now |
16+
|:--|:--|:--|
17+
| a comparand a declared temporal field cannot read: `{ placed_on: { $gt: 'not-a-date' } }` on a `date`, the same as an `$in` member, a `$between` endpoint, in the implicit slot, behind a `$or` branch that holds or under `$not`; a preset name (`'last_30_days'`) on a `datetime`; `'noon'` on a `time` | counted as written: no row for the bad bound, only the readable members of the `$in`, every row for the `$between`, held-`$or` and `$not` shapes. The same condition as a `where` was refused | refused by the temporal-comparand door `where` takes, run unchanged on this position, in its words. A `{placeholder}` is stepped around and resolved, as there |
18+
| a `{ $field }` naming no declared field of the object (`{ amount: { $gt: { $field: 'nope' } } }`), a dotted referent, or an `addDays` offset column the object does not declare | no row counted; every row under `$ne`, `$not` or a held `$or`. `driver-sql` refused the same comparison in a `where` | refused |
19+
| `addDays` on a pair `FieldReferenceSchema.addDays` does not declare it for: two numeric fields (`{ amount: { $gt: { $field: 'cap', addDays: 1 } } }`), a numeric referent, a text or a time pair, a `date` against a `datetime`, or an offset read from a column that is not numeric | answered by the evaluator's epoch-millisecond coercion: no row on the fixture, 4 of 6 for the `date` / `datetime` pair. `driver-sql` refused the same pair in a `where` | refused |
20+
21+
The two `{ $field }` rows are refused in the withholding posture `driver-sql` applies to the same comparison in a `where`: the message names the aggregation that carries the reference (`aggregations[1].filter`) and the rule, and withholds the fields, the operator and the specific reason. The engine logs the withheld diagnostic at `warn`, once per refusal. A referent is judged against the object's declared field map plus `id`, `created_at` and `updated_at`, the set the REST field gate reads; on a host whose registry holds no field map for the object, nothing is judged.
22+
23+
These refusals run after the walker's own (an unknown operator, a malformed reference), so a filter carrying both gets the walker's refusal first.
24+
25+
Not refused, but answering differently:
26+
27+
- **A `Date` bound is compared as an instant.** `{ opened_at: { $gt: new Date('2026-02-01') } }` against the ISO text a `datetime` column holds counted no row: JS compared the `Date` with the string by coercion. `$ne` and `$nin` counted every row, and a `$between` of two `Date`s counted every row. The same bound in a `where` counted 4 of 6 on both drivers. The comparison now reads the pair through `@objectstack/spec/data`'s `utcInstantMs`, the lift `@objectstack/formula`'s evaluator applies, whenever one side is a `Date` and both sides denote an instant. Every `datetime` row and a UTC-midnight `Date` on a `date` field now count what the same bound counts in a `where`. `having` shares this comparison, so a `Date` bound in `having` keeps the groups its ISO spelling keeps on a `datetime` column; on a `date`-class column (`min` / `max` of a `date` field) a UTC-midnight `Date` follows the calendar-day reading a `where` gives (`{ $gte: new Date('2026-02-01') }` keeps the group whose max is `2026-02-01`, and so does `$eq`), which the ISO-instant text, compared as text, did not. A `Date` is not JSON, so this reaches in-process callers only. Two readings still differ from a `where`, because the per-row comparison holds no declaration: a `Date` carrying a time of day against a `date` field, which a `where` reads as that UTC calendar day, and a `Date` against a `time` field, which is not an instant and is left as before.
28+
29+
Not changed, measured identical before and after on both drivers: every `where`, `groupBy` and `having` answer outside the `Date` shapes above, and a per-aggregation filter that uses implicit equality, ordering bounds, `$in`, `$between`, `$or`, `{}`, a `{ $field }` between two declared numeric fields, `addDays` between two `date` or two `datetime` fields (literal or read from a numeric column), and a reference to `created_at` or `id`.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#20148] The KEYS inside a per-aggregation `filter`, at the read door.
5+
*
6+
* `findData` judged a `where`'s keys against the object's field set (#7534,
7+
* {@link ObjectStackProtocolImplementation} `assertFilterFieldsExist`) and an
8+
* aggregation's `function` / `alias` / `field` (#4254), but never the keys of
9+
* the filter an aggregation carries. Measured on the base through
10+
* `POST /api/v1/data/:object/query` on driver-memory and driver-sql:
11+
*
12+
* ```
13+
* aggregations: [{ …, filter: { nope: 1 } }] -> 200, that count 0
14+
* aggregations: [{ …, filter: { nope: { $ne: 1 } } }] -> 200, EVERY row counted
15+
* where: { nope: 1 } -> 400 INVALID_FIELD
16+
* ```
17+
*
18+
* Now each aggregation's filter goes through the `where` gate itself: the same
19+
* field set, the same `unknown` > `dotted` > virtual ladder, the same
20+
* `INVALID_FIELD` / 400, the caller's position (`aggregations[1].filter`) as
21+
* the parameter — and the engine is never reached. Every pin below asserts
22+
* `code` AND `status` AND that `engine.aggregate` was not called; the
23+
* preservation half asserts what the engine received.
24+
*/
25+
26+
import { describe, it, expect, vi } from 'vitest';
27+
import { ObjectStackProtocolImplementation } from './protocol.js';
28+
29+
const SCHEMA = {
30+
name: 'invoice',
31+
fields: {
32+
name: { name: 'name', type: 'text' },
33+
status: { name: 'status', type: 'text' },
34+
amount: { name: 'amount', type: 'number' },
35+
doubled: { name: 'doubled', type: 'formula', expression: 'amount * 2' },
36+
owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' },
37+
},
38+
};
39+
40+
function makeProtocol() {
41+
const find = vi.fn(async () => [] as unknown[]);
42+
const aggregate = vi.fn(async () => [] as unknown[]);
43+
const engine = {
44+
registry: { getObject: (n: string) => (n === 'invoice' ? SCHEMA : undefined) },
45+
find,
46+
aggregate,
47+
count: vi.fn(async () => 0),
48+
};
49+
return { p: new ObjectStackProtocolImplementation(engine as any), find, aggregate };
50+
}
51+
52+
/** The second of two aggregations carries `filter`, so the position must name WHICH one. */
53+
function withFilter(filter: unknown) {
54+
return {
55+
aggregations: [
56+
{ function: 'count', alias: 'n' },
57+
{ function: 'count', alias: 'm', filter },
58+
],
59+
};
60+
}
61+
62+
async function refusalFor(query: Record<string, unknown>) {
63+
const { p, find, aggregate } = makeProtocol();
64+
let answered: unknown;
65+
try {
66+
answered = await p.findData({ object: 'invoice', query } as never);
67+
} catch (e) {
68+
expect(find, 'the engine was reached before the refusal').not.toHaveBeenCalled();
69+
expect(aggregate, 'the engine was reached before the refusal').not.toHaveBeenCalled();
70+
return e as Error & { status?: number; code?: string; param?: string; field?: string; fields?: string[] };
71+
}
72+
throw new Error(`${JSON.stringify(query)} was ACCEPTED (answered ${JSON.stringify(answered)}) instead of refused`);
73+
}
74+
75+
describe('[#20148] findData — the keys inside a per-aggregation filter meet the where gate', () => {
76+
const REFUSED: ReadonlyArray<readonly [string, Record<string, unknown>, string]> = [
77+
['an unknown key (the card\'s row 4)', { nope: 1 }, 'nope'],
78+
['an unknown key under $ne', { nope: { $ne: 1 } }, 'nope'],
79+
['an unknown key under $and', { $and: [{ amount: { $gt: 0 } }, { nope: 1 }] }, 'nope'],
80+
['an unknown key behind a $or branch that holds', { $or: [{ amount: { $gt: 0 } }, { nope: 1 }] }, 'nope'],
81+
['an unknown key under $not', { $not: { nope: 1 } }, 'nope'],
82+
['an unknown key beside an unknown operator', { nope: { $median: 1 } }, 'nope'],
83+
];
84+
85+
for (const [name, filter, key] of REFUSED) {
86+
it(`${name}: 400 INVALID_FIELD at the aggregation's position, as the same key in where is`, async () => {
87+
const err = await refusalFor(withFilter(filter));
88+
expect(err.code).toBe('INVALID_FIELD');
89+
expect(err.status).toBe(400);
90+
expect(err.field).toBe(key);
91+
expect(err.param).toBe('aggregations[1].filter');
92+
// The twin: the same condition as the call's `where`.
93+
const twin = await refusalFor({ where: filter, aggregations: [{ function: 'count', alias: 'n' }] });
94+
expect(twin.code).toBe('INVALID_FIELD');
95+
expect(twin.status).toBe(400);
96+
expect(twin.field).toBe(key);
97+
});
98+
}
99+
100+
it('the gate\'s other two verdicts come with it — a dotted path and a virtual field, each as in where', async () => {
101+
for (const [filter, field] of [[{ 'status.name': 'x' }, 'status.name'], [{ doubled: { $gt: 10 } }, 'doubled']] as const) {
102+
const err = await refusalFor(withFilter(filter));
103+
const twin = await refusalFor({ where: filter, aggregations: [{ function: 'count', alias: 'n' }] });
104+
for (const e of [err, twin]) {
105+
expect(e.code).toBe('INVALID_FIELD');
106+
expect(e.status).toBe(400);
107+
expect(e.field).toBe(field);
108+
}
109+
expect(err.param).toBe('aggregations[1].filter');
110+
}
111+
});
112+
113+
it('an unknown aggregated field keeps its own verdict ahead of a filter key', async () => {
114+
const err = await refusalFor({
115+
aggregations: [{ function: 'sum', field: 'amout', alias: 't', filter: { nope: 1 } }],
116+
});
117+
expect(err.code).toBe('INVALID_FIELD');
118+
expect(err.param).toBe('aggregations');
119+
expect(err.field).toBe('amout');
120+
});
121+
122+
const ADMITTED: ReadonlyArray<readonly [string, unknown]> = [
123+
['declared keys, composed', { $or: [{ amount: { $gt: 100 } }, { status: 'paid' }] }],
124+
['the columns every row carries', { id: 'i1', created_at: { $ne: null }, updated_at: { $ne: null } }],
125+
['a relation head in the nested-object form', { owner_id: { $ne: null } }],
126+
['a filter that is not an object (left to the engine\'s shape gate)', 'status = paid'],
127+
['an array filter (left to the engine\'s shape gate)', [['amount', '>', 1]]],
128+
];
129+
130+
for (const [name, filter] of ADMITTED) {
131+
it(`${name}: reaches engine.aggregate with the filter untouched`, async () => {
132+
const { p, aggregate } = makeProtocol();
133+
await p.findData({ object: 'invoice', query: withFilter(filter) } as never);
134+
expect(aggregate).toHaveBeenCalledTimes(1);
135+
const opts = (aggregate.mock.calls[0] as unknown[])[1] as { aggregations: Array<{ filter?: unknown }> };
136+
expect(opts.aggregations[1].filter).toEqual(filter);
137+
});
138+
}
139+
});

0 commit comments

Comments
 (0)