Repository navigation
Commit a75311d
fix(objectql)!: engine aggregate asks the field-type table for every row — min / max / avg over a refused type answer INVALID_FIELD / 400 on every driver (#21037)
Part of #20914
Clause-②: no (narrowing)
#20914 remains open for one row: the census found an authored `sum` over
a type the table refuses, so the `sum` row is held back here and goes
back to triage (section "The census" below). Every other row of the
table is judged by this PR.
## What this changes
`engine.aggregate` now asks `AGGREGATE_FIELD_TYPE_COMPATIBILITY`
(`@objectstack/spec/data`, through `isAggregateCompatibleWithFieldType`)
for EVERY aggregation that names a declared field, not only for
`count_distinct`, with the `isMultiValueField` declaration half beside
it. A pair the table refuses answers `INVALID_FIELD` / `400` before any
driver is resolved. No second table: the door reads the table's rows and
names the row's accepted set in its words, read off the table.
- The door is `count-distinct-json-stored-door.ts` generalized and
renamed `aggregate-field-type-door.ts`
(`assertAggregationFieldTypesAccepted`), at the same call site in
`engine.ts`, right after the `groupBy` door. The old name would have
lied about what it judges.
- `count_distinct`'s refusal words are byte-identical, so its #20808
pins are unchanged (the one clause of its GUARD pin that said "no
verdict for any other function" is flipped, see Pins).
- The declaration half reads the table too: a field declared `multiple:
true` holds the same value as the multi-option types (a list in a JSON
column), so it takes the verdict the row gives that class. Rows refusing
any multi-option type (`count_distinct`, `avg`, `min`, `max`) refuse it;
`count`, which accepts every type, accepts it.
- Fail-closed tiers kept for every function: no field map, an undeclared
name or a relationship path, a type outside `FieldType` (a driver alias
such as `string` / `integer`), a fieldless aggregation or `'*'`, and a
function outside the table's vocabulary all get no verdict. Pinned.
## FROM → TO, measured on three drivers
Through `engine.aggregate` (the door the REST query route, flows, hooks,
roll-up recompute and the analytics ObjectQL strategy reach), two rows,
real `InMemoryDriver`, `SqlDriver` on SQLite and a private PostgreSQL
16.14. Before = `origin/main` `dfe5a0863`; after = this branch's built
`dist` at `9497f4c61`.
| aggregation | before: memory / SQLite / PostgreSQL | after, all three
|
|:--|:--|:--|
| `max` over `json` | `{"a":1}` / `"{\"b\":1}"` (a string) / **500**
`function max(json) does not exist` | 400 `INVALID_FIELD` |
| `min` over `json` | `{"a":1}` / `"{\"a\":1}"` / **500** | 400
`INVALID_FIELD` |
| `max` / `min` over `tags` | an array / a serialized array / **500** |
400 `INVALID_FIELD` |
| `max` over `select` with `multiple: true` | `["a","b"]` / `"[\"a\"]"`
/ **500** | 400 `INVALID_FIELD` |
| `avg` over `datetime` | `null` / `2026` / **500** | 400
`INVALID_FIELD` |
| `avg` over `tags` | `null` / `0` / **500** | 400 `INVALID_FIELD` |
| `max` over `text`, `min` over single `select`, `max` over `email`
(string-class rows, as ruled) | `"y"` / `"y"` / `"y"` | 400
`INVALID_FIELD` |
| controls: `max` `number`, `min` `datetime`, `avg` `percent`, `max`
`boolean`, `sum` `number`, `count_distinct` `text` | one answer each |
unchanged |
| `sum` over `json` / `text` / single `select` (held row) | `0` / `0` /
**500** | unchanged (held) |
The words: `aggregate('OBJ'): aggregations[0].field takes the max of
'meta', a declared json field — a structured-JSON value, which the
engine does not take the max of. The query was NOT run. max accepts a
field of type number, currency, percent, rating, slider, progress,
summary, date, datetime, time, boolean or toggle: aggregate a field of
one of those types, or count the rows with count.` followed by the
reason. The route lands inside the 500 characters the REST door keeps.
The thrown error carries `code`, `status`, `httpStatus`, `field`,
`fields`, `object`, `param`.
## The census (and the held `sum` row)
Query: every line carrying an aggregate-function literal `'min' | 'max'
| 'sum' | 'avg'` (either quote), across `examples/**` and the published
hotcrm stack, each hit resolved by hand to its object and the field's
declared type, then asked of `isAggregateCompatibleWithFieldType`.
- `examples/**` at `dfe5a0863` (crm, todo, multi-package, showcase,
embed-objectql): **21 lines** (dataset measures, roll-up
`summaryOperations`, a `kpi` metric, an `ObjectChart` aggregate, two
cube measures). **0 `min` / `max`.** Every `sum` / `avg` is over
`currency`, `number`, `progress` or `summary`: **0 refused pairs.**
- hotcrm (`objectstack-ai/hotcrm` cloned at `4ca8e2d4bb`, source read,
deps not installed): **41 lines** in `src` / `apps` plus 2 in `test`.
**0 `min` / `max`.** **1 refused pair:**
`src/sales/views/forecast.view.ts:28`, the grouped list view
`all_forecasts` on `crm_forecast` declares `summary: 'sum'` on
`expected_amount`, a `Field.formula`. The table refuses `sum` ×
`formula` (a formula is virtual in SQL storage).
- In-repo shipped sources (`packages/**`, tests excluded): 0 refused
pairs.
- Positive control: the same query finds the hotcrm hit itself, and over
`packages/services/service-analytics/src/__tests__/aggregate-datetime-measure-refusal.test.ts`
it finds that file's authored `avg` over a `datetime`.
Per triage's direction ("A hit stops that row and goes back to triage"),
the `sum` row is held: `ROWS_HELD_FOR_TRIAGE` in the door, named in its
header. Evidence for triage: on the base, `sum` over a `formula` already
answers 400 `INVALID_FIELD` on SQLite and PostgreSQL (the SQL driver has
no column) and `0` in memory; and at objectui `be0ad00` a list view's
column `summary` is a client-side footer, while the server header query
reads `object-grid.aggregations`, so this hotcrm pair does not reach
`engine.aggregate` through the console today. Releasing the row is
deleting that one entry and flipping the `sum` pins.
## Other doors on the engine path (H2)
No other door enforces a row of the table. The `groupBy` door
(`group-by-structured-json-door.ts`) judges a group KEY, not a
function's operand, and stays beside this one; the number-comparand,
temporal-comparand, text-operator and no-operator-object doors judge
filter comparands. So there is one door asking the table, and one
verdict per pair.
## Pins
- `packages/objectql/src/engine-aggregate-field-type-door.test.ts` (new,
recording driver, so the in-memory cell by construction): `max` / `min`
over `json`, `tags`, `address`, a `multiple: true` select; `max` over a
`lookup` with `multiple: true` under `groupBy: ['title']` (the shape a
sibling card measured as PG 500 / SQLite serialized text / memory
array); `avg` over `datetime` and `json`; `max` over `text`, `min` over
`select`; first-offending-position across functions; scalar controls
reach the driver; `sum` held for every type; a GUARD that asks every
judged row × every `FieldType` against the table with per-row floors;
the fail-closed tiers. Each refusal asserts `code` + `status` +
`httpStatus` and the field, declaration, function and position in the
message.
- `packages/rest/src/data-aggregate-field-type-door.test.ts` (new, `POST
/api/v1/data/:object/query` over `SqlDriver`): SQLite always, live
PostgreSQL where `OS_TEST_POSTGRES_URL` is set, MySQL a named skip. No
CI job sets those variables for this package; the local PostgreSQL 16.14
run is below.
- Flipped, not deleted:
`engine-json-stored-group-distinct-door.test.ts`'s GUARD clause "no
verdict for any other function" now asserts `count` and the held `sum`
pass and `avg` / `min` / `max` over `json` are refused in the same
envelope. `engine-nested-object-door.test.ts` reached `having` through a
`max` over a `master_detail` and a `json` field; those two cases now pin
the earlier refusal at this door (`INVALID_FIELD`, no `having.` words,
no read). Its `lookup` group-key case is unchanged.
## Ablation (from committed code)
`node scripts/ablation-replace.mjs` (WRAP, trap-restored) replaced the
held-row check in `aggregate-field-type-door.ts` with one that also
skips every function but `count_distinct` (marker `ablation-20914-A1`):
anchor 1 to 0, blob `67c76fa23d15` to `80100111e881`. `pnpm --filter
@objectstack/objectql build`, then `ablation-dist-preflight.mjs` found
the marker in 4 built files. Predicted red on the new pins only.
Observed: objectql 8 failed / 22 passed (the new suite, the flipped
GUARD clause and the flipped `having` fixture; every `count_distinct`
pin green), REST 4 failed / 10 passed / 7 skipped (the SQLite and
PostgreSQL refusal cells; controls and the #20808 cells green). Restore:
blob equals HEAD, `git diff HEAD` empty, rebuilt, `--absent` found the
marker in 0 of 14 built files and the tree clean, pins green again (30 /
30, 14 passed + 7 skipped).
## Tests (at `9497f4c61`, the merge of `origin/main` `2f2fa11d7`)
- `pnpm --filter @objectstack/objectql exec vitest run --project local`:
349 files, 6830 passed.
- `OS_TEST_POSTGRES_URL=(private PG 16.14) pnpm --filter
@objectstack/rest exec vitest run --project local`: 252 files, 5055
passed, 42 skipped (MySQL cells).
- `pnpm --filter @objectstack/service-analytics exec vitest run`: 150
files, 3458 passed.
- `@objectstack/metadata-protocol` and `@objectstack/plugin-security`
full suites at `e37028132`, before the merge of `origin/main` (not
re-run after it; the merge moved metadata-protocol's flow read path, not
an aggregate path): 194 files / 2886 passed, and 150 files / 3262
passed.
- `typecheck` for objectql and rest: exit 0, test-typecheck ledgers held
(objectql 40 / 234 / 65, rest 0).
- `pnpm --filter @objectstack/spec build` and `check:generated`: all 15
generated artifacts up to date.
- Lint, narrowed and proven: eslint (`allowInlineConfig: false`) over
the 7 changed `.ts` files: 0 ignored, 7 results, 0 errors, 0 warnings;
no file has `parserOptions.project` / `projectService`, so type-aware
linting is off and this diff cannot move a verdict on any untouched
file.
- Driver conformance ledger: 50 covered cells, 0 DEBT, 0 exempt, before
and after.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands` at `9497f4c61`: 88
derived, 88 run, all exit 0; `--ran` with exit codes: 0 NOT-MEASURED.
`check:dual-build-cjs-loads` and `check:type-check-debt` first answered
PREREQUISITE NOT MET (exit 3), then 0 after `turbo run build` over every
package (71 tasks). Also run: `check-changeset-fixed`,
`check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`, `check:error-status-conformance`, all exit
0. `check:adr-0087-registration` accepts `not-required
(already-registered
dataset-measure-selecting-aggregate-field-type-refused,
dataset-measure-aggregate-field-type-refused)`.
## Beyond the claimed file surface
- `packages/spec/src/data/aggregate-field-type-compatibility.ts`: TSDoc
only, three sentences that ship in `dist/data/index.d.ts` and said the
engine door reads only the `count_distinct` row. The table's value is
untouched (the diff has no non-comment line).
- `packages/objectql/src/engine-nested-object-door.test.ts`: the fixture
triage above.
## Acceptance notes
- `no-operator-object-door.ts`'s `having` words for an aggregated column
that "carries a" relation or JSON type were reached only through `min` /
`max` over such a field. The door now refuses those pairs first, so that
branch is unreachable through `engine.aggregate` for a declared field.
Not touched here.
- `.changeset/20783-groupby-structured-json-refused.md` (pending) lists
a structured-JSON field as an aggregated `min` / `max` column as
unchanged; this PR's changeset says it is the later word on that shape,
as the #20808 changeset did for its two shapes.
- `content/docs/data-modeling/queries.mdx` still says `count_distinct`
is not lowered by the SQL drivers; it is (measured `2` on SQLite and
PostgreSQL). Not made false by this change.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0c5a71b commit a75311d
9 files changed
Lines changed: 885 additions & 180 deletions
File tree
- .changeset
- packages
- objectql/src
- rest/src
- spec/src/data
| 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 | + | |
| 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 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
0 commit comments