Repository navigation
Commit 7e60dc2
fix(service-analytics): a dataset selection asking one column twice is refused with a repair the selection can make, not the engine's "own 'alias'" (#28851)
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 #28798
Clause-②: no — the accept set of `queryDataset` is unchanged on every
face: each selection served on `main` is served, each refused is refused
with status 400. Only the refusal's prescription moves, and the ObjectQL
strategy now refuses these keys itself, right before `engine.aggregate`.
Nothing is widened: no package-root export, public member, answer
member, error code or status is added.
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` or `content/docs` quotes
the duplicate-column refusal or tells a dataset author to alias a group
entry.
Card thread: `session_01FEkQ5kbUJTTWAxjV9nqr1P`.
## 维护者速读
- **改了什么**:数据集查询里同一列被要两次(原始列加它自己的时间桶,或同一列的两个粒度),三条执行面(原生
SQL、ObjectQL、草稿预览)现在用同一个数据集拒绝回答。`code`、状态码、`param`、`path`、`object`、`fields`
都不变,只换处方:不再叫作者「给分组项起一个 alias」(数据集选择里没有
alias),而是点名选择里涉及这一列的成员,并给出能做的修法:去掉原始维度或去掉那个时间桶的粒度;两个粒度时只留一个。
- **为什么改**:卡 #28798(p3),是 PR #28794 合约评审的 should-fix
2。席位裁定:预览门变成数据集面唯一的拒绝,ObjectQL 策略在 `engine.aggregate` 之前调用它;引擎的文字不动。
- **风险与回滚**:接受集合不变(之前服务的仍服务,之前拒绝的仍 400
拒绝),只是拒绝文字变了。有一处要你知道:一个选择同时犯两个错(既把文本列按月分桶,又把同一列要两次)时,两个线上面以前答引擎的「非日期列不能分桶」(`INVALID_FIELD`),现在答本拒绝(`INVALID_QUERY`),与草稿预览原本的回答一致。见下文「一处优先级变化」。回滚即
revert。
-
**先看**:`packages/services/service-analytics/src/dataset-group-column-door.ts`(由
`preview-group-column-door.ts` 改名重写),`strategies/objectql-strategy.ts` 中
`execute` 和 `executeCrossObject` 在 `ctx.executeAggregate` 前各一行调用。
- **两分钟验证**:`pnpm --filter @objectstack/service-analytics exec vitest
run src/__tests__/preview-time-filter-fk-parity.test.ts`,预期 37 passed。
- **需要你做的**:无,席位落地;那处优先级变化由席位裁定。
## What changes
- **One refusal for every dataset face.** `preview-group-column-door.ts`
becomes `dataset-group-column-door.ts`, and
`assertPreviewGroupColumnsDistinct(query, cube)` becomes
`assertDatasetGroupColumnsDistinct(object, groupBy, query,
memberColumn)`. It judges the `groupBy` keys it is handed, so each
caller hands it the keys it built:
- `ObjectQLStrategy.execute` calls it on `aggregateGroupKeys(query,
cube)`, right before `ctx.executeAggregate`;
- `executeCrossObject` calls it on its own keys (an FK column for a
cross-object dimension), right before its base `ctx.executeAggregate`;
- the draft preview calls it where it called the old door, on
`aggregateGroupKeys(q, cube)` over `aggregateObjectOf(cube)`.
Each call sits right before the engine call, so every refusal the
strategy gave before keeps its precedence.
- **The sentence.** The head and tail are the engine's, so a reader of
both sees the same shape. Only the prescription between them differs,
and it names the selection's members over the column, read with the
strategy's `resolveCubeFieldName`:
- raw beside its bucket (either key the bare column): "The selection
asks for 'closed_at' both raw and bucketed (`dimensions` 'closed_at';
`timeDimensions` 'OBJECT.closed_at' by month). Drop the raw dimension
from `dimensions`, or drop the granularity from the `timeDimensions`
entry."
- two buckets: "The selection asks for 'closed_at' at two granularities
(`timeDimensions` 'closed_at' by month, 'closed_at' by year). Ask one
granularity of it in `timeDimensions`."
A dimension bucketed by its own `timeDimensions` entry is not listed as
raw. "Drop the granularity" rather than "drop the entry", because a
`timeDimensions` entry without a granularity still windows the query.
- **Surface.** `resolveCubeFieldName` becomes a module-level export of
`objectql-strategy.ts`, as `aggregateGroupKeys` and `aggregateObjectOf`
already are. The package root re-exports `ObjectQLStrategy` by name
only, so the root surface is unchanged. `ObjectQLStrategy` gains no
member. The door imports only a type from the strategy module, so there
is no runtime import cycle.
- **Untouched.** `packages/objectql` is untouched, and a direct
`engine.aggregate` caller still gets the engine's "own 'alias'"
prescription (pinned). `generateSql` (`/analytics/sql`, #28841) is
untouched.
- Tests: `src/__tests__/preview-time-filter-fk-parity.test.ts` (34 → 37
cases). Changeset:
`.changeset/28798-dataset-column-twice-refusal-prescription.md`,
`@objectstack/service-analytics` patch.
## Why
Measured on `origin/main` `fa59f787` with a probe over real SQLite (all
three faces):
- **Every duplicate reaches the strategy.** Two keys over one column
need a bucket, because the strategy names identical keys once (#28744).
The native strategy declines every bucketed time dimension, so both live
faces answered from the ObjectQL strategy's call into the engine. There
is no native-own refusal on any of these selections. This held for raw
beside a bucket of another spelling, a second dataset dimension over the
column, and two buckets.
- **Every face prescribed the alias.** All three answered `… The query
was NOT run. Give one of the group entries its own 'alias'. …`, and a
dataset selection has no alias.
- **A direct `engine.aggregate`** with `['closed_at', { field:
'closed_at', dateGranularity: 'month' }]` answers the same refusal. That
is the backstop this change leaves in place.
### One precedence change (for the seat's ruling)
The engine runs its other `groupBy` doors before the duplicate-column
door. One of them refuses a date bucket over a non-date column
(`INVALID_FIELD`). Measured on `main`, the selection `dimensions:
['region']` with `timeDimensions: [{ dimension: 'zone', granularity:
'month' }]` (`zone` is declared over the text column `region`) answered:
- both live faces: `INVALID_FIELD`, `groupBy.1.dateGranularity`;
- the draft preview: the column refusal.
With the door before `engine.aggregate`, as ruled, all three faces now
answer the column refusal (`INVALID_QUERY`, `groupBy.1.field`). It is
still a 400 refusal, and fixing it meets the other refusal. The new pin
`[#28798] answers it on every face before the engine judges a bucket
over a text column` holds this. No other measured selection changed. The
os-dev-report carries this as an open question with options.
## Tests
<details><summary>Red first, green, ablations</summary>
- **Red on base.** `analytics-service.ts`,
`strategies/objectql-strategy.ts` and `preview-group-column-door.ts`
were restored to `fa59f787`, and the new door file was moved aside. The
test stayed at HEAD. Result: `Tests 11 failed | 26 passed (37)`.
- Every refusal case failed on the message: `Received: "… Give one of
the group entries its own 'alias'. …"`.
- The precedence pin failed on the live faces' `INVALID_FIELD`.
- The direct-engine pin passed.
Restored with `git checkout HEAD --`, then `git status` was clean.
- **Green.** `37 passed (37)`. The full service-analytics suite gave
`Test Files 203 passed (203)`, `Tests 4876 passed | 282 skipped`. `tsc
--noEmit` and scoped eslint both exited 0.
- **Ablations** used `node scripts/ablation-replace.mjs` with the fix
committed first. Each anchor went 1 → 0, and each restore came back with
blob == HEAD and an empty `git diff HEAD`:
- A1: delete the `execute` call, so the engine answers again. Result: `9
failed | 27 passed`, every live-face refusal pin on the direct path.
- A2: delete the `executeCrossObject` call. Result: `1 failed | 35
passed`, the cross-object pair.
- A3: make the preview's call a no-op. Result: `9 failed | 27 passed`,
because the preview serves both columns again.
- A4: always use the two-granularity sentence. Result: `3 failed | 33
passed`, the raw-beside-bucket cases.
- A5: list a dimension bucketed by its own entry as raw. Result: `1
failed | 36 passed`.
- A6: compare a `timeDimensions` member's spelling instead of its
column. Result: `5 failed | 32 passed`.
</details>
## Out of scope (reported to the seat, not fixed)
- On `main`, the draft preview serves a bucket over a text column
(`timeDimensions: [{ dimension: 'zone', granularity: 'month' }]`) as one
`null` bucket holding the total. Both live faces refuse it with
`INVALID_FIELD`. That is a separate preview parity gap.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5kbUJTTWAxjV9nqr1P)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent e7a6cc7 commit 7e60dc2
6 files changed
Lines changed: 252 additions & 105 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 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
38 | 45 | | |
39 | 46 | | |
40 | 47 | | |
| |||
350 | 357 | | |
351 | 358 | | |
352 | 359 | | |
353 | | - | |
| 360 | + | |
354 | 361 | | |
355 | | - | |
356 | | - | |
| 362 | + | |
| 363 | + | |
357 | 364 | | |
358 | 365 | | |
359 | 366 | | |
| |||
377 | 384 | | |
378 | 385 | | |
379 | 386 | | |
380 | | - | |
381 | | - | |
382 | | - | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
383 | 409 | | |
384 | | - | |
385 | | - | |
| 410 | + | |
| 411 | + | |
386 | 412 | | |
387 | 413 | | |
388 | 414 | | |
389 | 415 | | |
390 | 416 | | |
391 | 417 | | |
| 418 | + | |
392 | 419 | | |
393 | 420 | | |
394 | 421 | | |
395 | 422 | | |
396 | 423 | | |
397 | | - | |
398 | | - | |
| 424 | + | |
| 425 | + | |
399 | 426 | | |
400 | 427 | | |
401 | 428 | | |
402 | 429 | | |
403 | 430 | | |
404 | 431 | | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
405 | 439 | | |
406 | 440 | | |
407 | 441 | | |
408 | 442 | | |
409 | 443 | | |
410 | 444 | | |
411 | 445 | | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
412 | 457 | | |
413 | 458 | | |
414 | 459 | | |
| |||
422 | 467 | | |
423 | 468 | | |
424 | 469 | | |
| 470 | + | |
425 | 471 | | |
426 | 472 | | |
427 | 473 | | |
| |||
432 | 478 | | |
433 | 479 | | |
434 | 480 | | |
| 481 | + | |
435 | 482 | | |
436 | 483 | | |
437 | 484 | | |
| |||
445 | 492 | | |
446 | 493 | | |
447 | 494 | | |
| 495 | + | |
448 | 496 | | |
449 | 497 | | |
450 | 498 | | |
451 | 499 | | |
| 500 | + | |
| 501 | + | |
452 | 502 | | |
453 | 503 | | |
454 | 504 | | |
455 | 505 | | |
456 | 506 | | |
457 | 507 | | |
458 | 508 | | |
| 509 | + | |
459 | 510 | | |
460 | 511 | | |
461 | 512 | | |
| |||
467 | 518 | | |
468 | 519 | | |
469 | 520 | | |
| 521 | + | |
470 | 522 | | |
471 | 523 | | |
472 | 524 | | |
473 | 525 | | |
474 | | - | |
475 | | - | |
476 | | - | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
477 | 529 | | |
478 | 530 | | |
479 | 531 | | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
480 | 550 | | |
481 | 551 | | |
482 | 552 | | |
483 | 553 | | |
484 | | - | |
485 | | - | |
486 | | - | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
487 | 558 | | |
488 | | - | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
489 | 571 | | |
490 | 572 | | |
491 | 573 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | | - | |
| 88 | + | |
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
| |||
116 | 116 | | |
117 | 117 | | |
118 | 118 | | |
119 | | - | |
| 119 | + | |
120 | 120 | | |
121 | 121 | | |
122 | 122 | | |
| |||
2891 | 2891 | | |
2892 | 2892 | | |
2893 | 2893 | | |
2894 | | - | |
2895 | | - | |
2896 | | - | |
2897 | | - | |
2898 | | - | |
| 2894 | + | |
| 2895 | + | |
| 2896 | + | |
| 2897 | + | |
| 2898 | + | |
| 2899 | + | |
| 2900 | + | |
| 2901 | + | |
| 2902 | + | |
| 2903 | + | |
| 2904 | + | |
2899 | 2905 | | |
2900 | 2906 | | |
2901 | 2907 | | |
| |||
0 commit comments