Skip to content

Commit 086ad0a

Browse files
fix(service-analytics): native SQL judges a comparand against a declared number column by the spec's verdict, as the comparand walk's second arm (#21446)
Fixes #21426 Clause-②: no (narrowing) ## What this changes `NativeSQLStrategy` compiles its own SQL past the engine's field-aware filter walk, so it skipped the spec's number-comparand verdict (`numberComparandDoorVerdict`, `@objectstack/spec/data`). It now runs that verdict as the **second arm of the one walk** that the boolean arm added (#21376, PR #21424). The arm runs at the same three positions: - the query's `where`, with the dataset door's `runtimeFilter` merged into it; - each measure's own `filter`; - the dataset's own scope. Details: - **One walk, two arms.** `judgedBooleanComparands` / `narrowBooleanComparands` are renamed `judgedComparands` / `narrowComparands`. A walk named for booleans would lie once it also judges numbers. Each arm is a `ComparandArm` row with three parts, and nothing in it is copied from the spec (no table, regex or refusal words): - its spec field verdict (`numberComparandFieldVerdict` / `booleanComparandFieldVerdict`, `judged` alone); - its spec operator lists (`NUMBER_COMPARAND_DOOR_*_OPERATORS` / `BOOLEAN_COMPARAND_DOOR_*_OPERATORS`); - its judge. The two classes are disjoint, so at most one arm judges a member. The number arm is asked first, which is the engine walk's order. - **Refusal envelope.** The arm throws `invalidFilterError` (from the strategy's `filter-normalizer.ts`): `INVALID_FILTER` / 400, the same constructor the boolean arm throws through. The message is `numberComparandRefusalMessage`'s sentence behind the `[analytics]` prefix. Every native position is bound by the driver (a measure filter compiles into its conditional aggregate's bind), so the sentence uses the spec's default, driver-bound reading. - **Narrowing.** A numeric string narrows to the number the verdict names, so the native statement binds what the engine hands its driver: `12`, never `"12"`. It is copy-on-write: a subtree that nothing narrowed is returned by reference. The pins deep-freeze every filter handed in and every registered dataset. - **Member reader.** Unchanged. The declared type comes from the host's `declaredFieldType` hook, for the (object, column) pair that `resolveStorageTarget` returns. So `currency` and `percent` are judged, like every `NUMERIC_VALUE_TYPES` member. A relationship-path member is judged at the related object's declared column. A `formula` reaches the walk with no `returnType` (the plugin relays none), so both verdicts answer `deferred` for it. - **`{ amount: true }`.** The boolean arm never touched it: its field verdict is `not-judged` for a number column. The number arm refuses it as the `boolean` form. No `packages/spec` edit and no `objectql-strategy.ts` edit. `windowClauseSql` is untouched. ## Measured Setup: - **Data:** three rows, with `amount` 5, 12 and 30. - **Composition:** `AnalyticsServicePlugin` over a real `ObjectQL` engine and `SqlDriver`. - **Drivers:** SQLite in memory, and a live PostgreSQL 16.14 server started for this run (since stopped, with its data directory deleted). - **Doors:** `AnalyticsService.query` (what `POST /api/v1/analytics/query` relays) and `AnalyticsService.queryDataset` with a `runtimeFilter` (what `POST /api/v1/analytics/dataset/query` relays). - **Faces:** each door was asked on the native face and on the engine-aggregate face. | `where` | native, SQLite, base `3a6d92f78` | native, PG, base | engine face, SQLite and PG | native, this PR, SQLite and PG | |---|---|---|---|---| | `{ amount: "abc" }` | 200, 0 | 500 `DATABASE_ERROR` | 400 `INVALID_FILTER` | 400 `INVALID_FILTER` | | `{ amount: { $lte: "9999-12-31" } }` | 200, 3 (bound nothing) | 200, 3 | 400 | 400 | | `{ amount: { $ne: "abc" } }` | 200, 3 | 500 | 400 | 400 | | `{ amount: true }` | 200, 0 (bound 1) | 200, 0 | 400 | 400 | | `{ amount: 12 }` (the control) | 200, 1 | 200, 1 | 200, 1 | 200, 1 | | `{ amount: "12" }` | 200, 1, bound `"12"` | 200, 1, bound `"12"` | 200, 1, driver got `12` | 200, 1, bound `12` | - **Dataset door:** the `runtimeFilter` answered identically on every cell. - **Registered datasets:** a dataset's own scope `{ amount: "abc" }` and a measure filter `{ amount: { $ne: "abc" } }` answered 200 on SQLite and 500 on PG on the native face, and 400 on the engine face. Both answer 400 now. - **The card's counts:** the card measured `$lte` and `$ne` at count 2 over different rows. The shape is the same: 200 where the engine refuses. ## Pins: `native-sql-number-comparand-door.test.ts` The file mirrors `native-sql-boolean-comparand-door.test.ts`. Each parity cell asks both faces, at the cube read and at the dataset door. It asserts: - the engine face's answer, and the native face's equality with it; - which strategy answered; - for a refusal, that no statement ran and that the message names the member and `where`. What it covers: - **24 parity cells:** - the card's four cells; - the numeric controls; - narrowed strings on `number`, `currency` and `percent`; - refusals under `$or`, as a list member, a blank string and `"+5"`; - the null tests. - **Spellings:** the FilterArray spelling and the cube-qualified member. - **A narrowing pin:** the native face's bound values equal what the engine handed its driver, for example `[12, 30]` for `$in ["12", 30]`. - **Registered datasets:** one dataset's own scope and measure filter that narrow, and three that are refused. All four are frozen. The PostgreSQL cell runs where `OS_TEST_POSTGRES_URL` is set, and is a named skip otherwise, as in the boolean twin. It ran here against the live server: SQLite plus PG is 114 tests. Two cells are pinned on the native face alone, because the engine face does not reach this verdict there: - **A relationship path** (`account_credit`, the cube dimension over `account.credit`). The engine face refuses every cross-object filter (`INVALID_FIELD` / 400). The native face joins and judges the related object's `number` column: `"abc"` is refused, and `{ $gt: "100" }` binds `100`. - **`{ amount: { $gt: [10] } }`.** The shared analytics lowering hands the engine only the list's first member, so the engine face answers 200, 2. The spec's verdict refuses a list where one number belongs, and the native face now does too. See the acceptance notes. ## Ablations (one-shot and restored; nothing left in the tree) How each leg ran: - **Mutation:** `native-sql-strategy.ts` was mutated through `scripts/ablation-replace.mjs`. Its anchor must hit exactly once. - **Landing proof:** the anchor count and the blob hash. - **Restore proof:** the blob equals HEAD and `git diff HEAD` is empty. Each leg also ran inside a script with an EXIT/INT/TERM restore trap. - **No rebuild needed:** the test imports `../plugin.js` by relative path, so it reads the source, not `dist/`. The predicted direction was named before each run. The observation matched the prediction on every leg, on SQLite and PG: | leg | predicted | observed | |---|---|---| | number arm removed (`comparandArmFor` never returns `NUMBER_ARM`) | 31 per driver: the 11 refusal cells red at both doors, plus FilterArray, the narrowing bind, 3 native-only and 4 registered tests. Boolean twin stays green | 62 failed, 120 passed of 182 (number and boolean files); the boolean file stayed green | | narrowing removed (a `narrows` verdict read as `passes`) | 6: only the bind assertions (the narrowing pin, the relationship-path bind, the registered narrowed dataset). Every count stays green | 6 failed, 108 passed | | measure-filter position's call removed | 6: two registered measure refusals and the registered narrowed bind | 6 failed | | dataset-scope position's call removed | 4: the registered scope refusal and the registered narrowed bind | 4 failed | | `where` position's call removed | 54: 22 parity refusals, FilterArray, the narrowing pin and 3 native-only cells | 54 failed | The narrowing leg's first attempt did not run. `ablation-replace` refused it because the replacement text contained the anchor (count 1 → 1), restored the file, and ran no test. The row above is the re-run with a non-overlapping replacement. ## Gates **Patch round (declaration only), at HEAD `7a626eb09`.** The only change is the changeset; no code moved. - **Derived gates:** `dispatch-gates --commands` derived the same 63 commands as round 1. All 63 ran in a freshly recreated worktree (full `turbo run build`, 72 of 72 tasks from cache, first) and exited 0. `--ran` reconciled 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN. - **`check-adr-0087-registration --base origin/main`:** reads the changeset as `[BREAKING+clause-②-narrowing]` and accepts the disposition `not-required (no-migration-prescription)`. - **`check-changeset-no-major`:** no `major` bump. Its level axis needs a `pull_request` payload, so it was also driven offline with `--event` carrying this body (see the report on the card). - **`check:changeset-gate-self-tests`:** exit 0. **Round 1, at HEAD `a17f21b80`:** - **Derived gates:** `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 63 commands. All 63 ran and exited 0. `--ran` reconciled 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN. - **`pnpm check:dual-build-cjs-loads`:** the first attempt answered `PREREQUISITE NOT MET` (exit 3, no `dist/` for unbuilt packages). It was re-run after a full `turbo run build` (72 of 72 tasks, 71 cached) and exited 0. For `@objectstack/service-analytics`: - **`typecheck`:** exit 0, and `tsc --listFiles` includes both edited files. - **Full `test`:** 170 files and 4065 tests passed, with `OS_TEST_POSTGRES_URL` pointing at the live server. This ran at `d77d545a1`; the later commit adds only the changeset. ## Docs I grepped `content/docs/**` (outside `releases/`) and `skills/**` for the analytics filter's comparand handling. No sentence describes the native face's number comparands, so none became false. ## Acceptance notes - **Changeset:** `@objectstack/service-analytics` `minor`, `Clause-②: no (narrowing)`, a BREAKING banner and an ADR-0087 `not-required (no-migration-prescription)` disposition. This matches the boolean twin's declaration for the same class of change; the seat's review on the card corrected the claim's line to `no (narrowing)`. - **Refusal path wording:** the native refusal of a registered dataset's measure filter roots its path at `where` (for example `where.amount.$ne`), as the boolean arm already did. The engine roots it at `aggregations[i].filter`. This is wording only, and no one is set to carry it. - **Out of scope, measured on SQLite and PG.** These were reported to the seat, not filed here; the seat's disposition follows each: - The shared analytics lowering keeps only the first member of a list at an ordering operator, on both faces. `{ note: { $gt: ["a", "z"] } }` binds `"a"`. On the engine face, `{ amount: { $gt: [10, 99] } }` answers 200, 2 (bound `10`). Filed by the seat as #21448. - The native face's bare-day `$lte` rule ignores the column type. `{ note: { $lte: "9999-12-31" } }` on a `text` column binds nothing and counts every row (3), where the engine face counts 0. The seat's carrier is #21417, which deletes that copy. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 53fd35e commit 086ad0a

3 files changed

Lines changed: 606 additions & 63 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/service-analytics': minor
3+
---
4+
5+
The analytics native-SQL path judges a comparand against a declared number field by the platform's number-comparand rule, the one the data engine's `where` already applies
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a refusal or narrowing of a filter comparand against a declared number column at the analytics native-SQL face alone, the same comparand the engine's where door already judges: no authorable key, spelling, export, type or stored shape moves. Every dataset, cube and analytics query parses and saves as before, the filter's text is untouched, @objectstack/service-analytics exports the same names with the same types, and no stored row is read or rewritten. Which number the author meant by a refused comparand is not something a ledger entry can decide, so there is nothing for objectstack migrate meta to rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a filter comparand's type and this diff adds none (not registered / already-registered); and the change is runtime behaviour with no published interface or type changed (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING**: this narrows what the analytics native-SQL face accepts. A query or dataset that compares a declared number field with a comparand the number-comparand rule refuses used to answer 200 with a count on the native face (a 500 on PostgreSQL for a non-numeric string). It now refuses `INVALID_FILTER` / 400 before any statement runs, which is what the engine-aggregate face already answered. It ships as `minor` under the launch-window convention for accept-set narrowings. No export, type or error code changes.
12+
13+
- **What changed.** A comparand against a `number`, `currency`, `percent`, `rating`, `slider`, `progress` or `summary` column is judged by `numberComparandDoorVerdict` from `@objectstack/spec/data` before the native statement compiles. This covers the query's `where` (including the dataset query's `runtimeFilter`, which is merged into it), each measure's own `filter` and a dataset's own `filter`. The rule runs in the same pass as the boolean rule.
14+
- A numeric string (`'12'`, `'1e3'`) is bound as the number it names, which is what the engine binds.
15+
- Anything else the rule refuses (a string with no numeric reading such as `'abc'`, `''` or `'+5'`, a boolean, or a list where one number belongs) is refused `INVALID_FILTER` / 400 with the rule's own message, before any statement runs.
16+
- A relationship-path member is judged at the related object's declared column.
17+
- **Before.** The native strategy bound the comparand as written. So `{ amount: 'abc' }` counted no rows on SQLite and answered a 500 on PostgreSQL, `{ amount: true }` bound `1` and answered 200, and `{ amount: { $lte: '9999-12-31' } }` counted every row. The engine-aggregate strategy refused all three with 400.
18+
- **What you may notice.** An analytics query or dataset that compared a number field with a value outside the rule's accepted set now refuses instead of answering. Write a number, or a string of exactly that number's JSON spelling (`'12'`).
19+
- **Unchanged.** A number, `null` (the null test), a `{ $field }` reference, a column that is not a number or a boolean, and a host that relays no declared field types (nothing is judged without one).

0 commit comments

Comments
 (0)