Repository navigation
Commit de24d17
fix(objectql, service-analytics)!: a plain group over a formula field is refused alike on every analytics face, instead of a 500, a driver backstop, one null group or a rendered statement (#30768)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DU8jQoF8NLerVQyciDAynrX)_
domain:server#3 · os-elon-musk · session_016WV4cyTehJsfYGVMzcXLJD
Fixes #29304
Clause-②: no (narrowing)
Metadata-protocol: none
## 维护者速读
- 改了什么:按公式字段(不落库的虚拟字段)做普通分组,现在所有分析入口都给同一个 `INVALID_FIELD` /
400,带位置(`groupBy.N`)和"把值存到一个真实字段上再分组"的修法;原生 SQL 策略遇到这种维度会让路给引擎。
- 为什么改:同一个查询原来有四种答案:原生组合 500、ObjectQL 组合是驱动兜底的 400(无位置、建议"跑 schema
sync")、`/analytics/sql` 干跑渲染出 `GROUP BY calc`、草稿预览和内存聚合给一个 `null`
分组,看起来像成功。
- 风险与回滚:只收窄"按公式字段分组"这一种查询;存储字段分组、日期分桶、纯度量查询照旧,且照旧走原生 SQL。回滚即 revert。
- 先看:`packages/objectql/src/aggregate-query-shape-door.ts` 的
`assertGroupByTargetsAreStored`;`native-sql-strategy.ts` 的
`groupsByVirtualField` 让路;`objectql-strategy.ts` 的
`assertCrossObjectAttrsStored`。
- 两分钟验证:`pnpm --filter @objectstack/service-analytics exec vitest run
src/__tests__/objectql-echo-non-date-bucket.test.ts`,预期全部通过,含 5 条
`[#29304]` 用例。
- 需要你做的:无;契约复核后席位落地。
## What changes
**`@objectstack/objectql`: a storage axis at the aggregate door.**
- `assertGroupByTargetsAreStored` (new, in
`aggregate-query-shape-door.ts`, beside the date-bucket and column
doors) refuses a `groupBy` entry of any form naming a field the object
declares VIRTUAL, judged by the spec's `isVirtualSearchField`
(`SEARCH_VIRTUAL_TYPES`, today `formula`), whatever its `returnType`.
`INVALID_FIELD` / 400 with `path` `groupBy.N` for a name,
`groupBy.N.field` for the object form, and `groupBy.N.dateGranularity`
for a bucket.
- The #29167 bucket arm moved here from
`assertDateBucketTargetsAreTemporal`, its words and position unchanged
(the existing #29167 pins pass untouched). The date-bucket door keeps
its class arm only, and `engine.aggregate` calls the storage door first.
- The plain-group sentence is the #29167 message's voice: the position,
the field, its type, "The query was NOT run", the store-the-value
remedy, and what happened before.
**`@objectstack/service-analytics`: the service's one copy, and the
native route.**
- `dataset-date-bucket-door.ts` gains `assertDatasetGroupTargetsStored`,
the engine refusal field for field (the #29167 bucket arm moved into it
too). It is called ahead of `assertDatasetDateBucketsTemporal` at the
three existing call sites: the draft preview, the ObjectQL
`/analytics/sql` echo, and the delegated fallback face.
- `NativeSQLStrategy.canHandle` declines a query whose `dimensions`
entry resolves (through `resolveStorageTarget`, the compiler's own
resolution) to a column the host's `declaredFieldType` names virtual.
The existing decline mechanism (#20887, #21080): what native cannot
serve as the engine would, the engine serves. The query reaches the
ObjectQL strategy and the engine door, and the native composition's dry
run reaches the ObjectQL echo. A host that wired no field types is not
judged.
- A dimension through a relationship over the related object's formula
(`account.name_calc`): the cross-object path hands the related object
`groupBy: ['id', attr]`, which the engine refuses only once a base row
has a key to resolve. `assertCrossObjectAttrsStored` judges those keys
with the service copy in `executeCrossObject` (after the base aggregate,
before any related read) and in the echo, so the query and the dry run
give one answer on any data.
**File surface.** The seat approved one addendum before the PR:
`packages/services/service-analytics/src/strategies/native-sql-strategy.ts`
(the decline). `strategies/objectql-strategy.ts` was NOT in that
addendum; the first version of this body said it was, which was wrong.
Why that file moved (+33 / −1):
- The echo call, `assertDatasetGroupTargetsStored` beside
`assertDatasetDateBucketsTemporal`: `/analytics/sql` on the ObjectQL
composition is answered by this echo, the dry-run call site where `main`
already calls the #29167 service copy. The dispatch surface names the
service's dry-run call sites, but not this file, so it is named here.
- `assertCrossObjectAttrsStored` (one helper, called in the echo and in
`executeCrossObject`) goes beyond the named call sites. While pinning
the faces I found `account.name_calc` (a dimension through a
relationship over the related object's formula) still rendered on the
dry run and served on an empty base, because the engine refuses it only
once a base row has a key to resolve. I added it without asking the seat
for an amendment first. It is pinned by the `account.name_calc` rows in
`objectql-echo-non-date-bucket.test.ts`. The seat ruled that it stays in
this PR: a cross-object dimension over a formula is still a dimension
naming a formula field, so it is this card's defect on another path. The
seat's ACCEPT names `strategies/objectql-strategy.ts` (both sites) in
the surface addendum.
**Fixtures the narrowing moved (fixture triage, not new behaviour):**
- `engine-aggregate-reference-verdict-positions.test.ts`: at `having`, a
formula column can no longer be an aggregated column, so a formula pair
there is refused first by the storage door (`INVALID_FIELD`), as a
list-or-object pair already was. The "addDays pair from a formula
projection" case now asserts that refusal at `groupBy.0`.
- `dimension-structured-json-door.test.ts` and
`multi-value-json-stored-door.test.ts` GUARDs sweep every `FieldType`
through a native-only harness: `formula` now declines out of native and
that harness configures no ObjectQL path, so the guards record it as not
this door's refusal.
**Follow-up card:** #30773. `dataset-compiler.ts` refuses a formula as a
dataset measure but still accepts a dataset or cube dimension over one,
so an author can publish a dimension that every query now refuses. The
authoring-time refusal (Prime Directive 12, `Metadata-protocol:
narrowed`) is that card's, out of this one's scope (contract review
6674767869, should-fix).
## Tests
- `packages/objectql/src/engine-aggregate-query-shape-door.test.ts` (+2
and two control rows): a plain group over `doubled` / `opened_calc` as a
name and as `{ field }` / `{ field, alias }`, full message, `path`,
`param`, `field`, `fields`, `object`, no driver read; the first
offending position wins; the REST door through `findData` and
`mapDataError`. Controls: `status` as a name and `{ field: 'status' }`
served.
- `preview-date-bucket-type.test.ts` (+3): native, ObjectQL and preview
faces give one refusal, field for field, for a formula with `returnType`
`date` and `text`; the later position after a stored dimension; a stored
dimension served alike on all three faces.
- `objectql-echo-non-date-bucket.test.ts` (+5): `/analytics/query` and
`/analytics/sql` on the default (native) composition give one refusal
for an inferred dimension, after a stored one, a cube dimension's `sql`,
and `account.name_calc`. Route pin: a stored dimension, a measure-only
query and a cube dimension stay native (`engine.aggregate` never called,
the native `COALESCE` statement served); a formula dimension declines
and the engine judges it (one `engine.aggregate` call).
- `fallback-delegate-date-bucket-type.test.ts` (+2): the delegated face
refuses a plain formula group as the engine face does, query and dry
run, delegate never called.
-
`packages/qa/dogfood/test/analytics-formula-group-refusal.dogfood.test.ts`
(new, 5): the stock showcase over HTTP,
`showcase_project.budget_remaining` (a currency formula) on
`/analytics/query` and `/analytics/sql`: 400 `INVALID_FIELD`, the
plain-group sentence and remedy, one `error` body on both doors;
`status` served on both.
## Acceptance notes
- Measure-only queries over a formula were already refused before this
change by the measure-type door on both compositions (`calc_sum`: "sum
does not accept that type"), so the route pin uses a stored measure.
- A hand-composed native-only service (`queryCapabilities` with
`objectqlAggregate: false` and no fallback) now answers "No strategy can
handle query" for a formula dimension, where it compiled a `GROUP BY`
the database refuses. No shipped composition reaches it: the plugin
derives both flags from the bridges it wires. The same cost the #20887 /
#21080 declines carry.
- `account.name_calc` is refused at the position of the cross-object
path's own FK-resolve query on the related object
(`aggregate('…account'): groupBy.1`), a position the caller never wrote.
It is the engine's answer for that query, now given on any data and on
the dry run too; the structured-JSON door documents the same trait for
its ObjectQL face.
## Verification
Measured on `origin/main` `447226b2` with a real `SqlDriver` (scratch
test, deleted), a dataset and a cube dimension over `calc` (formula,
`returnType` `date`) and `calc_s` (`returnType` `text`):
| face | before | now |
|:--|:--|:--|
| `/analytics/query`, native composition | `DATABASE_ERROR` / 500 |
`INVALID_FIELD` / 400, `groupBy.0` |
| `/analytics/query`, ObjectQL composition | driver backstop
`INVALID_FIELD` / 400, no `path`, "run schema sync" | the same refusal |
| `/analytics/sql`, both compositions | `SELECT calc AS "calc" … GROUP
BY calc` | the same refusal |
| draft preview | `[{ calc: null, total: 150 }]` | the same refusal |
| `engine.aggregate`, in-memory aggregation | one `null` group |
`INVALID_FIELD` / 400, `groupBy.N` |
Controls at head, measured the same way: a `region` dimension and a
measure-only query are served by the native strategy on both doors, as
on `main`.
<details><summary>Ablations (each restored, `git diff HEAD` empty
after)</summary>
Through `scripts/ablation-replace.mjs` (anchor hit 1 → 0, blob moved),
service-analytics imports `@objectstack/objectql` from `src` (vitest
alias), so no rebuild:
- A1, the `assertGroupByTargetsAreStored` call deleted from
`engine.aggregate`: objectql 3 red (both #29304 pins and the #29167
bucket pin), service-analytics 18 red across the three face files.
- A2, `assertDatasetGroupTargetsStored` made a no-op: service-analytics
18 red (preview, echo and fallback pins, plain and bucket).
- A3, the native decline deleted: service-analytics 10 red (native-face
plain-group pins, the route pin, the fallback engine-face comparison).
</details>
<details><summary>Suites, gates and dogfood at 42c89af</summary>
The head after the contract review, `6bcd5781`, changes only the
changeset (`Clause-②: no (narrowing)`, and the BREAKING heading "Who
moves"). On it: `check-changeset-no-major --base origin/main`,
`check-adr-0087-registration --base origin/main`, `check-empty-changeset
--base origin/main` and `pnpm check:changeset-gate-self-tests` all exit
0; `scripts/pm/clause2-line.mjs` `readClause2Line` reads the changeset
as `value: no, arm: narrowing` and `readMetadataProtocolLine` as `none`.
The full dogfood run below carries over to that head, per the #29167
precedent.
- `@objectstack/objectql` `test`: 556 files / 11071 tests passed;
`typecheck` exit 0. `@objectstack/service-analytics` `test`: 211 files /
5007 passed, 300 skipped; `typecheck` exit 0. `eslint
--no-inline-config` over the 13 changed `.ts` files: exit 0.
- Full dogfood battery (the title carries `!`), `pnpm --filter
@objectstack/dogfood test` under the verify lock: VERDICT command-exit
0, Test Files 422 passed / 3 skipped (425), Tests 3521 passed / 36
skipped (3557). The new pin alone: 5 / 5.
- `dispatch-gates --commands` at `42c89af2`: 68 derived, 67 run with
exit 0, 1 NOT MEASURED (`check:dual-build-cjs-loads`, exit 3: needs a
whole-tree build; CI runs it). `--ran` reconcile: 0 UNRUN.
</details>
Skills-docs: `git grep -i formula -- skills content/docs`, read for
grouping, dimension, bucket and analytics. No page says a formula can be
grouped. `skills/objectstack-data/rules/field-types.md` says a formula
is "virtual: not aggregatable", which still holds; it does not name
grouping. It is a governed surface, so it is not touched here; a skills
seat can add "or grouped by" there.
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes. (Locally: eslint over the
changed files; the repo-wide run is CI's.)
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: its generated artifacts are
regenerated and committed. (Not touched.)
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English.
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
<!-- signed: domain:server#3 · os-elon-musk ·
session_016WV4cyTehJsfYGVMzcXLJD -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent f8195bd commit de24d17
16 files changed
Lines changed: 520 additions & 54 deletions
File tree
- .changeset
- packages
- client/src
- objectql/src
- qa/dogfood/test
- services/service-analytics/src
- __tests__
- strategies
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
534 | 534 | | |
535 | 535 | | |
536 | 536 | | |
537 | | - | |
538 | | - | |
| 537 | + | |
| 538 | + | |
539 | 539 | | |
540 | 540 | | |
541 | 541 | | |
| |||
781 | 781 | | |
782 | 782 | | |
783 | 783 | | |
784 | | - | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
785 | 788 | | |
786 | 789 | | |
787 | 790 | | |
| |||
836 | 839 | | |
837 | 840 | | |
838 | 841 | | |
839 | | - | |
| 842 | + | |
840 | 843 | | |
841 | 844 | | |
842 | 845 | | |
843 | 846 | | |
844 | 847 | | |
845 | 848 | | |
846 | 849 | | |
847 | | - | |
| 850 | + | |
| 851 | + | |
848 | 852 | | |
849 | 853 | | |
850 | 854 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
15 | 19 | | |
16 | 20 | | |
17 | 21 | | |
| |||
48 | 52 | | |
49 | 53 | | |
50 | 54 | | |
51 | | - | |
| 55 | + | |
52 | 56 | | |
53 | 57 | | |
54 | 58 | | |
55 | 59 | | |
56 | 60 | | |
57 | 61 | | |
58 | 62 | | |
59 | | - | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
60 | 66 | | |
61 | 67 | | |
62 | 68 | | |
63 | 69 | | |
64 | 70 | | |
65 | 71 | | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
71 | 83 | | |
72 | 84 | | |
73 | 85 | | |
| |||
127 | 139 | | |
128 | 140 | | |
129 | 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 | + | |
130 | 199 | | |
131 | 200 | | |
132 | | - | |
133 | | - | |
134 | | - | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
135 | 205 | | |
136 | 206 | | |
137 | 207 | | |
| |||
147 | 217 | | |
148 | 218 | | |
149 | 219 | | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | 220 | | |
163 | 221 | | |
164 | 222 | | |
| |||
0 commit comments