Skip to content

Commit a78f731

Browse files
fix(objectql)!: having takes the temporal-comparand door where and the per-aggregation filter take (#20263) (#20307)
Fixes #20263 Clause-②: no (narrowing) ## What this does `engine.aggregate` evaluates `having` itself, so no driver reads it, and none of the doors in front of it judged a temporal comparand. A comparand the aggregated column's storage rule cannot read was compared as written: `having { last_placed: { $lt: "not-a-date" } }` on `max(placed_on)` kept every group with a 200, while its `where` twin answered 400. `having` now takes the temporal-comparand door that `where` (#8690) and the per-aggregation `filter` (#20148) take: the same walk and the same `@objectstack/core` predicate (`isUninterpretableTemporalComparand`). When #20264 moves the year range in that predicate, `having` follows with no second edit (pinned: see "one predicate" below). - `packages/objectql/src/temporal-comparand-door.ts`: the walk is factored into one `walkCondition` with a per-position scope (the kind of the column a key names, and which operators are judged). `findUninterpretableTemporalComparand` (the `where` entry) keeps its signature and behaviour. New `assertHavingTemporalComparandsInterpretable(object, having, classes, query)`. - `packages/objectql/src/engine.ts`, `ObjectQL.aggregate`: one call, after `assertHavingIsEvaluable` and before `getDriver`, so it covers both `applyHaving` doors (native `driver.aggregate()` and the rows fallback) with zero driver reads on a refusal. - `packages/objectql/src/having-filter.ts`: `temporalKindOf` is exported (one keyword and a doc line), so the door and the storage rule read one kind. - Tests in `objectql` and `rest`, and `.changeset/20263-having-temporal-comparand-door.md` (`minor`, BREAKING narrowing, ADR-0087 `not-required (no-migration-prescription)`). ## How the kind is read (H3) From the class #20127 already derives, `aggregatedRowColumnClasses`, which `engine.aggregate` computes once and now hands to this door too; nothing is derived again. `min` / `max` of a `date` / `datetime` / `time` field keeps that kind, a groupBy projection of such a field takes its kind, a `day` bucket is a `date`. `count` / `count_distinct` / `sum` / `avg`, a `week` / `month` / `quarter` / `year` bucket, and every other column are not judged. An alias that names no column is still refused by #20123's words, first. ## Two deliberate differences from `where`, each measured 1. **Order: last among the `having` doors.** On a clause another `having` door refuses (an unknown operator, a key naming no column, a comparand of no comparable type, an `addDays` pair, an array in the equality slot), that door answers in its own words. So every cell refused at the base keeps its words byte for byte (H4), and only cells that answered 200 move. 2. **The text operators are not judged.** On `where` a text operator on a temporal field never reaches this door: #15661's declared-type door refuses it one step earlier, because "not a date value" is the wrong sentence for an operator no comparand could make runnable. That door does not front `having`, and #15661's ruling keeps the row beneath it answered there (pinned in `engine-text-operator-declared-type-door.test.ts`). Judging them here would revive the retired wording, so `$contains: "2026"` and `$startsWith: "not-a-date"` on `max(placed_on)` answer exactly as before. It judges the caller's own clause before the bigint narrowing, which is what `where`'s object form judges (see Acceptance notes for the bigint consequence). ## Measured: base `89f87f2344` and head, three drivers, both doors, both paths InMemoryDriver, SqlDriver on SQLite, SqlDriver on PostgreSQL 16 (private cluster). Through `engine.aggregate` and `POST /api/v1/data/:object/query`, native path and rows path, `groupBy` customer over four groups. 1650 cells per run; 162 moved, all of them `having` cells that went from 200 with one read to 400 `INVALID_FILTER` with zero reads. Within each row below the three drivers, both doors and both paths agree. | `having` column · comparand | `$gt` base → head | `$lt` base → head | `where` twin | |:--|:--|:--|:--| | `max(date)` · "not-a-date" | no group → 400 | c1–c4 → 400 | 400 | | `max(date)` · "+010000-01-01T00:00:00.000Z" | c1–c4 → 400 | no group → 400 | 400 | | `max(date)` · the number (REST and engine) or `Date` (engine) for 10000-01-01 | c1–c4 → 400 | no group → 400 | 400 | | `max(date)` · "2026-02-01" (control) | c2 → c2 | c1, c3 → c1, c3 | 200 | | `min(datetime)` · "not-a-date" | no group → 400 | c1–c4 → 400 | 400 | | `min(datetime)` · the extended-year ISO, number or `Date` | unchanged | unchanged | not refused by the door (the `datetime` rule reads it; PostgreSQL answers 500, #20264's range) | | `min(datetime)` · 2026 instant (control) | c2–c4 → same | c1 → same | 200 | | `max(time)` · "not-a-date" | no group → 400 | c1–c4 → 400 | 400 | | `max(time)` · extended-year ISO, number, `Date` | unchanged | unchanged | not refused by the door | | `max(time)` · "12:00" (control) | c3, c4 → same | c1 → same | 200 | | `count` / `sum` / `avg` · a string | unchanged | unchanged | not judged (not temporal) | H3 cells: "not-a-date" on a `placed_on` or `opened_at` groupBy key, "noon" on a `slot` key, "not-a-date" and the number for 10000-01-01 on a `day` bucket: 200 → 400 on all three drivers and both doors. A `month` bucket and a text key: unchanged. **H2, paths.** The call sits before `getDriver`, the middleware chain and both `applyHaving` doors. Driver reads of the object on a refusal: 0 on every refused cell (was 1), on both paths, on an empty and a populated object. **H4, collateral.** Byte-identical base → head on all three drivers, both paths, both doors: every `where` and per-aggregation `filter` cell (11 shapes, each at both doors), and 41 `having` shapes: a day, an ISO instant, an epoch-ms number or string, in-range `Date`, zone-naive instant, wall clock, extended-year instant on `datetime`, `{today}` / `{30_days_ago}` / `{not_a_token}`, the empty and whitespace string, `null`, `$exists`, `$in` / `$nin` / `$between`, `$not` / `$or` / `$and`, `{ $field }`, `$contains` / `$startsWith`, a string on `sum`, and the ten existing refusals (#20099 x3, #20123 x3, #20127, #19974, the type door, `$icontains`), in their words. **H5, the words.** Envelope `INVALID_FILTER` / 400, the `where` door's wording class, naming the `having` path, the column, what it aggregates and its kind: ```text aggregate('ledger_20263'): `having` on 'col' (max(placed_on), a date column) compares against "not-a-date" at having.col.$lt, which is not a date value this platform can interpret. Compared with each group as written, it would keep no group or every group, a 200 indistinguishable from a real answer. The `having` was NOT applied. Write a "YYYY-MM-DD" calendar day. ``` The year class (a number or `Date` outside 0..9999 on a `date` column) keeps #20240's sentence. Disclosure: like `where`, the column's declared kind, and otherwise only what the query carries (alias, function, field, comparand, path). The remedy names no `{placeholder}`, unlike `where`'s: `having` resolves none, so naming one would send the author to a literal. ## Tests and evidence (head `a2ff967202`) - `pnpm --filter @objectstack/objectql exec vitest run --project local --project repo --maxWorkers=2`: 321 files, 5828 tests passed, at `438014d26c`, before merging `main` (the merge touched no objectql file; the objectql source is byte-identical at head, and the one objectql test file changed since was re-run at head). - `pnpm --filter @objectstack/rest exec vitest run --project local --project repo --maxWorkers=2`, at `49d218bf7a` (after merging `main`, closure rebuilt): 204 files, 3681 passed, 1 skipped. - `pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2`, at `438014d26c`, with `OS_TEST_POSTGRES_URL` (server `Asia/Shanghai`, process `TZ=America/New_York`): 199 files passed, 3 skipped; 3888 tests passed, 88 skipped. The suite's own summary: `live postgres RAN`, `live mysql NOT RUN` (no MySQL here; CI's Temporal Conformance job runs it). - `pnpm --filter @objectstack/driver-memory exec vitest run --maxWorkers=2`, at `49d218bf7a`: 57 files, 1374 tests passed. - New: `engine-aggregate-having-temporal-door.test.ts` 51 passed; `data-query-having-temporal-door.test.ts` 11 passed (re-run at head). - `pnpm --filter @objectstack/objectql run typecheck` and `pnpm --filter @objectstack/rest run typecheck`: exit 0, test layers included (objectql debt held at 40 files / 234 errors, unchanged; rest 0). - ESLint, narrowed and proven: `eslint --no-inline-config --format json` over the 6 changed `.ts` files, 6 files counted in the JSON, 0 errors, 0 warnings. Population read from `eslint.config.mjs` (`packages/**/*.{ts,tsx,mts,cts}`); invariance: that config enables no type-aware linting (its own note), so this diff cannot move a verdict on an untouched file. **Ablation 1 (the door call), at `438014d26c`, through `scripts/ablation-replace.mjs` with restore proven.** The call in `engine.ts` was replaced by a marker; objectql rebuilt (exit 0); `ablation-dist-preflight` found the marker in 4 built files. objectql test: 20 failed / 31 passed (every refused, one-predicate and H3 case red; every unchanged and earlier-door case green). REST test: 6 failed / 5 passed. Restore leg: blob equals HEAD, rebuilt, marker absent from all 14 built files, tree clean, 51/51 and 11/11 green. **Ablation 2 (the text-operator skip).** `judgesOperator` forced to judge every operator: exactly the two text-operator cells went red (2 failed / 49 passed); restored byte-identical. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at head: 64 commands. All run at `a2ff967202`; `--ran` reconciliation: 64 derived, 62 run, 2 NOT MEASURED, 0 unrun. - NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt`, reason: PREREQUISITE NOT MET (exit 3), both read a whole-workspace `dist/` this worktree does not hold. CI builds it. - Red by design: `node scripts/check-empty-changeset.mjs --base origin/main` exits 1 on the two DELIBERATE CORRECTIONS below. Its own text: say so on the PR and get it confirmed; it stays red either way. - `check:query-options-erasure` went red on the first run (test surface 236 → 238, two `as any` option bags in the new tests); the options are typed now and it holds at 236. - `node scripts/check-issue-citations.mjs --base 2dccb7d`: exit 0, 16 citations resolve. - Also run, as the derivation flagged their rosters under `packages/`: `check:authz-resolver`, `check:error-code-casing`, `check:filter-alias-parity`, all exit 0. ## Deviations - **DELIBERATE CORRECTION, `.changeset/20240-date-year-four-digits.md`**, one clause. It said "`having` does not reach the temporal-comparand door for any comparand, so a number or `Date` outside 0..9999 there is still compared as written." Now: "`having` reaches the same door in the same release (#20263), so a number or `Date` outside 0..9999 is refused there too." - **DELIBERATE CORRECTION, `.changeset/19974-having-comparand-shape-face.md`**, one clause. It said neither the type door nor the declared-type gates "are run on `having`; this change adds the comparand-shape face only". Now it scopes that to "by this change" and adds that the comparand-TYPE door (#20099) and the temporal-comparand door (#20263) reach `having` in the same release. The type-door half was already false since #20099; this corrects both halves in one clause. - A test-file header in `packages/rest/src/data-query-date-year-range.test.ts` said `having` does not reach the door; corrected to point at the new pin. - **"Pin on the three drivers":** the refusal precedes any driver, so it is pinned where CI can boot the engine: both `having` paths with counting drivers (objectql) and the REST door over a real SqlDriver on SQLite (rest). InMemoryDriver and PostgreSQL were measured (the table above), not pinned: no package in the claim's surface has both the engine and those drivers (rest has no `driver-memory` dependency, and no CI job hands the REST suite a PostgreSQL URL). Adding either is outside the claimed file surface. ## Acceptance notes (observations, not filed) - A bigint comparand is judged by neither `having` nor `where`'s object form (the door runs before the type door narrows it); `where`'s array form narrows first and refuses it. In-process only, as JSON carries no bigint. Reported to the seat below. - On PostgreSQL, `where` with a non-numeric string on a `number` field answers 500 `DATABASE_ERROR` where memory and SQLite answer 200. Outside the temporal family; noted, not measured further. ## Out-of-scope findings for the seat (not filed by this PR) 1. `having` resolves no `{placeholder}`: `{ lp: { $gt: "{current_year_start}" } }` on `max(placed_on)` keeps no group (both groups are after 2026-01-01), and `{not_a_token}` answers 200 where `where` answers `FILTER_TOKEN_UNKNOWN` / 400. `resolveWhereTokens` reads `ast.where` only. 2. The per-aggregation `filter`'s temporal refusal names the wrong position: a bad date in `aggregations[1].filter` is reported "at where.placed_on.$gt". 3. On PostgreSQL, SqlDriver's native aggregate returns `count` / `sum` / `avg` as strings (`"n":"1"`, `"total":"20.000000000000000000000000000000"`, also over REST), so `having { n: { $in: [2] } }` keeps no group there while memory and SQLite keep c1, c2, and a string comparand on those columns answers differently on the native and the rows path. 4. The bigint hole in the first Acceptance note, if the seat grades it a defect. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 21ab410 commit a78f731

9 files changed

Lines changed: 776 additions & 15 deletions

‎.changeset/19974-having-comparand-shape-face.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,4 @@ The gate is ONE call in `engine.aggregate`, ahead of both `having` evaluations,
2727

2828
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.
2929

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.
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` by this change, which adds the comparand-shape face only (the comparand-TYPE door, #20099, and the temporal-comparand door, #20263, reach `having` in the same release).

‎.changeset/20240-date-year-four-digits.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,4 @@ What changes:
3535

3636
**Fix.** Compare against a `YYYY-MM-DD` day, or a number or `Date` whose UTC calendar day falls in a four-digit year.
3737

38-
**Unchanged**, measured identical before and after on memory, SQLite and PostgreSQL through the engine and REST: every `datetime` and `time` cell, the same numbers included; every string comparand on a `date` field; every number and `Date` in the years 1000 to 9999; `NaN`, ±Infinity and an Invalid Date, which name no year and are not judged; and every read-path presentation on those three. On MySQL, measured at the driver door, a stored year from 100 to 999 now reads back padded (`0999-06-15`, where it read `999-06-15`); a stored year below 100 still reads back a century late (`0009-03-04` as `1909-03-04`, mysql2's `Date.UTC` reading of a `DATE`), which this change does not touch. `having` does not reach the temporal-comparand door for any comparand, so a number or `Date` outside 0..9999 there is still compared as written. `service-analytics`' raw-SQL decline reads a time dimension by the `datetime` rule, so its answer does not move. `driver-mongodb` keeps its own copy of the `date` rule and is not changed here.
38+
**Unchanged**, measured identical before and after on memory, SQLite and PostgreSQL through the engine and REST: every `datetime` and `time` cell, the same numbers included; every string comparand on a `date` field; every number and `Date` in the years 1000 to 9999; `NaN`, ±Infinity and an Invalid Date, which name no year and are not judged; and every read-path presentation on those three. On MySQL, measured at the driver door, a stored year from 100 to 999 now reads back padded (`0999-06-15`, where it read `999-06-15`); a stored year below 100 still reads back a century late (`0009-03-04` as `1909-03-04`, mysql2's `Date.UTC` reading of a `DATE`), which this change does not touch. `having` reaches the same door in the same release (#20263), so a number or `Date` outside 0..9999 is refused there too. `service-analytics`' raw-SQL decline reads a time dimension by the `datetime` rule, so its answer does not move. `driver-mongodb` keeps its own copy of the `date` rule and is not changed here.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: `having` on `engine.aggregate` takes the temporal-comparand door `where` and the per-aggregation `filter` take, so a comparand its aggregated column cannot read is refused `INVALID_FILTER` / 400 instead of keeping no group or every group (#20263)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) this change adds no transition to migrate. Each refused comparand is one the column's own storage rule cannot read: `having` compared it as written and kept no group or every group, while the same bound in a `where` was already refused by the same door. There is no accepted spelling a refused comparand can be mechanically rewritten to: which day, instant or wall clock the author meant is an authoring decision, and the refusal names what the column takes. `having` is a request-only key that no metadata type stores, so there is no stored document for `objectstack migrate meta` to rewrite. The table below records the answer each comparand had and has; it prescribes no rewrite. -->
10+
11+
**BREAKING**: this narrows what `having` accepts on `engine.aggregate`, and on the REST aggregate query (`POST /data/:object/query`) that forwards it there. A comparand the aggregated column's storage rule cannot read used to answer 200; it is now 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, on an empty and on a populated object. It ships as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
Measured through `engine.aggregate` and `POST /data/:object/query`, on `driver-memory`, `driver-sql` on SQLite and `driver-sql` on PostgreSQL 16, on both paths, with `groupBy` customer over four groups. The three drivers gave the same answer in every cell:
14+
15+
| `having` | before | now | its `where` twin |
16+
|:--|:--|:--|:--|
17+
| `{ last_placed: { $lt: 'not-a-date' } }`, `last_placed` = `max(placed_on)` of a `date` field | 200, every group | 400 | 400 |
18+
| the same under `$gt` | 200, no group | 400 | 400 |
19+
| `'+010000-01-01T00:00:00.000Z'` on the same column, `$gt` | 200, every group | 400 | 400 |
20+
| the number for 10000-01-01 on the same column, `$gt` (and its `Date`, in-process) | 200, every group | 400 | 400 |
21+
| `'not-a-date'` on `min` of a `datetime` field or `max` of a `time` field, `$lt` | 200, every group | 400 | 400 |
22+
| `'not-a-date'` on a groupBy key that is a `date` or `datetime` field or on a `day` bucket, `'noon'` on one that is a `time` field, `$gt` | 200, no group | 400 | 400 |
23+
| the number for 10000-01-01 on a `day` bucket, `$gt` | 200, every group | 400 | 400 |
24+
25+
Each one read the object once before; each is now refused with no read.
26+
27+
What is judged:
28+
29+
- The same walk and the same predicate, `isUninterpretableTemporalComparand` in `@objectstack/core`, that the door runs on `where` and on each per-aggregation `filter`. A change to that rule reaches `having` with it.
30+
- The kind is the aggregated column's class, the one the `addDays` rule already reads: `min` / `max` of a `date`, `datetime` or `time` field keeps that kind, a groupBy projection of such a field takes its kind, and a `day` bucket is a `date`. `count`, `count_distinct`, `sum` and `avg`, a `week` / `month` / `quarter` / `year` bucket, and every other column are not temporal, so they are not judged.
31+
- Every comparison and set operator's comparand, each `$in` / `$nin` member and `$between` endpoint, and the implicit-equality slot, under `$and`, `$or` and `$not`. As on `where`, a `{placeholder}` string, the empty string, `null` and a `{ $field }` reference are not judged. `having` does not resolve placeholders, and did not before, so the refusal's remedy names none.
32+
- The text operators (`$contains`, `$notContains`, `$startsWith`, `$endsWith`, `$icontains`) are not judged. On `where` the text-operator declared-type door answers them first, and that door does not front `having`.
33+
- The door runs after every other `having` door, so a clause one of them refuses (an unknown operator, a key naming no column, a comparand of no comparable type, an `addDays` pair, an array in the equality slot) keeps that refusal and its words.
34+
35+
The refusal follows the `where` door's words. It names the `having` path, the column, what the column aggregates and its kind, and the comparand, for example: `` `having` on 'last_placed' (max(placed_on), a date column) compares against "not-a-date" at having.last_placed.$lt ``. Like the `where` refusal, it names the column's kind, and otherwise only what the query carries.
36+
37+
**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 compares a numeric aggregation alias, which is not judged. Callers of `engine.aggregate` and of the REST aggregate query in a deployment were NOT measured.
38+
39+
**Fix.** Compare a `date` column with a `YYYY-MM-DD` day, a `datetime` column with an ISO-8601 instant, a bare day or epoch milliseconds, and a `time` column with an `HH:MM` or `HH:MM:SS` wall clock.
40+
41+
**Unchanged**, measured identical before and after on the three drivers, both paths and both doors: every `where` and per-aggregation `filter` answer; every `having` on a temporal column whose comparand the rule reads (a `YYYY-MM-DD` day, an ISO instant, an epoch-millisecond number or string, an in-range `Date`, a zone-naive instant, a wall clock, an extended-year instant on a `datetime` column, which that rule reads); `{today}`-style placeholders, known or not; the empty and the whitespace-only string; `null`, `$exists`, `$in` / `$nin`, `$between`, `$not` / `$or` / `$and` and `{ $field }` references; `$contains` and `$startsWith`; every `count` / `sum` / `avg` column, a string comparand included; a `month` bucket; and every existing `having` refusal, in its words.

0 commit comments

Comments
 (0)