Skip to content

Commit aa04ea2

Browse files
fix(objectql)!: route having through the shared comparand-shape face — having: { total: [5] } is refused like the same shape in where (#20097)
Fixes #19974 Clause-②: no (narrowing) This routes the `having` clause of `engine.aggregate` through the shared comparand-shape face. It executes ruling 乙 (record 5793368540 on #19757, 「217 同意」) at the one position that ruling's face did not reach yet. The triage execution point, verbatim: 「钉子:`having: { total: [5] }` 和 `having: { total: { $eq: [5] } }` 被拒绝;标量照常通过。」 Session `session_01Bvd69VPa6puiNzzPUroDBx`, branch `claude/issue-19974-having-array-equality`. Base `9b8c74c6c5`, head `5649560428`. Every reading below was taken on one of those two commits, as stated. ## 1. Measured first, on the base `9b8c74c6c5` Every filter was run through the public `engine.aggregate` twice, once as a `where` and once as a `having`, with the same bytes each time. Two drivers were used: `driver-memory` and `driver-sqlite-wasm`. Both have a native `aggregate()`. Each `having` was run on both `applyHaving` doors. The native door is the `driver.aggregate()` path. The in-memory fallback door was forced by adding a per-aggregation filter. The data was grouped by customer: c1 has total 500 over 2 rows, c2 has 1250 over 3, and c3 has 20 over 1. Both drivers and both doors gave the same answer in every row. Every `where` was refused with `INVALID_FILTER` / 400 and the `aggregate('order'): ` prefix: | arm of the face | the shape, as a `having` | `having` answered | |:--|:--|:--| | equality-slot array (the triage shape) | `{ total: [500] }` | c1, because JS `500 == [500]` is true | | equality-slot empty array | `{ total: [] }` | no group | | `$eq` array (the triage shape) | `{ total: { $eq: [500] } }` | c1 | | equality-slot array under `$or` / `$and` | `{ $or: [{ order_count: 99 }, { total: [500] }] }` | c1 | | equality-slot array under `$not` | `{ $not: { total: [500] } }` | c2, c3 | | scalar `$in` / `$nin` | `{ customer_id: { $in: 'c1' } }` | `$in`: no group. `$nin`: every group | | null list member | `{ customer_id: { $in: ['c1', null] } }` | `$in`: c1. `$nin`: c2, c3 | | null ordering comparand | `{ total: { $gt: null } }` | `$gt` / `$gte`: every group. `$lt` / `$lte`: none | | malformed `$between` | `{ total: { $between: 500 } }`, `[500]` | the scalar kept every group; the one-bound list kept c1, c2 | | null / blank `$between` bound | `[null, 1000]`, `['', 1000]`, `[undefined, 1000]` | c1, c3 | | `{ $field }` `$between` bound | `[{ $field: 'order_count' }, 1000]` | c1, c3 | That is 20 rows, counting each operator spelling, and all 20 were refused on `where` and answered on `having`. Six controls gave the same rows on the base and at head: a scalar, `$eq` with a scalar, an `$in` list, a two-bound `$between`, `$eq: null`, and `$ne` with an array. The `$ne` row is not an arm of the face, see §4. - **H1 confirmed.** `having-filter.ts` sends an array into its implicit-equality arm (`value == condition`), and its `$eq` arm compares with `!=`. - **H3 confirmed, and wider than the equality slot.** The face refused 20 shapes on `where`, and `having` refused none of them. ## 2. What changed - **`packages/objectql/src/engine.ts`, `ObjectQL.aggregate`.** One call was added: `assertListComparandShapes(object, 'aggregate', query.having, 'having')`. - It sits after the per-aggregation filter loop and before `getDriver`, the middleware chain, and either door. - It is the same objectql wrapper that `where` (inside `lowerWhereFilterArray`) and `aggregations[i].filter` already call on this verb, and it delegates to `@objectstack/spec/data`'s `assertListComparandShapes`. - ⛔ No second face was added, and `having-filter.ts` is untouched. - **`packages/objectql/src/engine-aggregate-having-comparand-shape.test.ts`**, new, 37 tests. - **`.changeset/19974-having-comparand-shape-face.md`.** ## 3. The doors that reach `applyHaving` (H2) There are exactly two call sites in the repository. Both are inside `ObjectQL.aggregate` in `engine.ts`: - the native door: `return applyHaving(aggregated, ast.having)` after `drv.aggregate(...)`; - the in-memory fallback door: `return applyHaving(applyInMemoryAggregation(raw, ast, tz), ast.having)`. The PM's "earlier fork" near the first door is `having: query.having` on `opCtx.ast`. That line puts the clause where `plugin-security`'s predicate guard can walk its field references. It evaluates nothing, and nothing writes `ast.having` afterwards (`git grep` finds no `.having =` assignment in any package source). `applyHaving` and `matchesHaving` are not exported from `@objectstack/objectql`'s `.` or `./core` entry, and no driver reads `having`. The REST aggregate query forwards `having` to `engine.aggregate`, in `metadata-protocol`'s `protocol.ts`. So **one gate ahead of both doors** covers every door. It also judges the FILTER rather than the rows. `applyHaving` evaluates per aggregated row, so a gate inside the walker would stay silent on an empty grouped set. A pin covers that case. ## 4. The refused set, `where` against `having` (H3) | | `where` | `having` | |:--|:--|:--| | before (`9b8c74c6c5`) | 20 of 20 refused | 0 of 20 refused | | after (`5649560428`) | 20 of 20 refused | 20 of 20 refused, with the face's own words; the only difference from `where` is `having.` in place of `where.` in the path | The test's parity table asserts both halves on every row, on both doors: - `havingErr.message === whereErr.message.replaceAll('where.', 'having.')`; - `whereErr.message` equals the face's own refusal, so the row cannot be refused by some other gate; - neither door asked the driver for a row. **`$ne` with an array** is not an arm of the face. The ruling left it out, and #19886, which is ruled and dispatched, carries it. On `where` it is refused one layer down, by the drivers themselves: `driver-memory` gives the refusal without the `aggregate('order'): ` prefix, and `driver-sqlite-wasm` withholds the detail. On `having` it still answers c2, c3 by `==` coercion. A test holds `having` to whatever the face answers for it. When the `$ne` arm lands on the face, `having` refuses it through this same call, and that pin stays green in both states. ## 5. Changeset (H4) `@objectstack/objectql: minor` with `**BREAKING**`, `fix(objectql)!:`, `Clause-②: no (narrowing)` and one ADR-0087 marker: `not-required (already-registered filter-equality-array-comparand-refused, filter-between-blank-endpoint-refused, filter-between-field-reference-endpoint-refused)`. The gates' own lines: - `check-adr-0087-registration`: `✓ ... 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.` and `.changeset/19974-having-comparand-shape-face.md [BREAKING+bang+clause-②-narrowing] not-required (already-registered)` - `check-changeset-no-major`: `✓ This diff introduces no major bump.` Its clause-② level axis prints NOT APPLICABLE locally, because there is no `pull_request` payload; CI reads it from this body. ## 6. Tests, ablation, gates All counts below are on head `5649560428`, except the objectql local project, which ran on `0052ce29c1`. `engine.ts` has the same blob (`7830b0cee3`) at both commits, and the only later change is the new test file, which was re-run at head. - `@objectstack/objectql`, `vitest run --project local`: **313 files, 5320 passed**. `--project repo`: 1 file, 5 passed. - `@objectstack/objectql typecheck`: exit 0. `check:test-typecheck` is OK with 40 files and 234 errors held, unchanged. `tsc -p tsconfig.test.json --listFiles` includes the new test file (1 hit, 314 test files in the program). - New file alone: 37 passed. It sits beside the existing `engine-aggregate-having.test.ts` (5 passed). - Consumer suites that assert `having` behaviour, found by grep: - `metadata-protocol` `protocol.query-param-arity.test.ts`: 46 passed; - `plugin-security` `predicate-guard.test.ts`: 10 passed; - `rest` `list-view-grouping-query-door.test.ts`: 33 passed, with the objectql dist rebuilt and `rest`'s dependency closure built. - The real-driver measurement of §1, repeated at head: every one of the 20 rows is refused on `having` on both drivers and both doors, and the six controls give byte-identical output to the base. **Ablation.** The one gate line was replaced through `scripts/ablation-replace.mjs`: - the anchor hit once, and the blob moved `7830b0cee3` → `380a8f6fd9`; - `grep -c` read the anchor 1 → 0 and the marker 0 → 1; - an EXIT/INT/TERM trap restored the file with `git checkout HEAD --`, proven by the HEAD blob-hash match and an empty `git diff HEAD`. The test imports `./engine.js` from source, so no `dist/` is on the resolution path and there was nothing to preflight. **24 red, 18 green**, as expected: - red: all 20 parity rows, the 3 shared-table `door-refusal` rows, and the empty-set pin; - green: the 10 pass-through controls, the `$ne` pin that follows the face, the 2 partition checks, and the 5 existing `having` tests. **Gates.** `dispatch-gates --commands --repo objectstack-ai/objectstack`, re-derived at head, printed 64 families. `--ran` reconciled them: **64 derived, 62 run with exit 0, 2 NOT MEASURED, 0 UNRUN**. - NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt`, both exit 3 PREREQUISITE NOT MET. Both need the whole workspace built; CI runs them. - `check:query-options-erasure` went red once on an earlier head, at 236 → 243 test sites, from `as any` casts in the new test. The options are typed now, and the off-contract bags are cast through `unknown` to `EngineAggregateOptions`. It holds at 236. - `node scripts/check-issue-citations.mjs --base 9b8c74c`: `✅ every citation this change adds resolves`. **Lint, a declared narrowing** (`pnpm lint` is CI's). `eslint --no-inline-config --format json` was run over the two changed `.ts` files: - population: `eslint --print-config` returns a config for each file, so neither is ignored; - count: 2 files, 0 errors, 0 warnings; - invariance: `eslint.config.mjs` sets no `parserOptions.project` and no typed rule, so no rule is type-aware, and this diff cannot move a verdict on any untouched file. ## 7. Compile surfaces, face by face This card is the `having` half-face. | face | conclusion | |:--|:--| | 1 `driver-sql` (and `driver-sqlite-wasm`, local `driver-turso`) | not reached by `having`: no driver reads the clause, and the engine evaluates it after aggregation. Unchanged | | 2 turso `RemoteTransport` | not reached by `having`. Unchanged | | 3 `read-scope-sql` | not reached by `having`. Unchanged | | 4 analytics `filter-normalizer` | not reached by `having`; analytics sends no `having` to the engine. Unchanged | | 5 `formula` | not reached by `having`. Unchanged | | half-face objectql `having-filter` | **changed**: behind the shared comparand-shape face on both `applyHaving` doors, with a where/having parity table and a leg driven from `FILTER_COMPARAND_TYPE_CASES` | | `driver-memory` / `driver-mongodb` | not reached by `having`. Unchanged | `compile-surfaces.md` is governed (`.claude/**`), so it is not edited here. The row it should gain is in the report on #19974, and the seat routes it. ## Acceptance notes Observed and not fixed here. The report carries each one. Four of them are reproducible defects, handed to the seat to file. All four were measured at head on `driver-memory`: 1. **`having` resolves no `{ $field }` reference.** - Example: `having: { total: { $gt: { $field: 'max_cap' } } }` keeps no group, where c1 (500 against 50) should pass. - The face's `{ $field }` `$between` refusal, which `having` now shows, suggests the two-bound `{ $field }` spelling. `having` answers that spelling wrong, silently. - The changeset says so instead of prescribing it. 2. **The comparand-TYPE door is not run on `having`.** `having: { total: { $eq: { v: 1 } } }` answers 200 with no group, where the same shape in `where` is `INVALID_FILTER` / 400. 3. **The FilterArray sugar in `having` answers no group.** `having: [['total', '>', 100]]` gives 200 with no group, where `where` lowers the same sugar and answers c1, c2. 4. **`having`'s own walker refusals depend on the data.** `having: { total: { $median: 1 } }` and an empty `$icontains` are refused on a populated grouped set, and answer 200 with no group on an empty one. Noted only, not filed: - `$ne` with an array still answers on `having`. #19886 remains open and carries it; see §4. - The ADR-0087 entry `filter-equality-array-comparand-refused` lists the doors that refuse the shape "at this release", and `having` is not among them. The entry is in `packages/spec`, outside this claim. Carrier: none. - `having-filter.ts`'s header still calls HAVING "the only face no conformance table covers". That is still true of the logic axis. The comparand-shape axis now has one. The file is not edited, to keep the claim. Carrier: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8d76c2d commit aa04ea2

3 files changed

Lines changed: 400 additions & 0 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: `engine.aggregate({ having })` walks through the shared comparand-shape face, so `having: { total: [5] }` is refused exactly as the same shape in `where` is (#19974)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (already-registered filter-equality-array-comparand-refused, filter-between-blank-endpoint-refused, filter-between-field-reference-endpoint-refused) this change adds no new transition. It puts the `having` clause of `engine.aggregate` behind refusals the shared comparand-shape face already makes for `where` and `aggregations[i].filter` on the same verb. The arms whose transitions are on the ledger are named: the equality-slot array, the blank `$between` endpoint and the `{ $field }` `$between` endpoint. The first two prescriptions apply to a `having` as written, and so does the third one's literal-bound half; its column-to-column half does not, because `having` resolves no `{ $field }` reference in any slot (a gap this change does not touch, stated in the table below). The null list member, the null `$between` endpoint and the null ordering comparand were ruled at the face with no ledger entry (their changesets declared no-migration-prescription), and the non-list `$in` / `$nin` / `$between` refusal is the face's original rule. `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 is the author-facing remedy for each arm, not a mechanical rewrite. -->
10+
11+
**BREAKING**: this narrows what `having` accepts on `engine.aggregate` (and on the REST aggregate query that forwards it there). A `having` carrying one of the shapes below used to answer; it is now refused with `INVALID_FILTER` / 400, before any driver is asked for a row. The refusal is the shared face's own message, byte for byte the refusal the same shape gets in `where`, with the path rooted at `having` instead of `where`. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
The 2026-09-23 ruling on #19757 refuses an array in the equality slot at the shared comparand-shape face (`assertListComparandShapes` in `@objectstack/spec/data`) "for every driver at once". The face already ran on `where` and on each `aggregations[i].filter`. `having` never reaches a driver: the engine evaluates it itself after aggregation, on both the native `driver.aggregate()` path and the in-memory fallback. That evaluator answered every shape the face refuses. Measured on the base through `engine.aggregate` on `driver-memory` and `driver-sqlite-wasm`, over three groups with totals 500, 1250 and 20:
14+
15+
| you wrote in `having` | what it did before | write instead |
16+
|:--|:--|:--|
17+
| `{ total: [500] }` or `{ total: { $eq: [500] } }`, at any depth under `$and` / `$or` / `$not` | kept the 500 group, because JS `500 == [500]` is true; under `$not` it kept the complement, the 1250 and 20 groups | `{ total: 500 }`, or `{ total: { $in: [500, 1250] } }` for "one of these" |
18+
| `{ total: [] }` | kept no group | drop the condition, or write the value you meant |
19+
| `{ customer_id: { $in: 'c1' } }` / `{ customer_id: { $nin: 'c1' } }` | `$in` kept no group; `$nin` kept every group | `{ customer_id: 'c1' }` / `{ customer_id: { $ne: 'c1' } }`, or wrap the value in a list |
20+
| `{ customer_id: { $in: ['c1', null] } }` (or `$nin`) | the null member was compared as a value | `{ $or: [{ customer_id: { $in: ['c1'] } }, { customer_id: { $null: true } }] }` |
21+
| `{ total: { $gt: null } }` (or `$gte` / `$lt` / `$lte`) | `$gt` / `$gte` kept every group; `$lt` / `$lte` kept none | `{ total: { $eq: null } }` for "has no value", `{ total: { $ne: null } }` for "has a value" |
22+
| `{ total: { $between: 500 } }` or `{ total: { $between: [500] } }` | the scalar kept every group; the one-bound list kept the groups at or above it | `{ total: { $between: [min, max] } }` |
23+
| `{ total: { $between: [null, 1000] } }`, `['', 1000]` or `[undefined, 1000]` | the blank bound compared as a value | `{ total: { $lte: 1000 } }` for a one-sided range, or the bound you meant |
24+
| `{ total: { $between: [{ $field: 'order_count' }, 1000] } }` | the reference compared as a value | literal bounds. ⚠️ The refusal's own text suggests a two-bound `{ $field }` comparison, which `having` does not evaluate. In an operator slot the reference is compared as a value: under `$eq`, `$gt`, `$gte`, `$lt` or `$lte` it keeps no group, and under `$ne` it keeps every group. In the implicit slot (`{ total: { $field: 'order_count' } }`) it is refused as an unsupported operator (`INVALID_FILTER` / 400), though only when a grouped row carries that column: an empty grouped set evaluates nothing and comes back empty. That gap is not changed here |
25+
26+
The gate is ONE call in `engine.aggregate`, ahead of both `having` evaluations, so the two paths cannot disagree, and the verdict belongs to the filter rather than to the data: an empty grouped set refuses the same `having` a populated one does. Whatever arm the shared face gains later, `having` gains with it.
27+
28+
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 (`{ order_count: { $gt: 5 } }` and the like), and none authors a refused shape. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
29+
30+
Not changed: scalars, `null` in the equality slot (the has-no-value predicate), `$in` / `$nin` lists including the empty list, a two-bound `$between`, and scalar ordering bounds all answer exactly as before, on both paths. `$ne` with a list is not judged by the face yet, so `having` still answers it. Neither the comparand-TYPE door nor the unknown-field and declared-type gates that `where` also passes are run on `having`; this change adds the comparand-shape face only.

0 commit comments

Comments
 (0)