Skip to content

Commit 97005ae

Browse files
fix(objectql)!: a no-operator object where a scalar field's value belongs is refused INVALID_FILTER / 400 on every driver (#20546) (#20744)
Fixes #20546 Clause-②: no (narrowing) ## What this changes A plain object with no `$`-operator key where a scalar field's value belongs, for example `where: { amount: { a: 1 } }` on a `number` field, is now refused with `INVALID_FILTER` / 400. The refusal names the field, its declared type, the object's keys (never its values) and the path. It runs before any driver is resolved, on every driver, at the three positions the engine judges: `where` (object form and `FilterArray` sugar, on `find` / `findOne` / `count` / `aggregate` / `update` / `delete` and the judge-only `judgeFilter`), `aggregations[i].filter`, and `having`. **Landing site: the number-comparand door's walk, as a second arm. It adds no second traversal.** Triage said: "If the same walk is the natural site, it lands serially after that PR, in the same walk. ⛔ No second traversal of the filter." PR #20545's walk (`walkCondition` in `number-comparand-declared-type-door.ts`) is the only filter walk the engine runs at all three positions with each column's declaration in hand. It already stood on the exact branch: a field spec with no `$` key, which it stepped past (`return kept(spec)`). It now asks one question per field key before the number arm runs: - `packages/objectql/src/no-operator-object-door.ts` (new) holds the arm's classification (`holdsScalarValues`), its structure test (`isNoOperatorObject`) and its words. ⛔ Nothing in it walks a filter. - `number-comparand-declared-type-door.ts`: the walk's per-key resolver now supplies two facts, the number arm's meta and the column's scalar-valued type. The first refusal the walk meets is either arm's. - `having-filter.ts`: `aggregatedRowColumnTypes` reads each aggregated column's type off the query. `aggregatedRowColumnClasses` is now derived from it, so the class and the type are one reading of the query. The `having` arm needs the type because the `text` class lumps a `json` or `lookup` groupBy in with a real text column. - `engine.ts`: the `having` call passes the types; the other hunks are comments. PR #20738's warning-text region is untouched. **Which columns are judged (H3): a closed definition from spec's classes.** `SCALAR_FILTER_HEAD_TYPES` (spec's published "stores one scalar value" set, derived from the ADR-0104 value classes; the #8371 dotted-head verdict reads the same set) with or without `multiple: true`, plus `MULTI_OPTION_TYPES`. The accepted side is never judged: relation types (`lookup`, `master_detail`, `user`, `tree`, single or multiple), structured-JSON types, file and media types (the #8371 carve-out: a legacy stored value is an inline object), `formula` (refused one door earlier, `INVALID_FIELD`), undeclared keys, and unknown types. ## Before, measured on `origin/main` `fbec216e2d` Through `engine.find` / `engine.aggregate` and `POST /api/v1/data/:object/query` (both doors answered alike). Three rows (`amount` 5 / 12 / 30; `owner` u1 / u2 / u1 with u1 in region NA; `meta` `{a:1}` / `{a:2}` / `{b:1}`). InMemoryDriver, SqlDriver on SQLite (better-sqlite3), SqlDriver on a live PostgreSQL 16.13: | position · filter | InMemoryDriver | SQLite | PostgreSQL 16 | |:--|:--|:--|:--| | `where` `{ amount: { a: 1 } }` (number, the card) | 200, no rows | 400 `INVALID_FILTER`, the driver's words ("cannot be bound") | same as SQLite | | `where` `{ title: { a: 1 } }` (text) | 200, no rows | 400, the driver's words | 400, the driver's words | | `where` single select, boolean, date, autonumber, `multiple: true` select, `multiselect`, `tags` | 200, no rows | 400, the driver's words | 400, the driver's words | | `where` `{ $not: { amount: { a: 1 } } }` | 200, **every row** | 400 ("not one this driver evaluates") | same | | `where` `{ $or: [{ amount: { a: 1 } }, { amount: 30 }] }` | 200, 1 row | 400 | 400 | | `where` sugar `[['amount', '=', { a: 1 }]]` | 200, no rows | 400 | 400 | | `where` `{ amount: {} }` | 400, the #5240 words | 400, the #5240 words | same | | `aggregations[1].filter` `{ amount: { a: 1 } }`, `{ title: { a: 1 } }`, `{ amount: {} }` | 200, count 0 | 200, count 0 | 200, count 0 | | `having` `{ total: { a: 1 } }` (a `sum`), `{ title: { a: 1 } }` (a groupBy), `{ total: {} }` | 200, no group | 200, no group | 200, no group | | control: `where` `{ owner: { region: 'NA' } }` (lookup; `master_detail` and a multiple lookup alike) | 200, no rows | 400, the driver's words | same | | control: `where` `{ meta: { a: 1 } }` (json) | 200, 1 row (deep equality) | 400, the driver's words | same | | control: `where` `{ amount: { $gt: { $field: 'cap' } } }` | 200 | 200, 2 rows | 200, 2 rows | ## After, the same run on this branch Every non-control row above answers `400 INVALID_FILTER` in the engine's words on all three drivers, at the path the object sits at (`where.amount`, `where.$not.amount`, `where.$or[0].amount`, `aggregations[1].filter.amount`, `having.total`). No read of the object runs. Every control answers exactly as before: the lookup, master-detail, multiple-lookup and JSON filters reach the driver as written, and so do the file field, the `$field` reference, the undeclared key and the `id` key. Example of the words: ```text find('rp_ledger_20546'): filter on 'amount' puts an object with no operator key (keys "a") at where.amount, where a value of the declared number field 'amount' belongs. An object with no "$" operator is filter structure, not a value: beneath a field it is a nested-relation condition, which only a relation field (lookup, master-detail, user, tree) can carry, or a whole-value match, which only a JSON-bearing field can hold. A number column holds scalar values — one, or a list of them — so no record can match an object there, and an empty answer would read exactly like a real one. The filter was NOT applied. Compare 'amount' with a value ({ "amount": VALUE }) or an operator ({ "amount": { "$eq": VALUE } }). ``` ## Hypotheses (zone 2), which held - **H1: held, with one refinement.** `lowerWhereFilterArray` is the seam, and `narrowNumberComparands` is called there on both branches (the object branch and the lowered array branch). The number door's walk was number-specific only at its per-field gate (`numberComparandFieldVerdict(meta) !== 'judged'`), and its `where` resolver already returned every declared field's type. The text door and the temporal door each walk too, but neither runs at `having` with a column declaration, so neither covers every position. The number door's walk is the one walk that does. The arm rides it, and no traversal was added. - **H2: held, and all three positions are reached.** Measured above: `where` answered per driver, and `aggregations[i].filter` and `having` answered a silent empty on every driver. Each is pinned. - **H3: refined.** The closed definition is above. Multi-value fields were measured on their own: a `multiple: true` select, `multiselect` and `tags` split exactly as a scalar field does (memory 200 no rows, SQL 400). That includes `{ tags: { 0: 'x' } }`, the spelling the #8371 multi-value carve-out exists for: the nested-object form does not reach an array member on InMemoryDriver. So they are judged. A multiple lookup stays on the relation side. A `{ $field }` reference carries a `$` key, so it is never this arm's (measured: served 2 rows on SQL, as before). - **H4: held.** No driver file changes (`git diff fbec216 HEAD -- packages/drivers` is empty). The SQL driver's own `INVALID_FILTER` stays as defence in depth for driver-direct callers and for the columns this arm does not judge (the lookup and JSON controls above still meet it). ## Tests All from `b50627aca9` or from a commit whose non-test source is byte-identical to it (the last two commits touch only the changeset). - `pnpm --filter @objectstack/objectql test`: **338 files / 6704 tests passed**. `test:repo`: 1 file / 5 passed. - `pnpm --filter @objectstack/objectql typecheck`: exit 0 (`check:test-typecheck` OK, the debt ledger held). - `pnpm --filter @objectstack/rest typecheck && pnpm --filter @objectstack/rest test`: **229 files / 4391 passed / 63 skipped** (the live-dialect cells, no URL set). - New pin `packages/objectql/src/engine-no-operator-object-door.test.ts` (17 tests). It uses a recording driver, which is InMemoryDriver's cell by construction because the arm answers before a driver is resolved. It covers every scalar class, `{}`, every verb and the judge, `$and` / `$or` / `$not` paths, sugar, the three REST doors into `findData`, the per-aggregation filter, `having` (sum, groupBy, max of a date, a month bucket), the accepted side at all three positions, a `Map` and the classification GUARD over every `FieldType`. - New pin `packages/rest/src/data-no-operator-object-door.test.ts`: SQLite always, PostgreSQL and MySQL where `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` are set. `where` refusals, the per-aggregation filter, `having`, and the **two controls** (a lookup nested-relation filter and a JSON object comparand: the driver is asked, and the answer is never the arm's). Local run with a live PostgreSQL 16.13: **8 passed (sqlite 4, live postgres 4) / 4 skipped (mysql, no URL)**. ⚠️ As with the sibling door suites, no CI job sets these URLs for `@objectstack/rest`, so the live cells run only locally. - Consumer suites (downstream of `@objectstack/objectql`): `service-analytics` 137 files / 3216 passed; `plugin-security` 147 files / 3202 passed / 23 skipped. The other downstream consumers are declared to CI. **Reverse verification (ablation), from the committed fix.** It ran through `scripts/ablation-replace.mjs` (WRAP mode, trap-restored). The anchor `if (facts.scalarType !== null && isNoOperatorObject(value)) {` was replaced by `if (facts.scalarType === '__ablated_20546__' && …) {`. On disk the anchor went 1 → 0 and the marker 0 → 1, with blob `16151b29f6c1` → `0e8acf882100`. Then objectql was rebuilt and `ablation-dist-preflight` reported the marker present in 4 built files. Predicted direction: red. Observed: red. The objectql pin went **10 failed / 7 passed**: every refusal case failed, and every control and GUARD stayed green. The rest pin went **4 failed / 4 passed / 4 skipped**: the `where` and aggregate refusals failed on SQLite and live PostgreSQL, and the controls stayed green. Restore leg: blob equals HEAD (`16151b29f6c1`), `git diff HEAD` empty, the whole-tree `git status --porcelain` empty, rebuilt, `--absent` preflight (marker absent from all 14 built files), then both pins green again (17 / 17; 8 passed + 4 skipped). ## Gates `node scripts/pm/dispatch-gates.mjs --commands` at `b50627aca9` derived 65 commands. All were run on `b50627aca9`, and `--ran` reconciles them: **65 derived, 63 run, 2 NOT-MEASURED, 0 UNRUN**. 63 exit 0, including `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:engine-double-contract`, `check:where-matcher`, `check:driver-memory-census`, `check:cross-package-test-inputs`, `check:test-source-alias`, `check:type-check-coverage` and `check:query-options-erasure`. - NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt`. Reason: each exits 3 (PREREQUISITE NOT MET) because it reads the built closure of every package, and this box built only the objectql/rest closure. CI's `Lint & Repo Gates` builds that closure. - Driver-related families read before (on `fbec216e2d`, a detached comparison worktree) and after (on `b50627aca9`): - `check:where-matcher`: 440 matchers, 440 correct or loudly refusing, before and after. - `check:driver-memory-census`: 12 bindings / 2 ruled consumers, before and after. - `check:engine-double-contract`: pinned rows 825 → 825 and discovered files 953 → 953. Test files went 4231 → 4233 and production files 2997 → 2998, which are the two new tests and the new module. No new fake engine. - Lint, narrowed and proven: `pnpm exec eslint --no-inline-config --format json` over the 7 changed `.ts` files, at `b50627aca9`, found **7 files, 0 errors, 0 warnings**. The checked population comes from eslint's own config: `calculateConfigForFile` answers `isPathIgnored=false` for all 7. The file count comes from the JSON output (7 results). Untouched files cannot change verdict: `parserOptions.project` and `projectService` are `null` for every file, so type-aware linting is not enabled and this diff cannot move any untouched file's lint result. ## Changeset `.changeset/20546-no-operator-object-on-scalar.md`: `@objectstack/objectql` `minor`, a BREAKING banner, `Clause-②: no (narrowing)` and the ADR-0087 marker `not-required (no-migration-prescription)`, following the #20501 / #20545 precedent. Its "Who is affected" section names a caller that sends the shape to the in-memory driver: a test suite, a local or embedded deployment on `InMemoryDriver`, or a flow or hook calling the engine in-process. No export or published type changes: the door modules are internal, and `@objectstack/objectql`'s root and `./core` exports are unchanged. ## Acceptance notes - **Out of scope, reported to the PM, not filed:** the two controls still answer two ways, because this card's direction keeps them accepted. `{ owner: { region: 'NA' } }` on a `lookup` gives memory 200 with no rows and SQL 400. `{ meta: { a: 1 } }` on a `json` field gives memory 200 with 1 row and SQL 400. So does the undeclared `id` key (`{ id: { a: 1 } }`: memory 200 no rows, SQL 400), because the registry's declared map carries no `id`. Spec's `FilterCondition` declares the nested-relation form, but no data-path driver serves it. #20546 is not the card for that. - File and media fields keep the #8371 carve-out and stay unjudged. `{ photo: { url: 'x' } }` answered memory 200 with no rows (on fresh rows) and SQL 400. A legacy inline value could still match on memory. - At `where`, a `{}` under a judged column is now answered in the engine's words instead of each driver's #5240 words, with the same `INVALID_FILTER` / 400 envelope. Under a column this arm does not judge, `{}` keeps the drivers' refusal. - `findNonNumericComparand` (internal, tests only) still answers the number arm alone. When the walk's first refusal is the new arm's, it answers `null`, and its docblock says so. --- _Generated by [Claude Code](https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 697845d commit 97005ae

8 files changed

Lines changed: 1007 additions & 54 deletions
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
fix(objectql)!: a plain object with no `$` operator where a scalar field's value belongs is refused with `INVALID_FILTER` / 400 at `where`, a per-aggregation `filter` and `having`, on every driver
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 object with no `$` operator key under a column whose declared type holds scalar values. No authorable key, spelling, export or stored shape moves (the engine's door modules are internal; `@objectstack/objectql` exports nothing new and nothing less), and no stored row is read or rewritten. What is refused could match no record on any backend, and which value or operator 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 structure (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 filter may put beneath a scalar field. A plain object with no `$`-operator key — `{ "amount": { "a": 1 } }`, `{}` included — where a value of a field that holds scalar values belongs is refused by the engine before any driver is asked, where the in-memory driver answered it with no records (every record under `$not`) and the SQL driver refused it in its own words. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
12+
13+
The judged fields are the spec's scalar-valued classes: every type in `SCALAR_FILTER_HEAD_TYPES` (text-like, numeric, boolean, date, datetime, time, single-option, `autonumber`, `summary`), with or without `multiple: true`, and the multi-option types (`multiselect`, `checkboxes`, `tags`). A `having` column is judged by the type it carries: a `count` / `sum` / `avg` is a number, a groupBy or `min` / `max` column the type of its field, a date bucket a date or text label.
14+
15+
The refusal names the field, its declared type, the object's keys and the position (`where.amount`, `aggregations[1].filter.amount`, `having.total`). No mechanical rewrite exists, because which value or operator the caller meant is not in the object; the fix is one line by hand: compare the field with a value (`{ "amount": 12 }`) or an operator (`{ "amount": { "$gt": 12 } }`), and to filter by a related record, name a relation field.
16+
17+
Measured through `engine.find` / `engine.aggregate` and `POST /data/:object/query`, three rows:
18+
19+
| position | filter | before: memory · SQLite · PostgreSQL 16 | now, on all three |
20+
|:--|:--|:--|:--|
21+
| `where` | `{ amount: { a: 1 } }` (number), `{ title: { a: 1 } }` (text), and the select, boolean, date, autonumber, multi-select and `multiple: true` select twins | no records · the driver's 400 · the driver's 400 | `INVALID_FILTER` / 400, the engine's words |
22+
| `where` | `{ $not: { amount: { a: 1 } } }` | every record · the driver's 400 · the driver's 400 | `INVALID_FILTER` / 400 |
23+
| per-aggregation `filter` | `{ amount: { a: 1 } }`, `{ amount: {} }` | count 0 on all three | `INVALID_FILTER` / 400 |
24+
| `having` | `{ total: { a: 1 } }` (a `sum`), `{ title: { a: 1 } }` (a groupBy) | no group on all three | `INVALID_FILTER` / 400 |
25+
| `where` | control: `{ owner: { region: "NA" } }` on a `lookup`, `{ meta: { a: 1 } }` on a `json` field | no records / one record · the driver's 400 · the driver's 400 | unchanged: reaches the driver as written |
26+
27+
**Who is affected.** A caller that sends a no-operator object beneath a scalar field to the in-memory driver — a test suite, a local or embedded deployment on `InMemoryDriver`, a flow or hook calling the engine in-process — and read the empty answer as a real one. On `SqlDriver` the same filter was already a 400, now in the engine's words; at the per-aggregation `filter` and `having` it was a silent count of 0 or an empty group set on every driver. No existing test in `@objectstack/objectql` or `@objectstack/rest` sent the shape: both suites pass with no fixture changed.
28+
29+
**Unchanged.** A relation field (`lookup`, `master_detail`, `user`, `tree`, single or multiple) keeps its nested-relation form, and a structured-JSON field (`json`, `composite`, `address`, …) its object comparand; both reach the driver as written, which answers them as before. File and media fields, `formula` (refused one door earlier, `INVALID_FIELD`), undeclared keys, a `{ $field }` reference and every operator bag are not judged by this refusal. A `Map` or a class instance keeps the comparand-type door's refusal in its own words.

0 commit comments

Comments
 (0)