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
34 changes: 34 additions & 0 deletions .changeset/20122-aggregation-filter-row-independent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"@objectstack/objectql": minor
---

fix(objectql)!: a per-aggregation `filter` (`aggregations[i].filter`) on `engine.aggregate` is judged once, before any row is read — its refusals no longer depend on whether the table has rows, and it takes the shape gate and the comparand-type door `where` takes (#20122)

Clause-②: no (narrowing)

<!-- adr-0087: not-required (already-registered filter-between-field-reference-endpoint-refused, filter-icontains-comparand-refused-at-parse, filter-regex-options-retired) this change adds no new transition. Each refusal below was already made by the per-aggregation filter's walker on a populated table, or is a shared door `where` already takes, run unmodified on one more position: the first id's replacement states that a { $field } reference is legal as the WHOLE comparand of $eq / $ne / $gt / $gte / $lt / $lte only, which is the position refusal the walker now makes whatever the rows; the other two register the $icontains comparand and the retired $regex / $options refusals it already made on a populated table. The comparand-type set was ruled at the shared face with no ledger entry. An unknown operator has no meaning to migrate to. An array filter is refused because the slot's declared type, FilterConditionSchema, admits no array form, exactly as `having`'s condition check already refuses one, and the REST door already refused it. A stored measure filter reaches this position through the analytics service, which lowers it; the refused inputs are ones that position never evaluated as written, so there is no rewrite for `objectstack migrate meta` to perform. The table below is the author-facing remedy for each row, not a mechanical rewrite. -->

**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. The refusal names the aggregation that carries the filter (`aggregations[1].filter`). It ships as `minor` under the launch-window convention for accept-set narrowings.

The engine evaluates a per-aggregation filter itself, in the in-memory fallback, once per source row of each bucket, with the same walker `having` uses. So the walker's refusals were reached only when a row was. Measured on the base through `engine.aggregate` on `driver-memory` and `driver-sql`, and through `POST /data/:object/query` on both, over six rows in three groups, with `{ function: 'count', alias: 'n', filter }`:

| you wrote in `aggregations[i].filter` | what it did before | write instead |
|:--|:--|:--|
| an unknown or retired operator (`{ amount: { $median: 1 } }`, `$nand`, `$regex`, `$regex` with `$options`), an operator of `where` the walker does not evaluate (`$like`, `$ilike`), a non-`$` key beside an operator, or an empty or non-string `$icontains` | refused on a populated table only. An empty table answered `200 []` with a `groupBy`, and `[{ n: 0 }]` without one. (At the REST door an empty or non-string `$icontains` was already refused at ingress, whatever the rows.) | the operator the refusal names |
| an unknown operator on a column the source row does not carry (`{ nope: { $median: 1 } }`) | counted no row, with no error, on a populated table too | the operator the refusal names |
| an unknown operator in a `$or` branch after one that held (`{ $or: [{ amount: { $gt: 0 } }, { amount: { $median: 1 } }] }`) | counted EVERY row on a populated table: the walk stopped at the branch that held | the operator the refusal names |
| `{ amount: { $field: 'cap' } }` (a reference with no operator) | refused as an unsupported `$field` operator on a populated table only | `{ amount: { $eq: { $field: 'cap' } } }`, or `$ne` / `$gt` / `$gte` / `$lt` / `$lte` |
| a `{ $field }` reference as an `$in` / `$nin` member, a `$contains` pattern, or an `$exists` operand | compared the reference object itself: no row under `$in` / `$contains`, every row under `$nin` / `$exists` | a literal there, or the comparison as one of the six scalar operators |
| a `{ $field }` reference whose `addDays` is not an integer (`1.5`, `'7'`) | counted rows by the in-memory evaluator's own reading of that offset, which `FieldReferenceSchema` refuses | a whole-day `addDays` |
| `{ amount: { $eq: { v: 1 } } }`, `{ amount: undefined }`, `{ $eq: new Map() }`, a function, an `undefined` `$in` member, a bigint beyond 2^53, or `{ $gt: { $field: 5 } }` | counted no row (each measured under the operator shown, or in the implicit slot). The same comparand in `where` is refused by the comparand-type door; the per-aggregation filter now gets that door's refusal, rooted at its own position | a string, number, bigint, boolean, `null` or `Date` |
| a `Symbol` comparand | under `$ne`, counted every row; under `$gt`, threw a raw `TypeError` with no `code` and no `status` on a populated table | a literal of one of the types above |
| a `filter` that is not a filter object: a string (`"stage = 'won'"`), a number, `true` / `false`, `''`, a `Map`, a `Date` or a `Set` | dropped: the aggregation read every row of its group, with no error. `driver-sql`'s native aggregate answered a non-empty string with `NOT_IMPLEMENTED` / 501. (The REST door already refused the JSON-expressible ones, through `AggregationNodeSchema`.) Now refused by the shape gate `where` takes, with the aggregation named (`'aggregations[1].filter' must be a filter object, received …`) | a filter object, `{ stage: 'won' }` |
| an array, `[]` included: a condition array (`[['amount', '>', 100]]`, `['and', …]`) or an empty one | a condition array counted NO row: the walker read its index positions as column names. `[]` was read as no filter. The REST door already refused every array here with `VALIDATION_FAILED` / 400. Now refused in-process too (`'aggregations[1].filter' must be a filter object, received an array (…)`), because `AggregationNodeSchema.filter` is declared `FilterConditionSchema`, which admits no array form: the condition-array sugar is lowered on `where` alone | the object form, `{ amount: { $gt: 100 } }`; omit `filter` for no filter |

Not refused, but answering differently:

- **An exact-range bigint comparand is narrowed to a number, as it is in `where`.** `{ amount: { $in: [400n, 20n] } }` counted no row, because `[400n].includes(400)` is false. It now counts the rows it names. The caller's aggregation entry is not edited.

Who is affected: a per-aggregation filter reaches `engine.aggregate` from a direct engine call, from the REST aggregate query, and from the analytics service, which lowers a dataset measure's own `filter` onto it. On a populated table the refusals in the first and fourth rows above were already raised; what changes there is that an empty table refuses them too. The two shape rows are reachable in-process only: the REST door already refuses a non-object `filter` and every array. Callers in a deployment were NOT measured.

Not changed: implicit equality, scalar ordering bounds, `$in` / `$nin` lists, a two-bound `$between`, `$icontains` / `$startsWith` with a non-empty string, `$ne: null`, `$exists`, `$null`, `$or` / `$not` composition, a `{ $field }` reference as the whole comparand of a scalar comparison (with or without a whole-day `addDays`), an exact bigint in the implicit slot, and `{}`, on both drivers, measured identical before and after. The zero-row filter `{ $not: {} }`, which the analytics service lowers FALSE to, still counts no row (a unit pin, green against the base code too). `null`, an absent `filter` and a null-prototype filter object are not refused by the shape gate, as they are not on `where`, and answer as before.
28 changes: 28 additions & 0 deletions .changeset/20123-having-unknown-column-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
"@objectstack/objectql": minor
---

fix(objectql)!: a `having` key that names no column of the aggregated row is refused on `engine.aggregate`, instead of answering as if that column had no value in every group (#20123)

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. A `having` key that names no column of the aggregated row never had a meaning: the rows carry exactly the columns the query projects, so such a key could only ever read "no value". There is no accepted spelling it can be mechanically rewritten to — which of the query's columns the author meant is an authoring decision, and the refusal prints the list. `having` is a request-only key: no metadata type stores it, so 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. -->

**BREAKING**: this narrows what `having` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query`) that forwards it there. A `having` key — at any depth under `$and` / `$or` / `$not` — must name a column of the aggregated row: a groupBy projection (the field name, or a structured item's `alias`) or an aggregation alias. Any other key is refused with `INVALID_FILTER` / 400, once per query, before any driver is asked for a row, on both the native `driver.aggregate()` path and the in-memory fallback, whether or not any group exists. It ships as `minor` under the launch-window convention for accept-set narrowings.

The engine evaluates `having` itself, per aggregated row, and read a key the row does not carry as a column with no value. So a typo for an alias answered like a real query. Measured on the base through `engine.aggregate` on `driver-memory` and `driver-sql`, both paths, and through `POST /data/:object/query` on both, over three groups by `customer_id`, each with a positive `total` (a `sum` alias) beside a `count` alias `n`:

| `having` | before | now |
|:--|:--|:--|
| `{ totl: { $gt: 100 } }`, `{ totl: 500 }`, or `{ amount: { $gt: 100 } }` (a source column the aggregated row does not project) | no group, no error | refused, naming the key, its position and the query's columns |
| `{ totl: { $ne: 1 } }`, `{ totl: { $exists: false } }`, or `{ $not: { totl: { $gt: 100 } } }` | EVERY group, no error | refused |
| `{ $or: [{ total: { $gt: 0 } }, { totl: { $gt: 100 } }] }` | every group whose `total` is positive: the walk stopped at the branch that held | refused |
| `{ $and: [{ total: { $gt: 0 } }, { totl: { $gt: 100 } }] }` | no group | refused |
| `{ 'customer_id.name': 'c1' }` (a dotted path) | no group | refused |
| `{ customer_id: 'c1' }` when the groupBy item is `{ field: 'customer_id', alias: 'cust' }` | no group: the row projects `cust` | refused; `{ cust: 'c1' }` answers |

The refusal opens the way the REST ingress's refusal of an unknown `where` field does ("filters on 'totl' … which is not a column of the aggregated row"), names every unknown key, and lists the aggregated row's columns. Its code is `INVALID_FILTER`, the code of every other `having` refusal: the name is a column of the query's own projection, not a field of the object. It is judged after the rest of the clause: a condition on an unknown column that also carries an unknown operator (`{ nope: { $median: 1 } }`) is still refused for its operator first, as before.

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 names an aggregation alias of its own query (`{ 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.

Not changed: a key naming a groupBy column, a structured item's alias, a `count` / `sum` / `max` alias, or any of those under `$and` / `$or` / `$not`, answers exactly as before on both paths, measured identical before and after.
34 changes: 34 additions & 0 deletions .changeset/20127-having-adddays-temporal-pair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"@objectstack/objectql": minor
---

fix(objectql)!: in `engine.aggregate({ having })`, a `{ $field, addDays }` reference is evaluated only between two temporal columns of one class, with a numeric offset column, as `FieldReferenceSchema.addDays` declares, instead of answering by epoch-millisecond coercion (#20127)

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. It enforces on `having` the rule `FieldReferenceSchema.addDays` already declares ("between two temporal columns of the same class (date/date, datetime/datetime)", the offset a whole number or a numeric column), which `driver-sql` already enforces on `where`. A refused pair never had the meaning the declaration gives `addDays`: on a number, a text or a mixed pair, it was answered by the in-memory evaluator's coercion. There is no accepted spelling it can be mechanically rewritten to: whether the author meant two date columns, no offset, or a different column is an authoring decision. `having` is a request-only key: no metadata type stores it, so there is no stored document for `objectstack migrate meta` to rewrite. The table below records the answer each pair had and has; it prescribes no rewrite. -->

**BREAKING**: this narrows what `having` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query`) that forwards it there. A `{ $field }` reference that carries `addDays` in one of the six scalar comparisons is now refused with `INVALID_FILTER` / 400 unless the column it filters and the column it references are both `date` or both `datetime`, and an `addDays` offset read from a column reads a numeric one. The refusal is raised once per query, before any driver is asked for a row, on both the native `driver.aggregate()` path and the in-memory fallback, whether or not any group exists. It ships as `minor` under the launch-window convention for accept-set narrowings.

An aggregated row has no declared field types, so each column's class is now read off the query and the object's declaration, before any row exists:

- a groupBy projection takes its field's declared type. A `day` date bucket is a `date`, because its label is `YYYY-MM-DD` on every face. A `week`, `month`, `quarter` or `year` bucket is a text label;
- `count`, `count_distinct`, `sum` and `avg` are numeric;
- `min` and `max` take the type of the field they read.

A column whose class the declaration cannot tell is not judged: an object with no field map, a field it does not declare, or a `formula` field.

`having` resolved every pair through `@objectstack/formula`'s evaluator, which reads a number as epoch milliseconds, while `driver-sql` refuses the same pair on `where`. The refusal reuses `driver-sql`'s sentences for the pair, naming each aggregated column's class where `driver-sql` names a stored type ("is numeric" for "is stored as numeric"), because an aggregated column is computed rather than stored. Measured on the base through `engine.aggregate` on `driver-memory` and `driver-sql`, both paths, and through `POST /data/:object/query` on both, over three groups with a `sum` alias `total`, a `max` of a number `max_cap`, `max` / `min` of two `date` fields, `max` / `min` of two `datetime` fields and a `count` `n`:

| `having` | before | now |
|:--|:--|:--|
| `{ total: { $gt: { $field: 'max_cap', addDays: 1 } } }` (two numeric columns) | no group, no error | refused: "addDays adds whole days to a date or datetime column, and "max_cap" is numeric — an offset has no meaning on it." |
| `{ n: { $gte: { $field: 'n', addDays: 0 } } }` (a count against itself) | every group | refused, in the same words |
| a `date` column against a numeric column, a numeric column against a `date` one, or the `customer_id` groupBy text column against a `date` one | no group | refused, naming both columns and their classes: "… and a cross-class comparison answers differently in SQL (storage-class ordering) than in memory (JS coercion) — compare same-class columns." |
| a `date` column against a `datetime` one | one group | refused, in the cross-class words |
| a `datetime` column against a `date` one | two groups | refused, in the cross-class words |
| a `date` pair whose `addDays` reads a text column or a `date` column | no group | refused: "the addDays offset … is not a numeric column, and a day offset must be a number of days." |

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.

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.
Loading
Loading