Skip to content

Commit a17770a

Browse files
committed
fix(objectql)!: a per-aggregation filter that is not a filter object is refused (#20122)
The seat's amendment to the combined claim folds the per-aggregation filter's shape door into this card. In the `aggregations[i].filter` loop, a filter that is neither absent, `null`, a filter object nor an array (a string, a number, a boolean, `''`, a `Map`, a `Date`) was stepped around by every door there and then dropped: the aggregation read every row of its group, with no error. The loop now opens with the shape gate `where` takes on its own seam, reusing its accept set (`isWhereFilterObject`) and its words (`describeNonFilterWhere`), with a consequence clause for this position, INVALID_FILTER / 400 before any driver call. An array is left to the doors below, as `where` leaves one to its array branch. The note that excluded this gate from the loop is removed. Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3c2fa82 commit a17770a

3 files changed

Lines changed: 74 additions & 8 deletions

File tree

‎.changeset/20122-aggregation-filter-row-independent.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"@objectstack/objectql": minor
33
---
44

5-
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 comparand-type door `where` takes (#20122)
5+
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)
66

77
Clause-②: no (narrowing)
88

@@ -22,11 +22,12 @@ The engine evaluates a per-aggregation filter itself, in the in-memory fallback,
2222
| 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` |
2323
| `{ 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` |
2424
| 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 |
25+
| 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 (`'where' must be a filter object or condition array, received …`), in the same words, naming the aggregation | a filter object, `{ stage: 'won' }` |
2526

2627
Not refused, but answering differently:
2728

2829
- **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.
2930

30-
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. Callers in a deployment were NOT measured.
31+
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 shape row is reachable in-process only: the REST door already refuses a non-object `filter`. Callers in a deployment were NOT measured.
3132

32-
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, the zero-row filter `{ $not: {} }`, and `{}`, on both drivers, measured identical before and after. A `filter` that is not a filter object at all (a string, a number, a boolean, a `Map`, an array) is answered as before.
33+
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, the zero-row filter `{ $not: {} }`, and `{}`, on both drivers, measured identical before and after. `null`, an absent `filter`, a null-prototype filter object and an array are not refused by the shape gate, as they are not on `where`: `[]` is still read as no filter, and a condition array (`[['amount', '>', 100]]`) is still read by the walker as an object whose index keys are column names, so it counts no row, as before — `AggregationNodeSchema.filter` does not declare that sugar, and the REST door refuses it.

‎packages/objectql/src/engine-aggregate-filter.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,3 +523,54 @@ describe('[#20122] per-aggregation filter — what the walk leaves alone answers
523523
});
524524
}
525525
});
526+
527+
describe('[#20122] per-aggregation filter — the shape gate `where` takes: a filter that is not a filter object is refused', () => {
528+
// Before, measured through `engine.aggregate` on driver-memory and
529+
// driver-sql: each shape was DROPPED — the aggregation counted every row of
530+
// its group, on a populated table, with no error (driver-sql's native
531+
// aggregate answered the non-empty string with NOT_IMPLEMENTED / 501). The
532+
// REST door refuses each one already, through `AggregationNodeSchema`.
533+
const SHAPE_REFUSED: ReadonlyArray<readonly [string, () => unknown, string]> = [
534+
['a string', () => "stage = 'closed_won'", `received string "stage = 'closed_won'"`],
535+
['a number', () => 42, 'received number 42'],
536+
['a Map', () => new Map([['stage', 'closed_won']]), 'received Map'],
537+
['true', () => true, 'received boolean true'],
538+
['the empty string', () => '', 'received string ""'],
539+
['a Date', () => new Date(0), 'received Date'],
540+
];
541+
542+
for (const [name, filter, received] of SHAPE_REFUSED) {
543+
it(`${name}: refused before any read, whatever the rows`, async () => {
544+
const message = await expectFilterRefusal(filter);
545+
expect(message).toContain(`aggregate('crm_opportunity'): '${AT}' must be a filter object or condition array, ${received}.`);
546+
expect(message).toContain('It was not applied');
547+
});
548+
}
549+
550+
it('a filter object answers as before — a literal and a null-prototype one alike', async () => {
551+
for (const native of [true, false]) {
552+
for (const filter of [{ stage: 'closed_won' }, Object.assign(Object.create(null), { stage: 'closed_won' })]) {
553+
const { driver } = makeCountingDriver(OPPORTUNITIES, native);
554+
const engine = await makeEngine(driver);
555+
expect(await engine.aggregate('crm_opportunity', withFilter(filter))).toEqual([{ opp_count: 6, picked: 2 }]);
556+
}
557+
}
558+
});
559+
560+
it('an array is not this gate\'s to refuse, as it is not on `where` — `[]` and a FilterArray answer as before', async () => {
561+
// Held, not endorsed: the FilterArray's answer is the walker's reading of an
562+
// array (its index keys read as column names, so no row matches) — the
563+
// pre-existing reading, left for its own decision; `[]` is the vacuous
564+
// filter. What this pins is that the shape gate moves neither.
565+
for (const native of [true, false]) {
566+
const { driver } = makeCountingDriver(OPPORTUNITIES, native);
567+
const engine = await makeEngine(driver);
568+
expect(await engine.aggregate('crm_opportunity', withFilter([['amount', '>', 100]]))).toEqual([{ opp_count: 6, picked: 0 }]);
569+
}
570+
// `[]` is read as no filter and so keeps the native push-down; the stand-in's
571+
// native door computes nothing, so it is read on the fallback door here.
572+
const { driver } = makeCountingDriver(OPPORTUNITIES, false);
573+
const engine = await makeEngine(driver);
574+
expect(await engine.aggregate('crm_opportunity', withFilter([]))).toEqual([{ opp_count: 6, picked: 6 }]);
575+
});
576+
});

‎packages/objectql/src/engine.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16090,6 +16090,25 @@ export class ObjectQL implements IObjectQLEngine {
1609016090
for (const [i, agg] of aggregations.entries()) {
1609116091
const aggFilter = (agg as { filter?: unknown })?.filter;
1609216092
if (aggFilter == null) continue;
16093+
// [#20122] The SHAPE gate `where` takes first on its own seam
16094+
// (`lowerWhereFilterArray`, #20121) — first here too, with its
16095+
// accept set and its words. A filter that is not a filter object
16096+
// (a string, a number, a boolean, `''`, a `Map`, a `Date`) was
16097+
// stepped around by every door below, each of which walks a filter
16098+
// node's keys, and then dropped: the fork below reads it as "no
16099+
// filter", so the aggregation read every row of its group
16100+
// (`driver-sql`'s native aggregate answered a non-empty string
16101+
// with a 501 instead). The wire door already refuses these shapes
16102+
// through `AggregationNodeSchema`; this closes the in-process one.
16103+
// An array is left to the doors below, as `where` leaves one to
16104+
// its array branch.
16105+
if (!Array.isArray(aggFilter) && !isWhereFilterObject(aggFilter)) {
16106+
throw invalidFilterError(
16107+
`aggregate('${object}'): 'aggregations[${i}].filter' must be a filter object or condition ` +
16108+
`array, received ${describeNonFilterWhere(aggFilter)}. It was not applied, and an ` +
16109+
`unapplied filter would have aggregated every row of each group for that aggregation.`,
16110+
);
16111+
}
1609316112
assertListComparandShapes(object, 'aggregate', aggFilter, `aggregations[${i}].filter`);
1609416113
assertFilterIsMaterializable(object, 'aggregate', this._registry.getObject(object), aggFilter);
1609516114
// [#15661] …and the declared-type door for the text operators: a
@@ -16112,11 +16131,6 @@ export class ObjectQL implements IObjectQLEngine {
1611216131
// walks the filter per SOURCE row, so `{ amount: { $median: 1 } }`
1611316132
// was a 400 on a populated table and a `200 []` on an empty one,
1611416133
// and a `$or` whose first branch held counted every row.
16115-
// ⛔ Not the condition-object check `having` got: a `filter` that is
16116-
// not a filter node at all (a string, a number, a `Map`) is a
16117-
// question about the clause's SHAPE, answered by the shape door
16118-
// for every filter position at once — not a refusal that depends
16119-
// on the data, which is what this block closes.
1612016134
const typed = normalizeFilterComparandTypes(aggFilter, `aggregate('${object}')`, `aggregations[${i}].filter`);
1612116135
assertAggregationFilterIsEvaluable(typed, i);
1612216136
if (typed !== aggFilter) {

0 commit comments

Comments
 (0)