Repository navigation
Commit 157baa7
Fixes #20783
Clause-②: no (narrowing)
## What this changes
A `groupBy` entry that names a declared **structured-JSON** field
(`json`, `composite`, `repeater`, `record`, `location`, `address`,
`vector`) is now refused by `engine.aggregate` with `INVALID_FIELD` /
400, in the engine's words, before any driver is asked. It holds on
every driver and for every caller that reaches the engine: the REST
query door, a flow or hook calling the engine in-process, and the
analytics strategy that lowers a cube query onto `engine.aggregate`.
Both entry spellings are judged: the field name (`groupBy[0]`) and the
`{ field }` object (`groupBy[0].field`), a `dateGranularity` bucket
included.
The words, as `POST /api/v1/data/:object/query` returns them (the route
is inside the 500 characters the REST door keeps, and the REST pin
asserts it there):
```text
aggregate('rest_group_by_json_20783'): groupBy[0] names 'meta', a declared json field — a structured-JSON value, which the engine does not group by. The query was NOT run. Group by a field that stores one scalar value: store the part you group on in a field of its own and group by that field. A JSON document is no group key the drivers share: one merged every row into a single group, one grouped each serialized document apart, one refused the statement.
```
The thrown error carries `code: 'INVALID_FIELD'`, `status` and
`httpStatus` 400, `field`, `fields` (every offending entry), `object`
and `param: 'groupBy'`.
**Landing site, as the order expected: `packages/objectql`'s aggregate
admission. No driver file, no serialization rule.**
- `packages/objectql/src/group-by-structured-json-door.ts` (new):
`assertGroupByNamesNoStructuredJsonField(object, schema, groupBy)`. The
class is the spec's `STRUCTURED_JSON_TYPES` (the set PR #20781's JSON
arm judges), never a list minted here. Not judged: an undeclared name
(the engine's registry-less tolerance; the REST ingress answers an
unknown name `INVALID_FIELD` first), a host with no field map, and every
other type.
- `packages/objectql/src/engine.ts`: one call at the entry of
`aggregate`, right after `rejectCredentialAggregation` (which reads the
same `groupBy` entries), so a protected field keeps that refusal's
words. `aggregate` is the only engine verb that takes `groupBy`: `find`
refuses the key (`ENGINE_FIND_OPTION_KEYS`).
- `packages/objectql/src/number-comparand-declared-type-door.ts`:
comment only. Its sentence "a `json` … groupBy … is judged now" at
`having` became false for a json groupBy, which no longer reaches
`having`.
**Why `INVALID_FIELD`, an existing code.** The verdict is about the
named field's type at a position. That is the question the REST ingress
answers with `INVALID_FIELD` for an unknown `groupBy` name
(`assertGroupByFieldsExist`), the search axis answers with
`INVALID_FIELD` for a field whose type it cannot scan, and the engine's
`assertFilterIsMaterializable` answers with `INVALID_FIELD` for a
virtual field ("this verdict is about the NAME's type").
`INVALID_FILTER` is the engine's value-shape envelope and `groupBy` is
not a filter; `INVALID_QUERY` is the ingress's malformed-shape code, and
the entry here is well formed.
## Before, measured on `origin/main` `7a09eee1b1`
Through `POST /api/v1/data/:object/query` (the real `RestServer` route
over `ObjectStackProtocolImplementation` and `ObjectQL`) with `{
groupBy: [FIELD], aggregations: [{ function: 'count', alias: 'n' }] }`,
and through `engine.aggregate` directly (same answers). Drivers:
InMemoryDriver, SqlDriver on SQLite (better-sqlite3), and SqlDriver on a
private PostgreSQL 16.13 started for this run. Three rows: `title` x, x,
y; `meta` `{a:1}`, `{a:2}`, `{b:1}`; and one differing value per row
under every other structured-JSON field.
| `groupBy` | InMemoryDriver | SQLite | PostgreSQL 16 |
|:--|:--|:--|:--|
| `title` (text, the control) | 200, `x` 2 · `y` 1 | same | same |
| `meta` (json, the card) | 200, **one group** `{a:1}`, `n` 3 | 200, one
group per serialized document (3) | **500 `DATABASE_ERROR`** ("could not
identify an equality operator for type json") |
| `composite`, `repeater`, `record`, `location`, `address` | 200, one
group, `n` 3 | 200, one group per serialized document | 500 |
| `vector` | 200, one group per array (`[1,2]` 2 · `[3,4]` 1) | 200, one
group per serialized array | 500 |
| `{ field: 'meta' }` | 200, one group, `n` 3 | 200, 3 groups | 500 |
| `{ field: 'meta', dateGranularity: 'month' }` | 200, one `null`
bucket, `n` 3 | 200, one `null` bucket, `n` 3 | 500 ("cannot cast type
json to timestamp with time zone") |
| `['title', 'meta']` | 200, 2 groups | 200, 3 groups | 500 |
## After, the same run on this branch (`43ae6c1fcc`)
Every structured-JSON row above answers `400 INVALID_FIELD` in the
engine's words on all three drivers, naming the position (`groupBy[0]`,
`groupBy[0].field`, `groupBy[1]` for the mixed entry), the field and its
declared type. No read of the object runs. The `title` control answers
`x` 2 · `y` 1 on all three, unchanged. The InMemoryDriver cells of this
run came from a scratch script over the built packages. They are not
committed: `check:driver-memory-census` refuses a new test consumer of
that driver without a ruling, so the committed memory cell is the
recording driver below, by construction.
## Hypotheses (zone 2): which held
- **H1: held.** `aggregate` resolves `groupBy` entries against the
declared field map before the driver in `rejectCredentialAggregation`
(credential and `internal` fields) and, for `having`, in
`aggregatedRowColumnTypes`. The refusal sits beside the first, at the
verb's entry, so it runs before the per-aggregation `filter` and
`having` doors and before any driver is resolved. Code: `INVALID_FIELD`,
reasons above. The REST ingress also resolves `groupBy` names
(`assertGroupByFieldsExist` in `metadata-protocol`), but only for the
REST path. The engine is the one door every caller shares, as triage
directed.
- **H2: held, refined.** Every member of `STRUCTURED_JSON_TYPES` answers
per driver today, and none answers one way. Six members split exactly
like `json` (memory one merged group, SQLite per serialized document,
PostgreSQL 500). `vector` splits differently on memory (one group per
array, not one merged group) and still 500 on PostgreSQL, so it does not
answer one way either. So the class is refused, not `json` alone.
- **H3: held, and it was NOT already refused.** A date-bucketed `{
field, dateGranularity }` over a `json` field passed the REST ingress (a
known field, a valid granularity) and answered one `null` bucket on
memory and SQLite and 500 on PostgreSQL. It is refused now with the `{
field }` form's words, since no granularity makes a JSON document a
date.
- **H4: measured. The analytics face is partly the same door.** Measured
through `AnalyticsService.query` wired with `AnalyticsServicePlugin`'s
own auto-bridges (`executeAggregate` to `engine.aggregate`,
`executeRawSql` to `engine.execute`), with a cube dimension `sql:
'meta'` on a `json` field:
| cube query | InMemoryDriver | SQLite | PostgreSQL 16 |
|:--|:--|:--|:--|
| `dimensions: [meta]`, before | 200, one merged group (raw SQL
unsupported, fell back to `engine.aggregate`) | 200, one group per
serialized document (**native SQL**, engine not reached) | 500
`DATABASE_ERROR` (**native SQL**) |
| `dimensions: [meta]`, after | **400 `INVALID_FIELD`** (this door) |
unchanged | unchanged |
| `timeDimensions: [{ meta, granularity: month }]`, before | 200, one
`null` bucket | 200, one `null` bucket | 500 |
| the same, after | **400 `INVALID_FIELD`** (the native strategy
declines a granularity, so `engine.aggregate` serves it) | **400** |
**400** |
So the ObjectQL strategy (any cube query on a driver without raw SQL,
and any bucketed time dimension) reaches this door and is folded in with
no `packages/services` edit. **`NativeSQLStrategy` does not**: on SQL
drivers it compiles `GROUP BY "meta"` by hand and bypasses the engine.
That half is a sibling card (reported to the PM below, not filed here).
The dataset compiler only checks that a dimension's field is declared
(`assertDeclared`), so a dataset dimension on a `json` field compiles
into the same cube dimension and splits the same way. The memory cube
face (`MemoryAnalyticsService`) has **zero** production constructors
(`git grep "new MemoryAnalyticsService"` outside tests: 0 hits at
`7a09eee1b1`), so it reaches a driver only in tests. It was not edited
and not measured for this shape.
## Producers measured (for "refuse, don't define")
Zero producers group by a structured-JSON field. Every `grouping` /
`groupBy` / `groupByField` / `dimensions` target in `examples/` at
`7a09eee1b1` is one of `status`, `priority`, `created_at`, `account`,
`stage`, `total`, `region`, `sales_region`, `progress`, `issued_on`,
`industry`, `category`, `signed_on`, `health`, `completed_date`,
`close_date`, and none is structured-JSON (the showcase's
structured-JSON fields are `field-zoo`'s `f_*`, `account.hq`,
`account.support_config` and `task.location`). The same count over the
platform packages names `object_name`, `user_id`, `id`, `action`,
`topic`, `provider_id`, `organization_id`, `namespace`, `kind`,
`actor_id` and `phone`, plus two dynamic engine callers in
`service-analytics` (see the acceptance notes).
## Tests
- New `packages/objectql/src/engine-group-by-json-door.test.ts` (6
tests, recording driver, so the in-memory cell by construction). It
covers every structured-JSON type with the full envelope (`code`,
`status`, `httpStatus`, `field`, `fields`, `object`, `param`) and the
position, and asserts zero reads. It covers the `{ field }` form, a date
bucket, an entry after a scalar one and two offenders, and the REST door
into `findData`. Controls: `text`, `number`, a `multiple: true` select,
an image field and an undeclared name all reach the driver, and a
structured-JSON field as an aggregated column is unchanged. GUARDs: the
judged types equal `STRUCTURED_JSON_TYPES` over every `FieldType`, and
there is no verdict without a field map, for an undeclared name, or for
an entry naming no field.
- New `packages/rest/src/data-group-by-json-door.test.ts`: SQLite
always, PostgreSQL / MySQL where `OS_TEST_POSTGRES_URL` /
`OS_TEST_MYSQL_URL` are set. Every structured-JSON type answers 400
`INVALID_FIELD` with the route in the REST body and zero reads. The
object and bucket forms and the mixed entry answer the same 400. The
control `title` answers `x` 2 · `y` 1 from the driver. ⚠️ As with the
sibling door suites, no CI job sets those URLs for `@objectstack/rest`,
so the live cells run only locally. Local run with the private
PostgreSQL 16.13: **6 passed (sqlite 3, live postgres 3) / 3 skipped
(mysql, no URL)**.
- Fixture triage in `engine-nested-object-door.test.ts` (PR #20781's
pin): its `having` case over `groupBy: ['meta']` pinned the branch this
PR closes, because a json groupBy never reaches `having` now. It is
**replaced**, not respelled. The JSON arm at `having` is still reached
through a `max` of a json field (`having: { top_meta: { a: 1 } }`),
which answers the same `INVALID_FILTER` whole-value words. No other
fixture groups by a structured-JSON field. Measured by a grep over every
test that both groups and declares a structured-JSON type; the
downstream suites below stayed green.
- `pnpm --filter @objectstack/objectql test` at `43ae6c1fcc`: **342
files / 6739 passed**.
- `pnpm --filter @objectstack/rest test` at `43ae6c1fcc` (with the live
PostgreSQL URL set): **232 files / 4516 passed / 33 skipped**.
- `typecheck` for `@objectstack/objectql` and `@objectstack/rest` at
`43ae6c1fcc`: exit 0, including each `check:test-typecheck` (objectql's
ledger held at 40 files / 234 errors / 65 signatures; rest 0), so both
new test files compile.
- Downstream consumers (`...@objectstack/objectql` direction), at
`cafaf885d8`: `@objectstack/service-analytics` **141 files / 3267
passed**; `@objectstack/metadata-protocol` **190 files passed, 3 skipped
/ 2792 passed, 19 skipped**. The other consumers are declared to CI.
**Reverse verification (ablation), from the committed fix at
`1eacad5296`.** It ran through `scripts/ablation-replace.mjs` in WRAP
mode, trap-restored. The engine's call
`assertGroupByNamesNoStructuredJsonField(object,
this._registry.getObject(object), query.groupBy);` was fed `(query as {
__ablated_20783__?: unknown }).__ablated_20783__` (always undefined)
instead of `query.groupBy`. On disk the anchor went 1 to 0 and the
marker 0 to 1, with blob `67198fc4a8fc` to `837fc9257e37`. objectql was
rebuilt, and `ablation-dist-preflight` found the marker in 4 built
files.
- Predicted direction: red. Observed: red.
- objectql pins: **3 failed / 18 passed**. Every refusal case failed.
The controls, both GUARDs and the whole
`engine-nested-object-door.test.ts` stayed green.
- rest pins: **4 failed / 2 passed / 3 skipped**. The refusal cases
failed on SQLite and live PostgreSQL, and the `title` control stayed
green.
- Restore leg: blob equals HEAD (`67198fc4a8fc`), `git diff HEAD` is
empty, and whole-tree `git status --porcelain` is empty. After a
rebuild, the `--absent` preflight found the marker absent from all 14
built files, and the tree reading was clean. Then both pin sets were
green again (21 passed; 6 passed + 3 skipped).
## Gates
`node scripts/pm/dispatch-gates.mjs --commands` at `43ae6c1fcc` (the
merge of `origin/main` `96e724475c`, which touches none of this diff's
packages) derived **65** commands over the 7 changed paths. All 65 were
run on `43ae6c1fcc`, and `--ran` reconciles them: **65 derived, 65 run,
0 NOT-MEASURED, 0 UNRUN**, all exit 0. Among them:
- `check:adr-0087-registration --base origin/main`: `not-required
(no-migration-prescription)` accepted.
- `check:changeset-no-major`, `check:empty-changeset`,
`check:doc-authoring`, `check:nul-bytes`, `check:issue-citations`.
- `check:engine-double-contract`, `check:where-matcher`,
`check:driver-memory-census`, `check:cross-package-test-inputs`,
`check:test-source-alias`, `check:type-check-coverage`,
`check:type-check-debt`.
- `check:query-options-erasure`. It caught a first draft of the REST pin
that erased an `engine.aggregate` options bag to `any` (test surface 236
to 237). Fixed by typing it (`cafaf885d8`), and the surface is back at
236.
- `check:dual-build-cjs-loads`, after a full `turbo run build` of
`./packages/*` and `./packages/*/*`.
Lint, narrowed and proven: `pnpm exec eslint --no-inline-config --format
json` over the 6 changed `.ts` files at `43ae6c1fcc` found **6 files, 0
errors, 0 warnings**. Three facts make this narrowing a measurement:
- The checked population comes from eslint's own config: `isPathIgnored`
answers `false` for all 6.
- The file count comes from the JSON output: 6 results.
- Untouched files cannot change verdict: `parserOptions.project` and
`projectService` are `null` for every file, so type-aware linting is not
enabled.
## Changeset
`.changeset/20783-groupby-structured-json-refused.md`:
`@objectstack/objectql` `minor`, a BREAKING banner, `Clause-②: no
(narrowing)`, and exactly one ADR-0087 marker, `not-required
(no-migration-prescription)`, in the form PR #20781's changeset uses. It
carries no rewrite table and no arrow: the body states what an author
sees now, why, who is affected and what is unchanged. No export or
published type changes: the door module is internal, and
`@objectstack/objectql`'s root and `./core` exports are unchanged.
## Acceptance notes
- **Reported to the PM, not filed here (same family, measured):**
- **The native-SQL analytics face** (triage's H4 sibling). A cube or
dataset dimension on a structured-JSON field through `NativeSQLStrategy`
answers one group per serialized document on SQLite and 500 on
PostgreSQL. It never reaches this door. The fix lands in
`packages/services/service-analytics`, which this claim does not touch.
- **`groupBy` on a `multiple: true` select** through `POST
/api/v1/data/:object/query`: memory answers one group per array, SQLite
one per serialized array, and PostgreSQL 500 (json equality). Not this
card's class, and a multi-value field has a plausible other meaning (a
bucket per member), so it is not refused here.
- **`count_distinct` over a `json` field** through the same door: memory
3, SQLite 3, PostgreSQL 500. `AGGREGATE_FIELD_TYPE_COMPATIBILITY`
accepts `count_distinct` for every `FieldType` on the ground that every
backend gives one answer. PostgreSQL does not for json.
- `service-analytics` has two dynamic `engine.aggregate` callers:
`resolveFkAttr` (`groupBy: ['id', attr]`, a cross-object dimension
attribute) and the display-label pass (`groupBy: ['id', displayField]`).
If that attribute or display field is structured-JSON, they now answer
this 400. Before, by reading and not measured: memory and SQLite grouped
under `id` first, so one row per record, and PostgreSQL would have
answered the same json-equality 500. No example or platform dataset
names such a dimension (the census above).
- A refusal relayed through the analytics engine path names the engine's
position (`groupBy[0]`), not the cube member the caller wrote
(`c20783.meta`). Carrier: the native-SQL sibling card, which touches the
same face.
- The native-SQL analytics path returned PostgreSQL's `count` as a
string (`"2"`) where SQLite returned a number, in the stand-in wiring
above. Observed only; not measured through the HTTP door.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4cc5bcd commit 157baa7
7 files changed
Lines changed: 574 additions & 2 deletions
| 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 | + | |
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
294 | 294 | | |
295 | 295 | | |
296 | 296 | | |
297 | | - | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
298 | 300 | | |
299 | 301 | | |
300 | 302 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
200 | 200 | | |
201 | 201 | | |
202 | 202 | | |
| 203 | + | |
203 | 204 | | |
204 | 205 | | |
205 | 206 | | |
| |||
16365 | 16366 | | |
16366 | 16367 | | |
16367 | 16368 | | |
| 16369 | + | |
| 16370 | + | |
| 16371 | + | |
| 16372 | + | |
| 16373 | + | |
| 16374 | + | |
| 16375 | + | |
| 16376 | + | |
16368 | 16377 | | |
16369 | 16378 | | |
16370 | 16379 | | |
| |||
0 commit comments