Repository navigation
Commit c2cd651
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>1 parent 7ebb543 commit c2cd651
6 files changed
Lines changed: 509 additions & 29 deletions
File tree
- .changeset
- packages
- objectql/src
- rest/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
802 | 802 | | |
803 | 803 | | |
804 | 804 | | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
| 877 | + | |
| 878 | + | |
| 879 | + | |
805 | 880 | | |
806 | 881 | | |
807 | 882 | | |
| |||
Lines changed: 91 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
822 | 822 | | |
823 | 823 | | |
824 | 824 | | |
825 | | - | |
| 825 | + | |
826 | 826 | | |
827 | 827 | | |
828 | 828 | | |
| |||
865 | 865 | | |
866 | 866 | | |
867 | 867 | | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
| 877 | + | |
| 878 | + | |
| 879 | + | |
| 880 | + | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
868 | 958 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17318 | 17318 | | |
17319 | 17319 | | |
17320 | 17320 | | |
| 17321 | + | |
| 17322 | + | |
| 17323 | + | |
| 17324 | + | |
17321 | 17325 | | |
17322 | 17326 | | |
17323 | 17327 | | |
| |||
0 commit comments