Commit fbe2deb
fix(service-analytics): the ObjectQL face applies a query's order, offset and limit, as its echoed sql says (#21363)
Fixes #21316
Clause-②: no
## What this changes
**The ObjectQL face applies `order`, then `offset` and `limit`, to the
aggregated answer**, as its echoed `sql` and `POST
/api/v1/analytics/sql` say it does
(`packages/services/service-analytics/src/strategies/objectql-strategy.ts`,
`orderAndWindow`).
- `engine.aggregate` declares no ordering or window grammar
(`EngineAggregateOptionsSchema`,
`packages/spec/src/data/data-engine.zod.ts`), and neither does the
`executeAggregate` contract (`StrategyContext.executeAggregate`,
`packages/spec/src/contracts/analytics-service.ts`). So there is nothing
to push down, and the strategy applies the three keys itself. It does
this on the direct path and on the cross-object (FK-expand) path, after
the re-bucket. Before the re-bucket, a limit would cut FK groups that
merge into a group it keeps.
- It runs on the remapped rows, which are keyed by the member spellings
the caller selected. So an `order` key names a column exactly as the
analytics door's order-key rule (`order-key-door.ts`, #21267) admitted
it. `order` is read verbatim, in its own key order, and no implicit
ordering is added. A bare `limit` slices the engine's order, the same as
`LIMIT` without `ORDER BY`.
- **One ordering rule.** The comparison is `applyOrdering` and the
window is `applyWindow`. Both come from `dataset-executor.ts`, the
package's one row comparator and window, which the dataset door's
post-pass already applies to every grid. Nothing is copied. Other
comparators exist in or near the package, and none was reused:
`preview-evaluator.ts` has its own `compare` for draft rows, and
`driver-memory`'s `applySort` is private.
**The dataset door no longer applies a pushed-down `offset` twice**
(`dataset-executor.ts`). For a single-query selection, `DatasetExecutor`
pushes `order`, `limit` and `offset` down to the strategy and then
windowed the answer again. The second slice applies `offset` a second
time. This is a shipped defect on the native face, and this change would
have extended it to the ObjectQL face, which was right before only
because it dropped the window. The post-pass now windows only a grid it
could not push down. It still re-sorts every grid.
## Measured before / after
The measurement went through the real `POST /api/v1/analytics/query` and
`/analytics/sql` route (`createDispatcherPlugin`), using
`AnalyticsServicePlugin` with its default composition and with
capabilities narrowed to the engine aggregate, a real `ObjectQL` engine
and `SqlDriver`. It ran on SQLite (better-sqlite3) and PostgreSQL 16.14
(C.UTF-8 collation).
| query (ObjectQL face) | `main` 97239c3, SQLite | `main`, PostgreSQL
| this branch, both |
|:--|:--|:--|:--|
| `timeDimensions [closed_on, month]`, `order { closed_on: desc }`,
`limit 1` | every month, ascending | every month, `04, 03, 05` | the
newest month alone |
| `dimensions [note]`, `order { note: desc }` | unordered | unordered |
the native face's rows |
| `order { amount_sum: desc }`, `limit 2`, `offset 1` | every group |
every group | the native face's rows |
On both drivers the echoed `sql` rendered `… ORDER BY "closed_on" DESC
LIMIT 1` (and the others), before and after.
Dataset door, `limit 2, offset 1`, ordered by a measure over five
groups. Before, the native face answered one row (the third) on both
drivers, and the ObjectQL face answered the second and third. After,
both faces answer the second and third.
## Pins (`src/__tests__/objectql-face-order-limit.test.ts`, SQLite
always, PostgreSQL where `OS_TEST_POSTGRES_URL` is set)
- The card's row: a bucketed time dimension ordered `desc` with `limit
1` answers the newest bucket, on both compositions, and no raw statement
runs. Also a bucketed time dimension with `offset 1, limit 2`.
- These answer exactly what the native face answers: a selected
dimension (the card's row), a selected measure with `offset`/`limit`,
two order keys, `limit 0`, and the cross-object path ordered by a
measure and by the relationship-path dimension.
- The echoed `sql`, run on the same driver, answers the rows the
ObjectQL face answered.
- CONTROL: with no `order`/`offset`/`limit`, the answer is exactly the
engine aggregate's rows in the engine's order (a selected dimension and
a bucketed one).
- The dataset door's pushed-down page, on both faces.
- The #21267 controls in `order-key-selected.test.ts` now assert the
ObjectQL face's order, not only its set.
## Ablations (committed tree, each leg through
`scripts/ablation-replace.mjs` with the anchor proven to land and the
restore proven by blob hash plus an empty `git diff HEAD`; the pins
import the strategy from `src/`, so no `dist/` is involved)
Recorded at `c6c4ae9d0`. The later merge of `main` (`a3179775d`) leaves
`packages/services/service-analytics/` byte-identical: `git diff
c6c4ae9 a317977` over that path is empty. The pin file has 26 cases,
13 per driver.
- **Ordering removed** (`applyWindow(rows, …)`): 17 red, 9 green. Every
order pin is red on SQLite. The 9 green are the controls, `limit 0`, and
windows that the arrival order happens to satisfy on one driver.
- **Window removed** (`applyOrdering(rows, query.order)`): 18 red. Every
`limit`/`offset` pin is red on both drivers. The order-only pins and the
controls stay green.
- **Ordering inverted** (`.reverse()`): 20 red. Every native-equality
pin and both controls are red on both drivers. The dataset-door pins
stay green, because the executor re-sorts a middle window that happens
to be symmetric. The next leg owns those pins.
- **Executor re-windows a pushed-down page**: exactly the 4 dataset-door
pins are red (2 per driver), and nothing else.
- The first run of that last leg was a no-op that the tool refused (its
replacement text was a substring of the anchor, so the count could not
rise). It was re-run with a distinct replacement. The first runs of the
other three legs, on an earlier fixture, showed that SQLite's ascending
group order satisfied two order pins. The fixture was changed so that no
order pin matches that arrival order, and all four legs were re-run.
## Acceptance notes
- **Comparator semantics against the native face.** Both faces answer
the same rows for numbers and for text of single-case ASCII letters.
They can differ on NULL, `''`, numeric text and mixed case. This was
measured on this branch at the route:
- The native `ORDER BY` places NULL lowest on SQLite and highest on
PostgreSQL, and orders text by the column collation (codepoint on
SQLite, and on PostgreSQL under C.UTF-8).
- `applyOrdering` keeps NULL and `''` last in both directions, compares
numeric text numerically, and compares other text with `localeCompare`.
- This is the dataset door's existing, documented rule ("Null / empty
buckets sort last in both directions",
`content/docs/data-modeling/analytics.mdx`). It is reused, not changed.
The open question is in the report.
- **`limit` / `offset` outside the non-negative integers.**
`AnalyticsQuerySchema` declares both as `z.number()`. Measured on this
branch:
- `limit: -1`: native SQLite returns every row, native PostgreSQL
answers 500. The ObjectQL face now answers `applyWindow`'s slice (all
but the last row), as the dataset door already does for the same value.
Before, it answered every row.
- `limit: 1.5`: native SQLite answers 500, PostgreSQL rounds to 2 rows,
the ObjectQL face answers 1 row.
- `offset: -1`: the native face answers 500 on both drivers.
- No answer agrees across drivers. These are in the report as a finding;
this PR leaves them as they are.
- **`offset` without `limit` on SQLite.** The native face renders
`OFFSET n` with no `LIMIT`, which SQLite refuses (`near "OFFSET": syntax
error`, 500). The ObjectQL face answers it, and its echo renders the
same statement. This is in the report as a finding.
- Docs: no page in `content/docs/**` (outside `releases/`) or
`skills/**` says the ObjectQL face ignores `order`/`limit`, so no doc
sentence becomes false. Code comments that said so, in the
`dataset-executor.ts` header and post-pass and in the
`order-key-door.ts` header, are corrected.
## Verification (head `a3179775d`, after merging `main`)
- `pnpm --filter @objectstack/service-analytics test` with live
PostgreSQL 16.14 (`OS_TEST_POSTGRES_URL`, so no cell skipped): 167
files, 3869 tests passed. Its dependency closure was rebuilt first.
- `pnpm --filter @objectstack/service-analytics typecheck`: exit 0. Both
touched test files are in its program (`tsc --listFiles`).
- The derived gate union (`node scripts/pm/dispatch-gates.mjs
--commands`, merge base `96b12b589`) has 63 commands. All 63 exited 0,
and `--ran` reconciles 63 derived, 63 run, 0 NOT-MEASURED. On the
earlier head, `pnpm check:dual-build-cjs-loads` first answered
PREREQUISITE NOT MET (no `dist/`). It was run again after a full `turbo
run build` and answered 0, and it answered 0 again on this head.
- Lint, narrowed to the 5 touched `.ts` files: `eslint
--no-inline-config --format json` reads 5 files with 0 errors and 0
warnings. `eslint.config.mjs` enables no type-aware linting (no
`parserOptions.project`), so this diff cannot change the verdict on any
untouched file. The repo-wide `pnpm lint` is left to CI.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 9360df4 commit fbe2deb
6 files changed
Lines changed: 459 additions & 25 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 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
Lines changed: 367 additions & 0 deletions
| 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 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 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 | + | |
0 commit comments