Repository navigation
Commit fc646cf
fix(objectql)!:
Fixes #20099
Clause-②: no (narrowing)
This gives the `having` clause of `engine.aggregate` the rest of the
filter doors `where` takes, at the entry PR #20097 added. It follows the
dispatch's binding frame: ruling 乙 on #19757 (record 5793368540), the
seat default 协议为基准, and the ADR-0087 entry
`filter-between-field-reference-endpoint-refused`, whose prescription
「#5222 compiles on every face」 `having` now honours.
Session `session_01Bvd69VPa6puiNzzPUroDBx`, branch
`claude/issue-20099-having-where-doors`. Base `aa04ea2964`, head
`98abdf4ee3`. Every reading below was taken on one of those two commits,
as stated. Heads `01f091f523` and `f08f8953fc` correct changeset cells
and change no code or test: `engine.ts`, `having-filter.ts` and the test
file are byte-identical to `98abdf4ee3`.
## 1. Measured first (H1)
Each shape was run through the public `engine.aggregate` on a real
`InMemoryDriver` and a real `SqliteWasmDriver`. Each `having` ran on
both `applyHaving` doors: the native `driver.aggregate()` door, and the
fallback door forced by a per-aggregation `filter`. Each ran on a
populated and on an empty grouped set. That is 8 cells per shape and
version. Each shape's `where` twin, over the raw columns, was the
control. Groups: c1 (total 500, max_cap 50), c2 (total 900, max_cap
5000), c3 (total 20, max_cap 20).
**H1 holds: all four rows still held at base `aa04ea2964`.** In every
row, all 8 cells answered alike on both drivers and both doors, except
where the table below says otherwise.
| row | shape (as a `having`) | `where` twin | `having` at base |
`having` at head |
|:--|:--|:--|:--|:--|
| 1 | `{ total: { $gt: { $field: 'max_cap' } } }` | sqlite: resolved
rows; memory: none | no group | c1 |
| 1 | `$gte` / `$lt` / `$lte` / `$eq` against `{ $field: 'max_cap' }` |
sqlite: resolved | no group | c1,c3 / c2 / c2,c3 / c3 |
| 1 | `$ne` against `{ $field: 'max_cap' }` | sqlite: resolved | every
group | c1, c2 |
| 1 | the two-bound spelling `{ total: { $gte: { $field: 'max_cap' },
$lte: 1000 } }` | sqlite: resolved | no group | c1, c3 |
| 1 | a bare `{ total: { $field: 'max_cap' } }` | 400, both drivers |
400 populated, **200 [] empty** | 400 on every cell, the operator
written in |
| 1 | a reference as an `$in` / `$nin` member, a `$contains` /
`$notContains` pattern, an `$exists` / `$null` operand | sqlite: 400 |
no group, or every group | 400 on every cell |
| 1 | a reference naming no column, e.g. `{ $field: 'nope' }` | sqlite:
400 | per operator (the reference object itself was compared): no group
under `$eq`, every group under `$ne`; under an ordering operator, none
against a number column, and against a text or date column an answer
that follows each value's string order against the text `[object
Object]` | 400 on every cell, listing the columns |
| 2 | `{ total: { $eq: { v: 1 } } }`, `{ total: undefined }`, `$eq: new
Map()`, a function, an undefined `$in` member, a bigint beyond 2^53, `{
$field: 5 }` | 400, the comparand-type door | per operator, on the
numeric `total`: no group in the implicit slot (the non-object values)
and under `$eq` / `$gt` / `$gte` / `$in`; every group under `$ne` /
`$nin`; under `$lt` / `$lte` no group, except the bigint, which kept
every group | 400, the same door's words rooted at `having` |
| 2 | `{ total: { $in: [500n, 20n] } }` | narrowed, the rows | no group
| c1, c3 |
| 3 | `[['total', '>', 100]]`, `['total', '>', 100]`, `['and', …]` |
lowered, the rows | no group | 400 on every cell |
| 3 | `[]`, `'total > 100'`, `100`, a `Map` | `[]`: no filter; scalars:
every row (see Acceptance notes) | every group | 400 on every cell |
| 4 | `{ total: { $median: 1 } }`, `$nand`, `$regex`, an empty or
non-string `$icontains` | 400 | 400 populated, **200 [] empty** | 400 on
every cell, the walker's own words |
| 4 | `{ nope: { $median: 1 } }` (a column the row lacks) | 400 | **200
[] on both** | 400 on every cell |
| 4 | `{ $or: [{ total: { $gt: 0 } }, { total: { $median: 1 } }] }` |
400 | **every group** on a populated set, [] on an empty one | 400 on
every cell |
54 shapes were measured, 432 `having` cells per version. At base, 28
refusal cells were the walker's, raised after the driver had been asked
for rows (`aggregate 1 / find 0` or `0 / 1`), and 8 were PR #20097's
face. At head, all 272 `having` refusal cells are raised before any
driver call: `aggregate 0 / find 0`.
The controls gave the same bytes at base and head, on every cell: a
scalar `$gt`, an implicit scalar, a key naming no column, an `$in` list,
`$ne: null`, a plain-object implicit value, `{}`, a `$ne` reference on a
missing column, a `Date` bound and the bigint `500n`. The re-measure of
base after the change matched the first base measurement byte for byte
on all 46 original shapes.
## 2. Where `where` gets each door (H2)
Located by symbol, in `ObjectQL.aggregate` → `lowerWhereFilterArray`
(engine.ts). The same function serves `find` and `count`.
- **FilterArray lowering.** The array branch calls `isFilterAST` →
`parseFilterAST(where, context)` from `@objectstack/spec/data`.
`parseFilterAST` runs the shape face and the type door internally with
the path fixed at `where`: it has no root argument. So it cannot lower a
`having` without printing `where.` in `having`'s refusals, and
`packages/spec` is not changed here. More decisive: the spec does not
declare the sugar on this slot (§3).
- **The comparand-type normaliser.** The object branch calls
`normalizeFilterComparandTypes(where, context)`. The function takes a
`path` argument, so it runs on `query.having` as it stands, rooted at
`having`, with no spec change. It needs no column set: it judges
comparand types, not names.
- **Structural validation.** Four engine doors run on `where`:
`assertListComparandShapes`, already on `having` since PR #20097;
`assertFilterIsMaterializable`,
`assertTextOperatorTargetsAreStringCapable` and
`assertTemporalComparandsInterpretable`. The last three judge the
OBJECT's declared fields, a namespace `having` does not filter.
Unknown-operator refusals for `where` are raised by the drivers.
`having` never reaches a driver, so its structural check is its own
walker's, run once against the aggregated row's column set. That set is
read off the query: the groupBy projections, a structured item's `alias`
or `field`, and every aggregation alias.
## 3. What changed
- **`packages/objectql/src/engine.ts`, `ObjectQL.aggregate`, the
`having` entry only**, beside the shape-face call:
1. `assertHavingIsFilterCondition(query.having)`. `having` is null,
absent or a plain object; anything else is refused. `QuerySchema.having`
and `EngineAggregateOptions.having` both declare
`FilterConditionSchema`. Measured: both schemas refuse every array, `[]`
included. The FilterArray sugar is declared on the `where` slot alone
(`TransportFilterValueSchema`, 「`where` widens to the `FilterArray`
sugar here and ONLY here」). The wire door already refuses a `having`
array (`protocol.query-param-arity.test.ts`).
2. The existing `assertListComparandShapes(…, 'having')`.
3. `normalizeFilterComparandTypes(query.having, "aggregate('order')",
'having')`. A narrowed bigint replaces the clause copy-on-write, and the
caller's object is not edited.
4. `assertHavingIsEvaluable(having, aggregatedRowColumns(query.groupBy,
query.aggregations))`.
- **`packages/objectql/src/having-filter.ts`:**
- `assertHavingIsEvaluable` walks the whole clause once, the `$and` /
`$or` / `$not` walk `matchesHaving` takes. It raises the walker's own
refusals through the same constructors (`unknownOperator`,
`icontainsComparandError`), in the order the per-row walk would meet
them. It adds four `{ $field }` refusals: a bare reference, a reference
outside the six scalar comparisons, a reference its own
`FieldReferenceSchema` refuses (a malformed `addDays`), and a reference
naming no column. The per-row throws stay as the floor for a caller that
evaluates rows directly.
- `checkCondition` resolves a `{ $field }` reference that is the whole
comparand of `$eq` / `$ne` / `$gt` / `$gte` / `$lt` / `$lte` against the
row. The comparison is `@objectstack/formula`'s
`matchesFilterCondition`, the in-memory evaluator the SQL cross-field
compiler is held to row for row, handed a three-column probe so a flat
column name is never read as a dotted path. It supplies the NULL
totality and the whole-day `addDays` arithmetic, which are not copied
here.
- **The test file** `engine-aggregate-having-comparand-shape.test.ts`
extends PR #20097's where/having parity table (37 → 112 tests), both
doors, with an empty-grouped-set leg on every refusal.
- **`.changeset/20099-having-where-doors.md`.**
## 4. `$field` against the aggregated row (H3)
Resolved, not refused. Resolution honours the declared form,
`FieldReferenceSchema`, and the two-bound spelling the `$between`
refusal prescribes on `having`. It needs no data at judgement time:
position and name are checked against the query's own column set before
any row exists. A reference that cannot resolve is refused on an empty
set exactly as on a populated one. A reference that can resolve answers
`[]` on an empty set and rows on a populated one, like any filter. The
resolution runs the same function on both doors, and `applyHaving` is
the only evaluator on either.
## 5. Declaration (H4)
**The accept set only narrows.** Every shape accepted at head was
accepted at base, and no refusal at base is lifted. Row 2, row 3, row 4
and the four `{ $field }` refusals are narrowings. Rows 1 (resolution)
and 2 (bigint narrowing) change answers of inputs that were already
accepted: no group, or every group under `$ne`, becomes the rows the
filter names. That is a correction of answers, not a widening of what is
accepted, so `Clause-②: no (narrowing)` stands.
- `check-changeset-no-major --base aa04ea2`: `✓ This diff introduces
no major bump.` Its clause-② level axis reads NOT APPLICABLE locally (no
`pull_request` payload); CI reads it from this body.
- `check-adr-0087-registration --base aa04ea2`: `✓ 1
declared-breaking changeset(s), each carrying an ADR-0087 disposition.`
· `.changeset/20099-having-where-doors.md
[BREAKING+bang+clause-②-narrowing] not-required (already-registered)`.
- The ids are `filter-between-field-reference-endpoint-refused` (the
prescription `having` now honours, and the list-member position it
refuses), `filter-icontains-comparand-refused-at-parse` and
`filter-regex-options-retired`. The marker's prose names the transitions
no entry covers, and why none is needed: `having` is a request-only key,
and no stored document exists for `objectstack migrate meta` to rewrite.
## 6. Tests, reverse verification, ablation, gates
Head `98abdf4ee3` unless stated. It differs from `ce22319645`, where the
typecheck ran, by the changeset only.
- `@objectstack/objectql` `vitest run --project local`: **314 files,
5417 passed**. `--project repo`: 1 file, 5 passed.
- `@objectstack/objectql` `typecheck`, at `ce22319645`: exit 0.
`check:test-typecheck` reads OK, 40 files / 234 errors held, unchanged.
- The extended parity file: **112 passed**.
- **Consumer census.** `engine.aggregate` takes `having` from ONE
non-test source caller, `metadata-protocol` `protocol.ts` (the REST
aggregate branch). Its suites were run with objectql's dist rebuilt
(turbo, 24 tasks, 18 cached; dist carries `assertHavingIsEvaluable`, 2
hits):
- `metadata-protocol` `protocol.query-param-arity.test.ts`: 46 passed;
- `rest` `list-view-grouping-query-door.test.ts`: 33 passed;
- `plugin-security` `predicate-guard.test.ts`: 10 passed.
- **Docs and skills**: 5 `having:` occurrences in 3 files
(`skills/objectstack-query/rules/aggregation.md` ×2,
`content/docs/data-modeling/queries.mdx` ×2,
`content/docs/protocol/objectql/query-syntax.mdx` ×1). All 5 are scalar
comparisons against an alias, which answer exactly as before. The
control is PR #20097's census, which reported the same.
- **Reverse verification.** `engine.ts` and `having-filter.ts` were put
back to their BASE blobs (`a9ec130693`, `2514be70bb`) with `git restore
--source`, under an EXIT/INT/TERM trap.
- Head's test file then read **69 failed, 43 passed (112)**. Every new
arm was red. Green were PR #20097's rows, the no-clause controls and the
where-sugar control.
- The trap restored both files: HEAD blobs matched, and `git diff HEAD`
was empty.
- **Ablation**, one per new door, through
`scripts/ablation-replace.mjs`. In each, the anchor hit once, `x1 → x0`,
the blob moved, and the file was restored to its HEAD blob with `git
diff HEAD` empty. The tests import `./engine.js` from source, so no
`dist/` is on the resolution path.
| ablated | failed / 112 | red set |
|:--|:--|:--|
| `assertHavingIsFilterCondition(query.having)` | 9 | exactly the 9
not-a-condition rows |
| the type-door call (clause passed through as is) | 21 | the 10
type-door parity rows, the 10 `FILTER_COMPARAND_TYPE_CASES` type rows,
and the bigint narrowing |
| `assertHavingIsEvaluable(…)` | 25 | the 10 walker rows, the 14
reference refusals, and the column-list row |
| the resolution branch in `checkCondition` | 14 | the 12 resolution
rows, the NULL-semantics row, and the per-aggregation-filter row |
- **Gates.** `dispatch-gates --commands --repo
objectstack-ai/objectstack` was re-derived at head: 64 families. Each
was run and its exit code recorded, then reconciled with `--ran`: `✓ 64
derived famil(ies) accounted for — 62 run, 2 NOT-MEASURED (2 DERIVED
from a recorded exit 3)`.
- NOT MEASURED: `check:dual-build-cjs-loads` and
`check:type-check-debt`, both exit 3 PREREQUISITE NOT MET. They need the
whole workspace built, and CI runs them.
- Selected lines: `query-options-erasure ratchet holds: 67 unswept
non-test site(s)`; `check-nul-bytes: OK (scanned 9536 text file(s)…)`;
`check-engine-double-contract: OK`; `where-matcher conformance holds`;
`ObjectQL double limit conformance holds`; `doc authoring guard: 403
files clean`; `check-driver-memory-census: OK`.
- `node scripts/check-issue-citations.mjs --base aa04ea2`: `✅ every
citation this change adds resolves` (19 judged).
- **Lint, a declared narrowing** (`pnpm lint` is CI's). `eslint
--no-inline-config --format json` on the 3 changed `.ts` files:
- count: 3 files, 0 errors, 0 warnings;
- population: `--print-config` returns a config for each file;
- invariance: `eslint.config.mjs` sets no `parserOptions.project` and no
typed rule, so no untouched file's verdict can move.
## 7. Compile surfaces, face by face
| face | conclusion |
|:--|:--|
| 1 `driver-sql` (with `driver-sqlite-wasm`, local `driver-turso`) | not
reached by `having`: no driver reads the clause. Unchanged |
| 2 turso `RemoteTransport` | not reached. Unchanged |
| 3 `read-scope-sql` | not reached. Unchanged |
| 4 analytics `filter-normalizer` | not reached; analytics sends no
`having` to the engine, and the analytics `having` is outside this
claim. Unchanged |
| 5 `formula` | **consumed, not changed**: `matchesFilterCondition` now
also evaluates a `having` reference comparison. Its code is untouched |
| half-face objectql `having-filter` | **changed**: behind the
comparand-type door, the condition-object check and the row-independent
walker, on both `applyHaving` doors. `{ $field }` is resolved in the six
scalar comparisons. The where/having parity table now covers the type
door and the new arms |
| `driver-memory` / `driver-mongodb` | not reached by `having`.
Unchanged |
**The author's text is the wire text.** No source writes the clause. The
two greps, over non-test package sources:
- `git grep -nE "\.having\s*=[^=]"` finds no hit.
- `git grep -n having -- packages/plugins/plugin-security/src` finds one
reader, `predicate-guard.ts:70`, `collectConditionFields(ast.having,
out)`, which walks field names and writes nothing.
Every refusal added here runs before `executeWithMiddleware` in any
case. Every refusal the tests pin asserts `code` and `status`.
## 8. Deviations from the claim, declared
- **FilterArray on `having` is REFUSED, not lowered.** The claim's
surface says "FilterArray lowering", and H4 expected "lowered sugar".
The declared contract answered otherwise. `having` is
`FilterConditionSchema` on both `QuerySchema` and
`EngineAggregateOptions`, and both refuse an array. The sugar is
declared on `where` alone, and the protocol door already refuses a
`having` array. Lowering it would widen `having`'s contract, a protocol
change the frame rules out. It would also print `where.` in `having`'s
refusals, because `parseFilterAST` has no root argument. The report
carries this as an open question.
- **The per-aggregation `filter` resolves `{ $field }` too.** It shares
`checkCondition` with `having` (its module note: 「a predicate moved
between a driver `where`, a `having`, and a per-aggregation `filter`
must select rows by one rule」), and forking the walker by clause would
give one walker two rules. The bounded in-place exemption applies: the
same defect class, the same code path, no other claim on the file, and
the same gates. Measured on both real drivers, `count` with `filter: {
amount: { $gt: { $field: 'cap' } } }` went from c1 0, c2 0, c3 0 at base
to c1 2, c2 1, c3 0 at head. A test pins it. Its entry-level refusals
are NOT added: that loop is outside the claimed `having` entry. See
Acceptance notes.
## Acceptance notes
Observed and not fixed here. The report carries each one with its class
and evidence.
- **A per-aggregation `filter`'s walker refusals still depend on the
data.** `aggregations: [{ …, filter: { amount: { $median: 1 } } }]` is
`INVALID_FILTER` / 400 on a populated table and `200 []` on an empty
one, on both drivers. This is row 4's class, at the sibling position;
the fix is this PR's `assertHavingIsEvaluable` walk run per aggregation
filter, in the engine loop outside this claim.
- **An engine `where` that is a string, a number or a `Map` is
dropped.** `engine.find('order', { where: 'amount > 100' })` returns
every row on both drivers.
- **The engine's refusal of a non-filter `where` array carries no
envelope.** `where: [1, 2, 3]` throws with `code` and `status`
undefined, on both drivers.
- **`driver-memory` does not resolve a `where` `{ $field }` reference.**
`{ amount: { $gt: { $field: 'cap' } } }` answers `[]`, while
`driver-sqlite-wasm` answers the resolved rows.
- **A `having` key naming no column keeps no group, silently** (`{ totl:
{ $gt: 100 } }`). The engine knows the column set, so the same check as
the reference's could refuse it; it is not one of this card's rows.
- **`addDays` against a numeric aggregate follows the in-memory
evaluator**, which reads a number as epoch milliseconds: `{ total: {
$gt: { $field: 'max_cap', addDays: 1 } } }` keeps no group. SQL
push-down refuses that pair on `where`, and `driver-memory` does not
resolve it at all. The aggregated row carries no declared type to judge
the pair statically. The report records this as an open question.
- `$like` / `$ilike` are still refused on `having`. That is the
documented staging in `FILTER_OPERATORS`, carried by the follow-up on
#7536, and not a new gap.
- The `having-filter.ts` header still calls HAVING 「the only face no
conformance table covers」. That remains true of the logic axis
(`FILTER_LOGIC_CASES`). The shape and type axes are now covered by the
parity table. The header is not edited, to keep the claim.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>having takes the rest of where's filter doors — the comparand-type door, row-independent refusals, a resolved { $field }, and a refused non-condition (#20117)1 parent 180ef90 commit fc646cf
4 files changed
Lines changed: 755 additions & 14 deletions
File tree
- .changeset
- packages/objectql/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
0 commit comments