Repository navigation
Commit a11faee
fix(objectql)!: a per-aggregation filter refuses a scalar comparison on a declared JSON-stored field, in where's words (#21097)
Fixes #21007
Clause-②: yes (widening)
A per-aggregation `filter` now refuses a scalar comparison on a declared
JSON-stored field (`$eq`, `$ne`, `$gt`, `$gte`, `$lt`, `$lte`,
`$between`, `$in`, `$nin`, implicit equality) with `INVALID_FILTER` /
400, in the words `where` refuses the same filter in. It no longer
counts rows the stored arrays cannot support. The operator set and the
refusal text move from `driver-sql` to `@objectstack/core`, byte for
byte, so both faces read one set and one sentence.
**Clause-② has two halves.** It is `yes (widening)` because
`@objectstack/core`'s root gains three exports
(`JSON_COLUMN_INCOMPATIBLE_OPERATORS`, `jsonColumnOperatorRefusalText`
and its return type `JsonColumnOperatorRefusalText`), and
`applyInMemoryAggregation` gains an optional trailing `reportWithheld`
parameter. It also narrows: `@objectstack/objectql` refuses queries it
used to answer 200, at `engine.aggregate` and at the published
`applyInMemoryAggregation` given a field map. The changeset therefore
carries `minor` for objectql with a BREAKING banner and one ADR-0087
marker, `minor` for core, and `patch` for driver-sql, whose output is
unchanged. The seat answer on the card (`## Seat answer — #21007`,
comment 5924546829) amended the claim to this surface and this
`Clause-②`.
## What was wrong (measured before this change, `d1f8ce865`)
`POST /api/v1/data/:object/query` on SQLite and a live PostgreSQL 16.14,
over the card's six rows (`owners` is a `multiple: true` lookup, and
`d1` and `d3` hold `u1`). Both dialects answered identically:
| filter | `where` twin | per-aggregation `m` before | now |
|:--|:--|:--|:--|
| `owners $in ['u1','u9']` (the card) | 400 `INVALID_FILTER` | 0 | 400,
same body |
| `owners $nin ['u1','u9']` (the card) | 400 | 6, with `d1` and `d3`
counted | 400, same body |
| `owners $eq 'u1'` / `{ owners: 'u1' }` | 400 | 0 | 400 |
| `owners $ne` / `$gt` / `$lte` / `$between` | 400 | 6 / 4 / 1 / 5 | 400
|
| `tags $eq 'red'` | 400 | 1 (`['red']` loosely `==` `'red'`) | 400 |
| `meta` (json) `$eq` / `$in` | 400 | 0 / 0 | 400 |
| `owners $contains 'u1'` (the prescribed spelling) | 2 | 2 | 2
(unchanged) |
| `title $in` / `$nin` / `$eq` (controls) | 2 / 4 / 1 | 2 / 4 / 1 |
unchanged |
## What changed
- **`@objectstack/core`:** a new
`src/utils/json-column-operator-refusal.ts`, exported from the root
beside `temporal-storage-form.js`. The `export *` publishes three names:
`JSON_COLUMN_INCOMPATIBLE_OPERATORS` (driver-sql's 22 spellings, member
for member), `jsonColumnOperatorRefusalText(field, op, bare)`, and its
return type `JsonColumnOperatorRefusalText` (`{ message, diagnostic }`).
These are the two strings `jsonColumnOperatorError` built, and nothing
else. Each face keeps its own error constructor: driver-sql keeps its
#8220 provenance seam, and objectql keeps its ADR-0112 envelope.
- **`driver-sql`** (`sql-driver.ts`): the module-private set and the two
template strings are gone. `jsonColumnOperatorError` keeps its name and
signature, and now calls the core builder (hunk at `:3298`). One import
line (with its comment) sits at `:153`–`:156`, after the top import
block. It is outside the declared `:3376`–`:3460` region on purpose, so
as not to touch the `@objectstack/core` import block that #20988 and
#20987 edit. `assertOperatorAppliesToColumn` (in #20988's former region)
is untouched: it reads the imported set under the same name.
- **`objectql`** (`having-filter.ts`):
`assertAggregationFilterIsEvaluable` gains
`assertAggregationFilterSparesJsonStoredFields`. It runs once on the
filter after the reference rule, against
`declaredJsonStoredFields(declared.fields)`, and before any driver is
asked for a row, so an empty table refuses too (#20122's rule). It walks
`$and` / `$or` / `$not` and refuses implicit equality (reported as `=`,
bare, as driver-sql does) and every operator in the shared set, whatever
the comparand (`null` and `[]` included). The withheld message is
thrown, and the diagnostic, with the aggregation position, goes to
`reportWithheld`, as #20148 does. `$contains` / `$notContains` /
`$exists` / `$null` / `$empty` keep answering. `checkCondition` carries
the same refusal above its no-value exit, but only as a backstop for a
row that reaches the arm. Its reach is the row's: an empty row set never
gets there, and spec lowering rule 3 puts a `$null` arm ahead of every
negation that the walker's `$or` short-circuit takes first. The gate is
the complete door, and the docblocks now say so (round 2, from the
review's ①.5).
- **`objectql`** (`in-memory-aggregation.ts`, round 2, the review's F10
(b)): the published `applyInMemoryAggregation(rows, ast, timezone,
fields, reportWithheld?)` now calls the same
`assertAggregationFilterSparesJsonStoredFields` (exported from
`having-filter.ts`, not from the package root) once per
`aggregations[i].filter` when it is handed `fields`, before any row is
judged. Before this, a direct caller reached only the backstop and got a
row-dependent answer. This entry point holds no logger, so the closest
seam is a new optional trailing `reportWithheld(diagnostic)`, which
receives the field, operator and position; without it the diagnostic is
dropped and the 400 is unchanged. `engine.aggregate` passes none,
because it has already judged and logged the same filter. The gate's
declaration parameter is narrowed to just the `fields` and
`reportWithheld` members of `AggregationFilterDeclaration`, since
`object` is the reference rule's.
- **`objectql`** (`engine.ts`): one comment block and the
`reportWithheld` log line at `assertAggregationFilterIsEvaluable`'s call
site. The log line now reads "as it is for the same refusal in a where"
instead of "…cross-field comparison…", since it carries two refusals
now.
## Measured findings behind the shape (H1–H5)
- **H1, the premise, holds.** `SET_MEMBER_DESCRIPTION`, the `$in` /
`$nin` entries of `FILTER_OPERATORS` and the `$contains` docblock give
no per-element reading. driver-sql's `where` refuses (it does not answer
membership), so triage's "membership, as driver-sql does" misread it.
- **H2, the set.** All ten operators, plus null and empty-list
comparands, were answered with a wrong count; none was already refused.
The bare infix spellings (`in`, `=`, `nin`) are refused earlier at both
positions by the nested-relation door, so the evaluator only meets the
`$` forms.
- **H3, the home.** None existed; per the seat answer, the home is
`@objectstack/core`.
- **H4, where it fires.** The engine's in-memory lowering is the only
evaluator of `aggregations[i].filter`: every driver's aggregate face
refuses a per-aggregation filter 501, and the analytics ObjectQL
strategy hands measure filters to `engine.aggregate`.
- **H5, `having`.** After #21037 (landed, merged here), `min` / `max`
over a multi-valued field is refused `INVALID_FIELD` at the aggregate
door. Measured on InMemoryDriver at the merged head: `max(owners)` with
or without `having` gives 400 `INVALID_FIELD`. So `having` cannot meet a
JSON-stored column, and it is left alone.
## Tests
Round 1 numbers were read at `6e541b101`; round 2 numbers are marked
with the head `f66bed950` (main merged).
- `@objectstack/core` `json-column-operator-refusal.test.ts`: 6 passed.
It pins the set member for member, and the message and three diagnostics
by SHA-256 and length against what driver-sql printed at `8f784959c`.
Hashes avoid a third literal copy of the sentence.
- `driver-sql` `sql-driver-json-column-refusal-shared-text.test.ts`, run
beside the existing JSON-column, compile-refusal-seam and provenance
suites: 248 passed. It checks every `FILTER_OPERATORS` member against
the shared set: the driver's thrown message and withheld diagnostic
equal the core builder's output.
- **Byte identity of the move.** A scratch capture through the built
driver-sql (`SqlDriver` over SQLite) covered all 22 spellings plus bare
equality, unmarked and author-marked, message and diagnostic, 46
entries. Before (`8f784959c`) and after: `cmp` identical, sha256
`dbcf32f5…534b2` on both. driver-sql's `dist` no longer contains the
sentence.
- `objectql` `engine-aggregate-filter-json-column-refusal.test.ts`
(engine-level cell over the `find()` read shape): 74 passed. It covers
18 family cases on each of `owners`, `tags` and `meta` (`code`,
`status`, the `$contains` / `$or` prescription, the field absent from
the message, field and operator in the logged diagnostic, and the driver
never asked for a row), an empty table (pure and grouped), the logged
position, 11 answered cases (membership, null predicates, `title`
controls) and the per-row floor.
- `rest` `aggregation-filter-json-column-refusal.test.ts`: 52 per cell.
SQLite passes and a live PostgreSQL 16.14 passes locally; MySQL is a
named skip. Every family case asserts that the per-aggregation 400
body's `error` is **the same string** as its `where` twin's. #21004's
`aggregation-filter-array-membership.test.ts` still passes beside it.
- **Full suites.** Read at merge `1a226419e`: objectql local 6948
passed, rest local 5072 passed / 247 skipped, core 1809 passed. Read
before the first merge: driver-sql 3285 passed / 188 skipped.
`typecheck` passed for core, driver-sql, objectql and rest. At head
`6e541b101`: core, driver-sql (refusal suites), objectql
`engine-aggregate*` (571 passed) and rest `aggregation-filter*` (150
passed with PostgreSQL) re-ran green.
- **Ablation A: the engine gate call deleted** (`ablation-replace`, plus
a rebuilt objectql `dist`, plus `ablation-dist-preflight --absent`):
- objectql suite: 56 of 74 red. The 54 family cases, the empty table and
the logged position failed; the 11 answered cases and the 7 floor cases
stayed green.
- rest suite: 92 of 104 red (46 per dialect). Populated `owners` /
`tags` cases still got a 400 from the per-row floor, without the logged
diagnostic. `meta` negations (`$ne`, `$nin`, `$nin []`, `$not $in`,
where `meta` is null on every row) answered 200 `{ n: 6, m: 6 }`; the
mechanism was not traced. The empty table answered 200.
- Restored: blob equals HEAD, `git diff HEAD` empty, objectql rebuilt,
preflight shows the marker present in 4 dist files with a clean tree,
and both suites green again (74 and 104).
- **Ablation B: the per-row operator floor replaced by a no-op** (src,
engine suite): 5 red, the 4 operator floor cases and the no-value row;
restored blob equals HEAD.
- **Round 2: the direct-caller pins**
(`engine-aggregate-filter-json-column-refusal.test.ts`, 92 passed). For
`meta` (json) `$ne`, `$nin` and `$not $in`, each on four cells: an empty
row set, an empty grouped row set, `meta` null in every row with the
filter as spec `lowerFilterCondition` lowers it (the shape that carries
the `$null` arm), and the same rows with the filter as written. Each
must refuse 400 `INVALID_FILTER` with exactly `engine.aggregate`'s
message, and hand the diagnostic (field, operator, `At
aggregations[1].filter.…`) to `reportWithheld`. Also pinned: no reporter
means the same refusal; no field map means nothing judged (`m: 0`, as
before); and `$contains` still answers.
- **Ablation C: the new `applyInMemoryAggregation` call deleted**
(`ablation-replace`, anchor 1 to 0, blob `c65412761a90` to
`f530761c0559`): 13 of 92 red.
- The 9 empty, empty-grouped and lowered-null cells, plus the
no-reporter case, answered instead of refusing. That is the backstop's
200.
- The 3 as-written null-row cells were refused by the backstop but with
no diagnostic reported.
- Restored: blob equals HEAD `c65412761a90`, `git diff HEAD` empty, 92
passed again.
- **Round 2 at `f66bed950`:** objectql local full suite 7008 passed (356
files), rest `aggregation-filter*` 150 passed / 61 skipped with a live
PostgreSQL 16.14, core refusal pin 6 passed, driver-sql refusal pins 141
passed.
- **Driver conformance ledger:** 50 covered cells, 0 in the DEBT ledger,
0 exempt, both before and after, in both rounds.
## Gates
- `node scripts/pm/dispatch-gates.mjs --commands` (no paths) derived 70
families at `6e541b101`.
- **68 ran, exit 0.** Among them: `check:adr-0087-registration`,
`check:changeset-no-major`, `check:engine-double-contract`,
`check:nul-bytes`, `check:doc-authoring`, `check:driver-conformance`,
`check:driver-memory-census`, `check:query-options-erasure` and
`check:test-source-alias`.
- **2 NOT MEASURED (exit 3, prerequisite not met):**
`check:dual-build-cjs-loads` and `check:type-check-debt`. Both need the
whole workspace built; two attempts at that build timed out in the
shared verify-lock queue. CI's lint job builds first.
- `--ran` reconciliation: 70 derived, 68 run, 2 NOT MEASURED, 0 unrun.
- **Round 2 at `f66bed950`:** re-derived with no paths, the same 70
families; 68 ran with exit 0 and the same 2 were NOT MEASURED (exit 3).
`--ran`: 70 derived, 68 run, 2 NOT MEASURED, 0 unrun. `typecheck` passed
for core and objectql. Narrowed lint: 10 changed `.ts` files, 0 errors,
0 warnings.
- An earlier run caught one real finding, fixed in `e7bd7f667` ("type
the shared-text pin's find options"): `check:query-options-erasure`'s
test surface grew 236 to 237 because of an `as any` on a `find` options
bag in the new driver-sql test.
- **Lint, narrowed and declared:** `eslint --no-inline-config --format
json` over the 9 changed `.ts` files reports 9 files, 0 errors, 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 a verdict on an untouched file. The full `pnpm lint` is
CI's.
## Acceptance notes
- **Round 2, from the at-tier review (5926186339).**
- F10 (b): `applyInMemoryAggregation` is gated (above).
- F10 (a): the backstop docblocks are corrected.
- F12 and ①.6: the export count is three, and the changeset's "Who is
affected" names `applyInMemoryAggregation` direct callers and the new
optional `reportWithheld`.
- **The "flip the `m: 0` / `m: 6` pins" step had nothing to flip.** No
suite on `main` pinned a per-aggregation `$in` / `$nin` count on a
JSON-stored field (#21004's two suites pin only `$contains` /
`$notContains`). The full objectql, rest and driver-sql runs found no
other pin that this change turns. The refusal pins are new files beside
#21004's.
- **A `{ $field }` comparand on a JSON-stored field** (`{ owners: { $eq:
{ $field: 'title' } } }`) is refused by this gate in the JSON-column
words. driver-sql's `where` refuses it through its cross-field class
rule, in that rule's words. Both answers are `INVALID_FILTER` / 400 with
the field withheld, so the two faces disagree only on which sentence
they print.
- **The REST envelope truncates the shared message at 500 characters**
on both faces, so it ends "…because the answ…". That is unchanged here
by direction, and filed separately by the seat.
- **Findings for the seat, not filed here:**
- driver-memory's `where` answers the family per element on a
multi-valued field, while the SQL family refuses it (engine-level
measurement). The seat files it.
- service-analytics' native measure-filter compiler has no JSON-column
gate (read at source, not measured). Carrier: #20987.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 9939854 commit a11faee
11 files changed
Lines changed: 1124 additions & 94 deletions
File tree
- .changeset
- packages
- core/src
- utils
- drivers/driver-sql/src
- objectql/src
- rest/src
Lines changed: 27 additions & 0 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 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
128 | 135 | | |
129 | 136 | | |
130 | 137 | | |
| |||
Lines changed: 69 additions & 0 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 | + | |
| 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 | + | |
| 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 | + | |
0 commit comments