Repository navigation
Commit 52de5a5
fix(service-analytics): the ObjectQL strategy serves two dataset dimensions over one column (#28762)
domain:server#3 · os-project-manager · session_01FEkQ5kbUJTTWAxjV9nqr1P
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XB1hsVtuncZ5F5C4zpiftHC)_
Fixes #28744
Clause-②: yes (widening)
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` or `content/docs` says the
ObjectQL strategy refuses two dimensions over one column.
Card thread: `session_01FEkQ5kbUJTTWAxjV9nqr1P`.
## 维护者速读
- **改了什么**:一个数据集把同一列暴露成两个维度(如 `region`,以及声明在 `region` 上的
`zone`),同时按这两个维度查询时,ObjectQL 策略现在只对这一列分组一次,结果按两个维度名都回填,与原生 SQL
和草稿预览一致。直连路径、带月份桶、带跨对象维度(`account.name`)三种情况都覆盖。顺带并入上次评审的
S1(`analytics-service.ts` 里三份相同的 where 叶子读取合成一个 `whereLeafMembers`)和
S2(补一条「按度量键筛选」的测试)。
- **为什么改**:卡 #28744(p3),是 PR #28742 开发时实测到的同族缺口:同一个查询原生面能答,ObjectQL 面
400 `INVALID_QUERY`。
- **风险与回滚**:只放宽:之前 400 的现在服务。引擎的重复列拒绝不动(`packages/objectql`
未改),同一列的原值和它的月份桶仍是两个键,引擎照旧拒绝,有测试钉住。回滚即 revert。
-
**先看**:`packages/services/service-analytics/src/strategies/objectql-strategy.ts`
新的 `pushGroupKey` 及 `execute`、`executeCrossObject`
两处调用;`analytics-service.ts` 的 `whereLeafMembers`。
- **两分钟验证**:`pnpm --filter @objectstack/service-analytics exec vitest
run src/__tests__/preview-time-filter-fk-parity.test.ts`,预期 23 passed。
- **需要你做的**:无,席位落地。
## What changes
- **#28744 — `strategies/objectql-strategy.ts`.** A module-level
`pushGroupKey(groupBy, item)` appends a group key unless an identical
one is already there: the same column, or the same column bucketed at
the same granularity. The direct path (`execute`) and the cross-object
path (`executeCrossObject`) both build their `groupBy` through it. Two
dataset dimensions over one column share its group, and the row mapping
that already reads each projected dimension from its resolved column
keys the answer by both names, so no re-key step was needed. In the
cross-object path the helper replaces #28692's two FK-only checks (an FK
column under two cross dimensions, a base dimension over the FK column),
which are the same rule.
- **What stays refused.** A column raw beside its own bucket
(`closed_at` with `OBJECT.closed_at` bucketed by month) is two different
keys under one output name, so both are handed on and the engine's
duplicate-column refusal answers `INVALID_QUERY` / 400 on both live
faces, as before. Nothing in `packages/objectql` changes.
- **Review S1 — `analytics-service.ts`.** One `whereLeafMembers(where)`
reader (the guarded
`collectFilterLeaves(normalizeAnalyticsFilterTree(...))` read) for
`namedQueryFields`, `relationshipMembers` and `previewWhereColumns`, in
place of three copies. Same behaviour: a non-object or refused `where`
names nothing. The fourth read in the `where` field gate passes the
whole query and returns early on a refusal, so it is left as it is.
- **Review S2 — test.** A `runtimeFilter` on the measure key `total`
(over `amount`) is read as `amount > 60` on every face, the `'any'` arm
of `declaredMemberEntry` that PR #28742 added for the preview.
- Tests: six cases added to
`src/__tests__/preview-time-filter-fk-parity.test.ts` (now 23, real
SQLite).
- Changeset `.changeset/28744-objectql-two-dimensions-one-column.md`,
`@objectstack/service-analytics` patch.
## Why
Measured on real SQLite (the #28201 / #28555 / #28690 harness), dataset
over a deal object with dimensions `region` and `zone` (both over
`region`), rows amer 100, amer 50, emea 30:
| selection | native | ObjectQL before | preview | ObjectQL after |
|:--|:--|:--|:--|:--|
| dims `region`, `zone` (either order) | amer 150 / emea 30, keyed by
both | `INVALID_QUERY` (groupBy.1 names 'region') | same as native |
same as native |
| …with td `closed_at` month | three groups | `INVALID_QUERY` | same as
native | same as native |
| backed, with `account.name` | three groups | `INVALID_QUERY` | refused
(#28555) | same as native |
| `closed_at` raw + `OBJECT.closed_at` month | `INVALID_QUERY` |
`INVALID_QUERY` | serves | `INVALID_QUERY` (unchanged) |
## Tests
<details><summary>Red first, green, ablations</summary>
- Red on base: `objectql-strategy.ts` restored to `272fec05` (the file
is unchanged up to `origin/main` `7a48bf73`), tests at HEAD: `Tests 4
failed | 19 passed (23)`; the four #28744 cases fail on the ObjectQL leg
with the engine's duplicate-column message. Restored with `git checkout
HEAD --`, `git status` clean. The S2 case is green on base too (PR
#28742 shipped that arm); its red leg is A-S2 below.
- Green at `cb122c70`: `23 passed (23)`.
- Ablations at `cb122c70` (`node scripts/ablation-replace.mjs`, fix
committed, each anchor 1 → 0, restore blob == HEAD, `git diff HEAD`
empty; the suite imports `src`):
- B1 the direct path's `pushGroupKey` back to `groupBy.push`: `3 failed
| 20 passed` (the three direct-path #28744 cases).
- B2 the cross path's base-dimension `pushGroupKey` back to
`groupBy.push`: `2 failed | 21 passed` (`account` after `account.name`,
#28744 beside `account.name`).
- B3 `pushGroupKey` appending unconditionally: `7 failed | 16 passed`
(every shared-column case of #28692 and #28744).
- B4 key identity by column only, so a raw column and its bucket merge:
`1 failed | 22 passed` (the raw-beside-bucket refusal pin).
- A-S2 `--delete` the `previewWhereColumns(q, cube)` argument: `8 failed
| 15 passed` (the S2 case among the #28691 cases).
- `pnpm --filter @objectstack/service-analytics exec vitest run`: `Test
Files 203 passed (203)`, `Tests 4862 passed | 282 skipped` (on
`c8c555f7`, before the rebase onto `7a48bf73`, which touches no
service-analytics file).
- `pnpm --filter @objectstack/rest exec vitest run src/analytics` at
`cb122c70` after a full build: `Test Files 23 passed (23)`, `Tests 298
passed | 22 skipped`.
- `pnpm --filter @objectstack/service-analytics typecheck` at `cb122c70`
exit 0; `eslint` on the three touched source and test files exit 0.
</details>
## Acceptance notes
- The order's hypothesis held for the groupBy and not for the re-key:
the direct path's row mapping already reads every projected dimension
from its resolved column, so once the column is grouped once, both names
are filled. No re-key map was added.
- S1 and S2 are folded in as the order allowed. S1 is in
`analytics-service.ts`, not the strategy file, but it is three call
sites in one file and behaviour-neutral (full suite green).
- Out of scope, measured, not fixed: for `closed_at` raw beside
`OBJECT.closed_at` bucketed by month, both live faces refuse
`INVALID_QUERY` / 400 (two values under one name), while the draft
preview serves both columns. Reported to the seat.
## Verification
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 62 families derived on `cb122c70` (4 paths
vs merge base `7a48bf73`); each run with stdin from `/dev/null`, exit
captured before any pipe, after a full `turbo run build
--filter=!@objectstack/docs --concurrency=2` (lock VERDICT command-exit
0). `--ran`: **62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN**, every one
`exit 0`.
- Hand gates: `node scripts/check-platform-checklist.mjs --changed-from
origin/main` exit 0; `node scripts/check-commit-card-trailers.mjs
--range origin/main..HEAD` exit 0.
- Dogfood battery: not run. Clause-② is widening only, with no `!` and
no narrowing arm.
- CI families: read from this PR's check-runs.
<details><summary>The 62 derived families and their exits</summary>
- `node scripts/check-adr-0087-registration.mjs --base origin/main` ::
exit 0
- `node scripts/check-adr-0087-registration.mjs --self-test` :: exit 0
- `node scripts/check-changeset-no-major.mjs --base origin/main` :: exit
0
- `node scripts/check-closing-keyword-parity.mjs` :: exit 0
- `node scripts/check-closing-keyword-parity.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs` :: exit 0
- `node scripts/check-comment-mask-adoption.mjs --self-test` :: exit 0
- `node scripts/check-comment-mask-corpus.mjs` :: exit 0
- `node scripts/check-dts-emitted.mjs --self-test` :: exit 0
- `node scripts/check-empty-changeset.mjs --base origin/main` :: exit 0
- `node scripts/check-empty-changeset.mjs --self-test` :: exit 0
- `node scripts/check-issue-citations.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs` :: exit 0
- `node scripts/check-plugin-teardown-shape.mjs --self-test` :: exit 0
- `node scripts/check-tenant-audit-census.mjs` :: exit 0
- `node scripts/check-tenant-audit-census.mjs --self-test` :: exit 0
- `node scripts/ci/assert-turbo-remote-cache.mjs --self-test` :: exit 0
- `node scripts/ci/plan-test-core-shards.mjs --self-test` :: exit 0
- `node scripts/ci/prune-turbo-cache.mjs --self-test` :: exit 0
- `node scripts/docs-audit/check-affected-docs.mjs` :: exit 0
- `node scripts/docs-audit/check-drift-comment.mjs` :: exit 0
- `node scripts/pm/release-rehearsal-clone.mjs --self-test` :: exit 0
- `node scripts/release-pending-publish.mjs --self-test` :: exit 0
- `pnpm --filter @objectstack/spec run check:duration-unit-keys` :: exit
0
- `pnpm check:changeset-gate-self-tests` :: exit 0
- `pnpm check:ci-filter-parity` :: exit 0
- `pnpm check:closed-ceremony-text` :: exit 0
- `pnpm check:cross-package-test-inputs` :: exit 0
- `pnpm check:dispatcher-error-vocabulary` :: exit 0
- `pnpm check:doc-authoring` :: exit 0
- `pnpm check:driver-memory-census` :: exit 0
- `pnpm check:dts-closure` :: exit 0
- `pnpm check:dual-build-cjs-loads` :: exit 0
- `pnpm check:engine-double-contract` :: exit 0
- `pnpm check:error-status-conformance` :: exit 0
- `pnpm check:gitlink-declared` :: exit 0
- `pnpm check:issue-citations` :: exit 0
- `pnpm check:keyed-text-bounds` :: exit 0
- `pnpm check:lean-entry-closure` :: exit 0
- `pnpm check:logger-receiver-detach` :: exit 0
- `pnpm check:nul-bytes` :: exit 0
- `pnpm check:objectql-double-limit` :: exit 0
- `pnpm check:org-identifier` :: exit 0
- `pnpm check:page-declaration-shape` :: exit 0
- `pnpm check:platform-object-tenancy-census` :: exit 0
- `pnpm check:pm-changeset-deadline-census` :: exit 0
- `pnpm check:published-files` :: exit 0
- `pnpm check:query-options-erasure` :: exit 0
- `pnpm check:refd-timer-probe` :: exit 0
- `pnpm check:registry-log-declared` :: exit 0
- `pnpm check:rest-log-spy-declared` :: exit 0
- `pnpm check:slot-lookup` :: exit 0
- `pnpm check:sourcemap-no-sources-content` :: exit 0
- `pnpm check:swallow-census-controls` :: exit 0
- `pnpm check:system-context-census` :: exit 0
- `pnpm check:test-source-alias` :: exit 0
- `pnpm check:tier-file-adoption` :: exit 0
- `pnpm check:type-check-coverage` :: exit 0
- `pnpm check:type-check-debt` :: exit 0
- `pnpm check:undeclared-dep-imports` :: exit 0
- `pnpm check:watch-hint-literal` :: exit 0
- `pnpm check:where-matcher` :: exit 0
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(No `packages/spec` change.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes (scoped run above; 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: 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 (the 维护者速读 block is the
dispatch contract's).
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P
Co-authored-by: Claude <noreply@anthropic.com>1 parent 60d8522 commit 52de5a5
4 files changed
Lines changed: 141 additions & 39 deletions
File tree
- .changeset
- packages/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
24 | 29 | | |
25 | 30 | | |
26 | 31 | | |
| |||
248 | 253 | | |
249 | 254 | | |
250 | 255 | | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
251 | 265 | | |
252 | 266 | | |
253 | 267 | | |
| |||
297 | 311 | | |
298 | 312 | | |
299 | 313 | | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
300 | 371 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
452 | 452 | | |
453 | 453 | | |
454 | 454 | | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
455 | 474 | | |
456 | 475 | | |
457 | 476 | | |
| |||
483 | 502 | | |
484 | 503 | | |
485 | 504 | | |
486 | | - | |
487 | | - | |
488 | | - | |
489 | | - | |
490 | | - | |
491 | | - | |
492 | | - | |
493 | | - | |
494 | | - | |
495 | | - | |
| 505 | + | |
496 | 506 | | |
497 | 507 | | |
498 | 508 | | |
| |||
630 | 640 | | |
631 | 641 | | |
632 | 642 | | |
633 | | - | |
634 | | - | |
635 | | - | |
636 | | - | |
637 | | - | |
638 | | - | |
639 | | - | |
640 | | - | |
641 | | - | |
642 | 643 | | |
643 | 644 | | |
644 | 645 | | |
645 | | - | |
| 646 | + | |
646 | 647 | | |
647 | | - | |
| 648 | + | |
648 | 649 | | |
649 | | - | |
| 650 | + | |
650 | 651 | | |
651 | 652 | | |
652 | 653 | | |
| |||
4798 | 4799 | | |
4799 | 4800 | | |
4800 | 4801 | | |
4801 | | - | |
4802 | | - | |
4803 | | - | |
4804 | | - | |
4805 | | - | |
4806 | | - | |
4807 | | - | |
4808 | | - | |
4809 | 4802 | | |
4810 | | - | |
| 4803 | + | |
4811 | 4804 | | |
4812 | 4805 | | |
4813 | 4806 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
220 | 220 | | |
221 | 221 | | |
222 | 222 | | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
223 | 245 | | |
224 | 246 | | |
225 | 247 | | |
| |||
253 | 275 | | |
254 | 276 | | |
255 | 277 | | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
256 | 282 | | |
257 | 283 | | |
258 | 284 | | |
259 | 285 | | |
260 | 286 | | |
261 | | - | |
| 287 | + | |
262 | 288 | | |
263 | 289 | | |
264 | 290 | | |
265 | 291 | | |
266 | 292 | | |
267 | | - | |
| 293 | + | |
268 | 294 | | |
269 | 295 | | |
270 | 296 | | |
| |||
1345 | 1371 | | |
1346 | 1372 | | |
1347 | 1373 | | |
1348 | | - | |
| 1374 | + | |
1349 | 1375 | | |
1350 | 1376 | | |
1351 | 1377 | | |
1352 | 1378 | | |
1353 | | - | |
1354 | | - | |
1355 | | - | |
1356 | | - | |
| 1379 | + | |
| 1380 | + | |
| 1381 | + | |
| 1382 | + | |
1357 | 1383 | | |
1358 | 1384 | | |
1359 | 1385 | | |
1360 | 1386 | | |
1361 | 1387 | | |
1362 | | - | |
| 1388 | + | |
1363 | 1389 | | |
1364 | 1390 | | |
1365 | 1391 | | |
| |||
0 commit comments