Repository navigation
Commit 0c0d3b6
fix(service-analytics)!: the /analytics/sql dry run refuses a date bucket over a column that is not a date or datetime, as the query does (#29089)
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 #28977
Clause-②: no (narrowing) — `/analytics/sql`
(`AnalyticsService.generateSql`) now refuses `INVALID_FIELD` / 400 at
`groupBy.N.dateGranularity` a time-dimension granularity over a field
declared neither a date nor a datetime (text, select, number, boolean,
time, …), with the refusal `/analytics/query` gives the same selection,
field for field. It used to render a statement for it (or, at a non-UTC
zone, answer `NOT_IMPLEMENTED` / 501 saying the query is served).
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` says the
`/analytics/sql` dry run renders a date bucket over a non-date column.
**BREAKING** accept-set narrowing on the `/analytics/sql` dry run
(`@objectstack/service-analytics`). `/analytics/query` is unchanged.
Card thread: `session_01FEkQ5kbUJTTWAxjV9nqr1P`.
## 维护者速读
- **改了什么**:
- `/analytics/sql`(只出
SQL、不执行的预演口)遇到把非日期列按时间粒度分桶的选择(文本、选项、数字、布尔、time),现在和 `/analytics/query`
一样拒绝:`INVALID_FIELD` / 400,路径
`groupBy.N.dateGranularity`,报错逐字段一致。以前预演口照样给出 `strftime('%Y-%m',
region)` 这样的 SQL;在非 UTC 时区下还会答 501,说「查询本身能服务」,其实查询会拒绝。
- 顺带修复(#28976 评审的两条 should-fix):
- 草稿预览把行过滤(`where` 和 `dateRange`
窗口)提前到「同一列要两次」和「非日期列分桶」两道门之前,只降一次,和线上的拒绝优先级一致。
- 钉住三个以前没钉的分支:`formula` 跳过、`time` 类型、数据集维度声明的默认粒度落在文本列上。
- **为什么改**:卡 #28977(p3),是我在 #28852 测量时发现的范围外问题。席位裁定:预演口复用 #28976
加的那份判读,用 `execute` 构建的同一组键,按 `query()` 的拒绝顺序;不加第三份文字,线上两面不加门,不碰
objectql。
- **风险与回滚**:这是收窄,以前能出 SQL 的预演现在被拒,但只限查询本身就拒绝的选择。日期、日期时间列的分桶 SQL
一字不差(已钉住)。回滚即 revert。
- **先看**:
- `strategies/objectql-strategy.ts` 的 `generateSql`:列重复门后面新加的一次调用;
- `dataset-date-bucket-door.ts`(由 `preview-date-bucket-door.ts`
改名,现在两面共用);
- `preview-evaluator.ts` 的新函数 `lowerPreviewRowFilters`,以及
`analytics-service.ts` 预览门清单里的调用位置。
- **两分钟验证**:`pnpm --filter @objectstack/service-analytics exec vitest
run src/__tests__/objectql-echo-non-date-bucket.test.ts
src/__tests__/preview-date-bucket-type.test.ts`,预期 14 + 14 passed。
- **需要你做的**:无。席位跑合约评审并落地。
## What changes
- **`generateSql` refuses through the service's one reading.** Right
after the column-asked-twice door (#28841), before any bucket renders,
the echo calls `assertDatasetDateBucketsTemporal`.
- It runs on the same keys as that door: `aggregateGroupKeys(query,
cube)` on the direct path, and `crossObjectGroupKeys(query, cube,
crossByDim)` when `plan` routes the selection to the cross-object path,
as `execute()` builds them. So `N` is the engine's position.
- It runs on the same object (`tableName`, the object `execute()`
aggregates).
- It reads the type through the strategy context's `declaredFieldType`,
which the service answers from `sourceFieldMeta`, the hook the preview
reads. A host that wires no reader is not judged, as the engine judges
no undeclared field.
- The order is `query()`'s: the `dateRange` windows are read first
(#28852), then the column refusal, then this one.
- **`preview-date-bucket-door.ts` → `dataset-date-bucket-door.ts`, and
`assertPreviewDateBucketsTemporal` →
`assertDatasetDateBucketsTemporal`.** The body is unchanged. Only the
header now names both callers, as `dataset-group-column-door.ts` was
renamed when the strategy began calling it (#28798). The engine's
sentence still has exactly two copies (engine, this module), and no live
face gains a gate.
- **Ride-along 1, the preview's precedence.** `preview-evaluator.ts`
gains `lowerPreviewRowFilters(query, cube, declaredType, whereColumns)`.
It is the evaluator's step 1 (the `where`, then each `dateRange`
window), moved verbatim and returning the row matcher.
- `evaluateAnalyticsQueryOverRows` takes it as an optional last
argument. Without it, the evaluator lowers them itself, first, as
before.
- The preview calls it once, after its `ensureCube` / `query()` gate
list and before its two doors, where the ObjectQL strategy lowers its
filter and its windows, and hands the result to the evaluator.
- Why the `where` moves too: lowering only the windows there would have
answered a refused filter after an unrecognised `dateRange`. Measured,
both live faces answer the filter first (pinned).
- **Ride-along 2, the pins.** In `preview-date-bucket-type.test.ts`:
- a `time` column on every face;
- a dataset dimension's declared `dateGranularity` over a text column;
- the `formula` skip: no face answers the date-bucket refusal for a
formula bucket, and the engine does not judge a formula either.
- **Census.** The new `objectql-echo-non-date-bucket.test.ts` calls
`analytics.query` twice. So
`packages/client/src/envelope-caller-census.test.ts` (#13079) gains its
ledger row (2), the service sites move 15 → 17 and `NOT_SDK` 15 → 17.
The file is declared in `scripts/cross-package-test-inputs.mjs` and
`turbo.json`'s `@objectstack/client#test` inputs.
- Tests: new `src/__tests__/objectql-echo-non-date-bucket.test.ts` (14
cases), plus 7 cases in `preview-date-bucket-type.test.ts` (7 → 14).
Changeset:
`.changeset/28977-sql-dry-run-refuses-date-bucket-over-non-date.md`,
`@objectstack/service-analytics` patch.
## Why
Measured on `main` `ab085a04` with a probe over real SQLite, comparing
`query()` with `generateSql()` on both compositions (default, and
ObjectQL-only). The two compositions answered alike throughout.
| selection | `/analytics/query` | `/analytics/sql` on `main` |
|:--|:--|:--|
| text `region` by month | `INVALID_FIELD` / 400
`groupBy.0.dateGranularity` | `SELECT strftime('%Y-%m', \`region\`) …
GROUP BY strftime(…)` |
| select, number (year), boolean | the same | renders over that column |
| `time` column by day | the same | renders `strftime('%Y-%m-%d',
\`opened_time\`)` |
| `account.name` beside a text bucket (cross-object path) | the same at
`groupBy.1` | the join and the bucket |
| raw date, datetime bucket, text bucket | the same at `groupBy.2` | all
three columns |
| a cube time dimension over `region` declaring one granularity, asked
raw | the same | renders the bucket |
| text bucket at `America/New_York` | the same | `NOT_IMPLEMENTED` /
501, "the query itself is served" |
| date / datetime bucket | served | renders, the same statement the
query echoes |
The draft preview, on the same `main`, compared with both live faces of
`queryDataset`:
| selection | native / ObjectQL | draft preview |
|:--|:--|:--|
| unrecognised or one-entry `dateRange` on a text bucket |
`ANALYTICS_DATE_RANGE_UNRECOGNIZED` | `INVALID_FIELD` (the bucket) |
| unrecognised `dateRange` beside one column asked twice |
`ANALYTICS_DATE_RANGE_UNRECOGNIZED` | `INVALID_QUERY` (the column) |
| refused `runtimeFilter` (`$lt: null`) beside a bad `dateRange` on a
text bucket | `INVALID_FILTER` | `INVALID_FIELD` (the bucket) |
| `time` column, dataset-default granularity over text | `INVALID_FIELD`
| already `INVALID_FIELD` (now pinned) |
## Tests
<details><summary>Red first, green, ablations</summary>
- **Red on base.** `analytics-service.ts`,
`strategies/objectql-strategy.ts`, `preview-evaluator.ts` and
`preview-date-bucket-door.ts` were restored to `ab085a04` (the renamed
door moved aside), with both test files at HEAD. Result: `Tests 13
failed | 15 passed (28)`.
- All 9 echo refusal cases failed on the dry run resolving, or on the
501.
- The 4 preview precedence cases failed on the door's refusal.
- The served pins and the three new preview pins (time, dataset-default
granularity, formula) passed: those arms already held.
Restored with `git checkout HEAD --`, then `git status` was clean.
- **Green.** Both files pass (14 + 14). The full service-analytics suite
gave `Test Files 206 passed (206)`, `Tests 4919 passed | 282 skipped`.
The client census file gave 21 passed. `tsc --noEmit` and scoped eslint
exited 0.
- **Ablations** used `node scripts/ablation-replace.mjs` with the fix
committed first. Each run covered both files plus
`objectql-echo-column-twice.test.ts` and
`preview-time-filter-fk-parity.test.ts` (80 cases). Each anchor went 1 →
0, and each restore came back with blob == HEAD and an empty `git diff
HEAD`:
- D1: make the echo's call a no-op. Result: `9 failed | 71 passed`,
every echo refusal case.
- D2: judge the bucket before the echo's column door. Result: `1
failed`, the column-asked-twice precedence case.
- D3: judge the bucket before the echo's `dateRange` read. Result: `3
failed`, the three precedence cases.
- D4: hand the evaluator no lowered filters, so it lowers them after the
doors again. Result: `4 failed`, the four preview precedence cases.
- D5: drop the `formula` skip. Result: `1 failed`, the formula pin.
- D6: exempt `time`. Result: `2 failed`, the time case on the echo and
on the preview.
- D7: make the preview's call a no-op. Result: `7 failed`, the four
#28852 types, `time`, the later position and the dataset-default
granularity.
</details>
- **Gates.** After a full turbo build (lock VERDICT command-exit 0), all
81 families `dispatch-gates --commands` derives for this diff ran with
stdin `/dev/null` and the exit captured before any pipe. `--ran` gave 81
derived, 81 run, 0 NOT-MEASURED, all exit 0.
- **Dogfood: NOT MEASURED.** The battery was running on `ec15d7c4` under
the verify lock when the seat asked to wrap up, and I did not wait for
its verdict. It is owed (the title carries `!`).
## Out of scope (reported to the seat, not fixed)
- A date bucket over a `formula` field that the table stores no column
for: both live faces refuse `INVALID_FIELD` / 400 from the SQL driver
("no column"). The engine does not judge a formula's value class, so the
date-bucket door does not either. Meanwhile, `/analytics/sql` renders
`strftime('%Y-%m', calc)` for it, and the draft preview serves one
`null` group holding the total. This is the virtual-formula family, not
the date-bucket one, and it was not touched here.
🤖 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 45e9b2f commit 0c0d3b6
10 files changed
Lines changed: 417 additions & 58 deletions
File tree
- .changeset
- packages
- client/src
- services/service-analytics/src
- __tests__
- strategies
- scripts
| 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 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
531 | 531 | | |
532 | 532 | | |
533 | 533 | | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
534 | 540 | | |
535 | 541 | | |
536 | 542 | | |
| |||
771 | 777 | | |
772 | 778 | | |
773 | 779 | | |
774 | | - | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
775 | 785 | | |
776 | 786 | | |
777 | 787 | | |
778 | 788 | | |
779 | 789 | | |
780 | 790 | | |
781 | 791 | | |
| 792 | + | |
782 | 793 | | |
783 | 794 | | |
784 | 795 | | |
| |||
825 | 836 | | |
826 | 837 | | |
827 | 838 | | |
828 | | - | |
| 839 | + | |
829 | 840 | | |
830 | 841 | | |
831 | 842 | | |
832 | 843 | | |
833 | 844 | | |
834 | 845 | | |
835 | | - | |
| 846 | + | |
| 847 | + | |
836 | 848 | | |
837 | 849 | | |
838 | 850 | | |
| |||
| 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 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 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 | + | |
0 commit comments