You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit c2cd651
Browse filesBrowse the repository at this point in the historyBrowse files
fix(objectql)!: having and the per-aggregation filter refuse a plain { $field } across two comparison classes, as where does (#21297)
Fixes#21255
Clause-②: no (narrowing)
This is the precondition #21242 waits on: after it lands, no `having` or
per-aggregation filter query reaches `@objectstack/formula`'s `lteBound`
with a cross-class plain reference. #21242 itself is not addressed here.
## What changed
`having` and the per-aggregation `filter` (`aggregations[i].filter`) now
apply the comparison-class rule to every plain `{ $field }` comparison,
as `where` does, and not only to an `addDays` pair. A plain reference
whose two columns belong to different classes of the spec's
`CROSS_FIELD_COMPARISON_CLASSES` is refused `INVALID_FILTER` / 400
before any driver is asked for a row.
- `packages/objectql/src/having-filter.ts`
- `crossClassReferenceViolation` asks the spec's own
`crossFieldComparisonVerdict` (the classification `driver-sql`'s
`crossFieldComparisonClass` delegates to) and refuses only its
`cross-class` answer. `comparable`, `no-class` and `unjudged` are not
this rule's, and a side with no declaration is not judged.
- The cross-class sentence is written once (`crossClassReason`) and
printed by both the `addDays` rule and the plain rule. No third copy of
the class table: `AggregatedColumnClass` is now the spec's
`CrossFieldComparisonClass`, the same six names.
- Per-aggregation filter: same refusal, same withholding and the same
log line as the #20148 reference rules. The wire message now names the
same-class rule ("compared as the same type class", `where`'s words)
beside the `addDays` one.
- `having`: the pair is judged against each aggregated column's TYPE
(`aggregatedRowColumnTypes`). The message names the two projected
columns and their classes, as the #20127 `addDays` refusal does
(`columnPairError`, which says "with addDays" only when the reference
carries one).
- `addDays` pairs: unchanged, still `offsetPairViolation` over the
having vocabulary's classes.
- `packages/objectql/src/engine.ts`: one argument at the
`assertHavingIsEvaluable` call site,
`aggregatedRowColumnTypes(query.groupBy, query.aggregations,
declaredFields)`, which is the same map the number-comparand door
already receives a few lines below. **This is outside the claim's file
surface.** The reason: the having vocabulary's class
(`classOfDeclaredType`) reads a `file` / `image` groupBy projection as
`text`, where the spec gives it no class. Judging plain references by
that class would have refused a file-against-number pair under the
cross-class words, a narrowing beyond cross-class references. Asking the
spec's verdict of the column's type avoids it.
- Pins: `packages/objectql` (both doors, empty and populated, grouped
and ungrouped) and `packages/rest`, where the `where` twin runs on a
real `SqlDriver`.
- `.changeset/21255-having-plain-reference-class.md`: `minor`, BREAKING
(accept-set narrowing), ADR-0087 `not-required
(no-migration-prescription)`.
## Measurements (H1 to H4)
- **H1, confirmed** at base `2791138cbf`: `assertConditionIsEvaluable`
judged the class only under `if (scope.classes && target.addDays !==
undefined)` (`:1425`). `assertAggregationFilterReferencesAreDeclared`
judged it only under `if (offset !== undefined)` (`:1329`).
- **H2, measured.** `where`'s words are `driver-sql`'s (`sql-driver.ts`
`applyCrossFieldComparison`): `"T" is stored as C1 but "R" as C2, and a
cross-class comparison answers differently in SQL (storage-class
ordering) than in memory (JS coercion) — compare same-class columns.`,
under `uncompilableFieldReferenceError` (`INVALID_FILTER`, withheld).
The having vocabulary and the spec verdict:
- They **agree** on the six class names, and on every single-valued
`FieldType` member the spec classes. The having side reads the same five
value-class sets, and everything else falls back to `text`.
- They **disagree** on the no-class families: `list-or-object`
(structured-JSON types, multi-option types, `multiple: true`) and `file`
read as `text` in the having vocabulary. `formula` is `undefined` (not
judged) there, while the spec says no class (refused by `where`). A type
outside `FieldType` is `text` there and `unjudged` in the spec.
- Hence the plain rule asks the spec's verdict directly and does not
reuse `classOfDeclaredType`. The tail of the sentence is pinned against
the twin's real diagnostic (see Pins).
- **H3, measured.** Each position takes its classes from its own source.
The per-aggregation filter reads the object's declared fields
(`declaredFieldMeta`: `type` plus `multiple`). `having` reads the type
the query gives each aggregated column: a `count` / `sum` / `avg` is a
`number`, a `day` bucket a `date`, a coarser bucket `text`, and `min` /
`max` the field's type. When the class cannot be told, nothing is
judged; that is the posture an `addDays` pair already had. Evidence:
`having` with no registered object keeps `['c1']` for `last_closed $lte
{ $field: 'first_due' }`, and a per-aggregation filter with no
registered object counts 3 for the card's query. Both are pinned on both
doors, and both stayed green under the reverse verification.
- **H4, reproduced** at base `2791138cbf` through `engine.aggregate` on
`SqlDriver` over better-sqlite3 (probe fixture):
`aggregations[0].filter` `{ closed_at: { $lte: { $field: 'due_on' } } }`
answered `[{ n: 4 }]`, and `having` `max(closed_at) $lte { $field:
'due_day' }` (`due_on` day bucket) kept 4 of 6 groups. The `where` twin
answered `INVALID_FILTER` / 400 with the diagnostic above. On the pinned
REST fixture, with the rule reverted: 3 rows (c1 2, c2 1) and 3 of 6
buckets.
## Pins
- **The two measured queries refused, each beside its `where` twin in
the same test**
(`packages/rest/src/aggregation-filter-where-doors.test.ts`, `[#21255]`
block, `POST /api/v1/data/:object/query` on `SqlDriver`). Each refusal
is 400 `INVALID_FILTER` on an empty and a populated table, with no
driver read. The twin is 400 `INVALID_FILTER`. The class names and the
sentence's tail are parsed out of the twin's own withheld diagnostic
(`withheldFilterDiagnosticOf`) and asserted inside the per-aggregation
log line and the `having` message, so the agreement is pinned rather
than retyped.
- **Refusals on both doors** (`packages/objectql`): 7 per-aggregation
shapes (datetime/date both ways, text/number, time/datetime, under
`$ne`, behind a `$or` branch that holds, under `$not`), the names
withheld and logged 8 times. 9 `having` shapes: the card's query,
datetime/date both ways, text/sum, sum/date, count/datetime under `$ne`,
`$or`, `$not`, and a month bucket against a date.
- **Same-class controls answer what they answered.** Per-aggregation:
date/date 3, datetime/datetime 4, number/number 4, time/time 6,
text/text 0 (REST: date/date and datetime/datetime counted as their
`where` twins count). `having`: day bucket/date `['2026-01-02',
'2026-01-15']`, datetime/datetime `['c1', 'c2']`, count/max `['c2',
'c3']`. The existing `ANSWERED` row "a numeric pair with NO addDays"
(`['c1']`) is relabelled "(one class on both sides)", because its old
label "(the rule is the offset's)" is no longer true.
- **`addDays` pairs unchanged:** the existing #20127 `REFUSED` /
`ANSWERED` pins and the #20148 `REFERENCE_REFUSED` / `REFERENCE_PASSING`
pins, left green and not edited.
## Reverse verification
Only the gate was reverted, through `node scripts/ablation-replace.mjs`.
In `crossClassReferenceViolation`, the anchor `if (target === undefined
|| referent === undefined) return undefined;` became `if
('ABLATION_21255'.length > 0) return undefined;`: the plain rule answers
"not judged" at both positions, and the `addDays` rule is untouched.
- The tool showed the anchor 1 → 0 and the blob `c425c56a9b` →
`604a038ce3`. Rebuild exit 0. `node scripts/ablation-dist-preflight.mjs
@objectstack/objectql ABLATION_21255`: marker present in 4 built files.
- Under the mutation:
- objectql: `16 failed | 265 passed (281)`. The 16 are exactly the 7
per-aggregation and 9 `having` refusal pins. Every same-class control,
both registry-less controls and every `addDays` pin stayed green.
- rest: `2 failed | 18 passed (20)`, the two refusal pins. The three
same-class controls stayed green.
- Restore: blob back to the HEAD hash, `git diff HEAD` empty. After the
rebuild, the preflight with `--absent` found the marker absent from all
14 built files and the tree clean. Re-run: objectql `281 passed`, rest
`20 passed`.
- The direction was as predicted: the refusal pins went red, the
controls stayed green.
## Tests and gates (HEAD `df85ac4baa`)
Every command was run through `bash scripts/pm/os-verify-lock.sh`
(`VERDICT command-exit 0` each time) unless it is listed as a gate.
- `pnpm --filter @objectstack/objectql exec vitest run --project local
--maxWorkers=2 src/engine-aggregate-filter.test.ts
src/engine-aggregate-having-comparand-shape.test.ts`: `281 passed
(281)`.
- `pnpm --filter @objectstack/rest exec vitest run --project local
--maxWorkers=2 src/aggregation-filter-where-doors.test.ts`: `20 passed
(20)`.
- The whole `@objectstack/objectql` suite (`vitest run --project local
--maxWorkers=2`): `360 files, 7108 passed`. This ran at `08678c107e`,
which differs from HEAD only by a second `main` merge (no objectql
file), a doc-comment reflow and the REST test's typed options.
- Typechecks, at `df85ac4baa`: `pnpm --filter @objectstack/objectql
typecheck` exit 0, including `check:test-typecheck` ("40 file(s) / 234
error(s) … held"); `pnpm --filter @objectstack/rest typecheck` exit 0,
test layer "0 error(s)".
- Consumers whose fixtures carry `{ $field }` on the aggregate path, run
against the same tree and still green:
- `packages/rest` `data-field-comparand-permission.test.ts`: `30
passed`;
- `packages/services/service-analytics` `cross-field-engine-fallback`,
`cross-field-offset-dataset` and
`include-relation-cross-field-boundary`: `175 passed`.
- Gates: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived 65 commands at
`df85ac4baa`. All 65 ran there with exit 0, `check:dual-build-cjs-loads`
and `check:type-check-debt` included. `--ran` with the recorded exit
codes reports: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN … a DERIVED
zero".
- NOT MEASURED, family `check-issue-citations --census` and
`check-shard-attestation`. Reason: each takes a value from the workflow,
so CI runs them.
- Lint, a proven narrowing: `pnpm exec eslint --no-inline-config
--format json` over the 5 changed source and test files reported 5
results, 0 errors and 0 warnings. `eslint.config.mjs` sets no
`parserOptions.project` and registers no typed rule, so linting is not
type-aware and this diff cannot move the verdict of any file it does not
touch. The repo-wide `pnpm lint` is CI's.
## Acceptance notes
- **No-class pairs are not judged by this rule.** This is a boundary,
kept on purpose: refusing them would narrow beyond cross-class
references. Measured at `engine.aggregate` on `SqlDriver`, where the
`where` twin is 400 every time:
- per-aggregation text vs `image`: counted 6, at HEAD and with the rule
reverted;
- number vs `multiselect`: counted 6, at HEAD and with the rule
reverted;
- `having` on an `image` groupBy against a count: answered, at HEAD and
with the rule reverted;
- `datetime` vs a `formula`, measured at HEAD only: counted 0, because
the formula has no value in the raw rows.
None of these reaches `lteBound` with a bare day. Reported to the seat
as a finding for this family's close-out card.
- `applyInMemoryAggregation` (exported) does not run the #20148
reference rules for a host that calls it directly, and so does not run
this one either. No in-repo caller passes a per-aggregation `{ $field }`
there. Noted, not filed.
- A `having` refusal message runs past the REST envelope's 500-character
bound, so the wire truncates its closing clause about how a column's
class is read. The class reason sits in the first part and survives. The
same is true of the existing `addDays` refusal.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xfMoEjKUuSh2xYB8sCozp)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(objectql)!: `having` and the per-aggregation `filter` refuse a plain `{ $field }` reference between two columns of different comparison classes with `INVALID_FILTER` / 400, as `where` refuses it
6
+
7
+
Clause-②: no (narrowing)
8
+
9
+
<!-- adr-0087: not-required (no-migration-prescription) a refusal of filter STRUCTURE at the engine's query door: a plain { $field } comparand whose two columns belong to different comparison classes, at having and at a per-aggregation filter. No authorable key, spelling, export or stored shape moves: FieldReferenceSchema, every query shape and every object definition parse as before, @objectstack/objectql exports nothing new and nothing less, and no stored row is read or rewritten. What is refused is a comparison the same query's where already refuses on driver-sql, and which same-class column the caller meant is not something a ledger entry can decide. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a filter's comparison class (not registered / already-registered); and the change is runtime behaviour, not a declaration (not runtime-interface-only / type-surface-only). -->
10
+
11
+
**BREAKING**: this narrows what a `{ $field }` reference may pair at two positions of `engine.aggregate`. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
12
+
13
+
**What was accepted before.** At `having` and at a per-aggregation `filter` (`aggregations[i].filter`), the comparison-class rule was applied only to a reference carrying `addDays`. A plain reference across two classes was answered: `{ closed_at: { $lte: { $field: 'due_on' } } }`, with `closed_at` a `datetime` and `due_on` a `date`, counted rows by `@objectstack/formula`'s whole-day reading of the bare day, and a `having` of `max(closed_at)` against a `day` date bucket kept groups the same way. The same comparison in a `where` is refused `INVALID_FILTER` / 400 by `driver-sql`.
14
+
15
+
**What is refused now.** A scalar comparison (`$eq`, `$ne`, `$gt`, `$gte`, `$lt`, `$lte`) whose comparand is a plain `{ $field }` naming a column of a different comparison class. The classes are the spec's `CROSS_FIELD_COMPARISON_CLASSES` (`numeric`, `text`, `boolean`, `date`, `datetime`, `time`), judged by the spec's `crossFieldComparisonVerdict`, the classification `driver-sql`'s `where` compiler reads. The refusal is `INVALID_FILTER` / 400, raised before any driver is asked for a row, on an empty set as on a populated one, through `engine.aggregate` and `POST /api/v1/data/:object/query`:
16
+
17
+
- in a per-aggregation `filter`, the fields, the operator and the reason are withheld from the message and written to the server log, as `where` withholds them; the message now names the same-class rule beside the `addDays` one;
18
+
- in `having`, the message names the two columns of the query's own projection and their classes, in the sentence `where` logs for the same pair. A `having` column's class is read off the query: a `day` date bucket is a `date`, a coarser bucket a `text` label, `count` / `count_distinct` / `sum` / `avg` are `numeric`, and `min` / `max` take the type of the field they read.
19
+
20
+
**The remedy.** Compare same-class columns: a `datetime` with a `datetime`, a `date` with a `date` (a `day` bucket is one), a number with a number. A comparison between a `datetime` and a calendar day has no single answer across SQL and memory, so the platform does not define one.
21
+
22
+
**Unchanged.** A reference between two columns of one class answers as before. A `{ $field, addDays }` pair keeps its judgement and its words. A column whose class the declaration cannot tell is not judged, as an `addDays` pair is not: a host with no registered object, a column the field map does not list (`id`), an aggregation over an undeclared field. A column the spec gives no comparison class at all (a structured-JSON, multi-valued or file field, a formula) is not judged by this rule either.
['a datetime against a date (the card\'s measured query)',()=>({closed_at: {$lte: {$field: 'due_on'}}}),'"closed_at" is datetime but "due_on" is date'],
822
+
['a date against a datetime',()=>({due_on: {$gte: {$field: 'closed_at'}}}),'"due_on" is date but "closed_at" is datetime'],
823
+
['a text against a number',()=>({customer_id: {$gt: {$field: 'amount'}}}),'"customer_id" is text but "amount" is numeric'],
824
+
['a time against a datetime',()=>({slot: {$lt: {$field: 'opened_at'}}}),'"slot" is time but "opened_at" is datetime'],
825
+
['a datetime against a date under $ne',()=>({closed_at: {$ne: {$field: 'due_on'}}}),'"closed_at" is datetime but "due_on" is date'],
826
+
['a datetime against a date behind a $or branch that holds',()=>({$or: [{amount: {$gt: 0}},{closed_at: {$lte: {$field: 'due_on'}}}]}),'"closed_at" is datetime but "due_on" is date'],
827
+
['a datetime against a date under $not',()=>({$not: {closed_at: {$lte: {$field: 'due_on'}}}}),'"closed_at" is datetime but "due_on" is date'],
828
+
];
829
+
830
+
for(const[name,filter,diagnostic]ofCROSS_CLASS){
831
+
it(`${name}: refused before any read, whatever the rows, the names withheld and logged`,async()=>{
['a datetime against a date',()=>({last_closed: {$lte: {$field: 'first_due'}}}),'having.last_closed.$lte','"last_closed" is datetime but "first_due" is date'],
899
+
['a date against a datetime',()=>({first_due: {$gte: {$field: 'last_closed'}}}),'having.first_due.$gte','"first_due" is date but "last_closed" is datetime'],
900
+
['a groupBy text column against a sum',()=>({customer_id: {$gt: {$field: 'total'}}}),'having.customer_id.$gt','"customer_id" is text but "total" is numeric'],
901
+
['a sum against a date',()=>({total: {$gt: {$field: 'last_placed'}}}),'having.total.$gt','"total" is numeric but "last_placed" is date'],
902
+
['a count against a datetime under $ne',()=>({order_count: {$ne: {$field: 'first_opened'}}}),'having.order_count.$ne','"order_count" is numeric but "first_opened" is datetime'],
903
+
['a datetime against a date behind a $or branch that holds',()=>({$or: [{total: {$gt: 0}},{last_closed: {$lte: {$field: 'first_due'}}}]}),'having.$or[1].last_closed.$lte','"last_closed" is datetime but "first_due" is date'],
904
+
['a datetime against a date under $not',()=>({$not: {last_closed: {$lte: {$field: 'first_due'}}}}),'having.$not.last_closed.$lte','"last_closed" is datetime but "first_due" is date'],
905
+
];
906
+
907
+
for(const[name,having,path,fragment]ofREFUSED){
908
+
it(`${name}: refused at ${path}, whatever the rows`,async()=>{
0 commit comments