|
| 1 | +--- |
| 2 | +'@objectstack/spec': major |
| 3 | +'@objectstack/objectql': major |
| 4 | +--- |
| 5 | + |
| 6 | +refactor(spec,objectql)!: retire `AggregationNode.distinct` — one face honoured it, five ignored it, and the same query answered two plausible numbers (#6815, ADR-0049) |
| 7 | + |
| 8 | +<!-- adr-0087: registered aggregation-node-distinct-retired --> |
| 9 | + |
| 10 | +**FROM → TO:** `{ function: 'count', field: 'x', distinct: true, alias: 'a' }` → |
| 11 | +`{ function: 'count_distinct', field: 'x', alias: 'a' }` — the deduplicating spelling |
| 12 | +every backend computes, lowered to `COUNT(DISTINCT x)` on both SQL faces since #6409. |
| 13 | +`{ function: 'sum' | 'avg' | 'min' | 'max', …, distinct: true }` → delete the key; there is |
| 14 | +no replacement, because no SQL backend ever computed `SUM(DISTINCT …)` here and the |
| 15 | +in-memory fallback was the only thing that did. `distinct: false` → delete the key; it |
| 16 | +selected the behaviour that is now the only behaviour. |
| 17 | + |
| 18 | +`AggregationNode.distinct` was read by exactly ONE of the six faces that consume an |
| 19 | +`aggregations[]` entry. `objectql`'s in-memory fallback (`in-memory-aggregation.ts`) |
| 20 | +deduplicated the values before applying the function; `SqlDriver.aggregate`, the Turso |
| 21 | +`RemoteTransport.aggregate`, `driver-mongodb`'s `buildAggregationStage`, `driver-memory`'s |
| 22 | +`computeAggregate` and `service-analytics`' `AGGREGATE_SQL` all ignored it. So |
| 23 | +`{ function: 'sum', field: 'amount', distinct: true }` returned a deduplicated sum when the |
| 24 | +engine fell back in memory and an ordinary sum on every SQL datasource — one query, two |
| 25 | +numbers, chosen by which backend answered. The engine picks that path per query (a driver |
| 26 | +without native aggregation, a non-UTC date bucket, a partial SQL driver), so the number |
| 27 | +could move under a dashboard with nothing changing in the query. |
| 28 | + |
| 29 | +That is the divergence class #6203 and #5907 each closed on the aggregate axis, still open |
| 30 | +on this key, and it is worse to leave: both answers are plausible NUMBERS rather than a |
| 31 | +refusal, so nothing surfaced it. It survived the #4286 sweep of this same schema because |
| 32 | +that sweep asked which members no executor reads — the wrong question for a key whose |
| 33 | +defect is *which* executor reads it. |
| 34 | + |
| 35 | +REMOVE rather than ENFORCE, per the maintainer ruling of 2026-08-09: `count_distinct` |
| 36 | +already covers the only deduplicating spelling with measured demand and took ADR-0049's |
| 37 | +enforce leg in #6409, while `SUM(DISTINCT …)` / `AVG(DISTINCT …)` are near-universally a |
| 38 | +modelling mistake and would have to be lowered across five faces — two of them frozen under |
| 39 | +#5499 — to buy it. |
| 40 | + |
| 41 | +The retirement kit: |
| 42 | + |
| 43 | +- **Tombstone, not deletion** (`retiredKey()`): `AggregationNodeSchema` is not `.strict()`, |
| 44 | + so a plain delete would let existing queries parse clean and lose the key in silence |
| 45 | + (#3733, ADR-0104) — trading a divergent flag for an ignored one. Authoring it is now a |
| 46 | + `tsc` error at the call site and a parse error carrying the prescription. One tombstone |
| 47 | + covers every aggregation door: `QuerySchema.aggregations` and |
| 48 | + `EngineAggregateOptionsSchema.aggregations` both reuse that one schema by reference. |
| 49 | +- **ADR-0087 D3 `SemanticMigration`** (`aggregation-node-distinct-retired`) plus the exact |
| 50 | + `RETIRED_KEYS_BY_MAJOR[17]` entry `data/AggregationNode:distinct`. No D2 conversion, |
| 51 | + deliberately: `QueryAST` is a request surface — the client SDK builder's output and the |
| 52 | + `POST /data/:object/query` body — never stored in stack metadata, so there is no source |
| 53 | + for `os migrate meta` to rewrite. That is the disposition every other `data.query.*` |
| 54 | + retirement in this major already takes (#4286). |
| 55 | +- `objectql`'s in-memory fallback loses its `collectValues` dedupe limb — the whole runtime |
| 56 | + cost of the removal. **The observable numbers change on that one path, and that is the |
| 57 | + point:** a `sum`/`avg` that used to be deduplicated there now answers what every SQL face |
| 58 | + has always answered for the same query. Verify against the SQL answer, not against the |
| 59 | + pre-upgrade fallback answer — the two disagreed. |
| 60 | +- Measured blast radius inside the fallback, narrower than the key suggests: only `sum` and |
| 61 | + `avg` ever changed answer. `count` returned from its own branch before reaching the |
| 62 | + dedupe, `count_distinct` fed the values into a `Set` (dedupe-then-`Set` is `Set`), and |
| 63 | + dedupe does not move `min`/`max`. |
| 64 | +- `POST /api/v1/data/:object/query` answers `400 VALIDATION_FAILED` with a `fields[]` entry |
| 65 | + at `aggregations.<i>.distinct` instead of serving a number — the #3899 entry validation |
| 66 | + descending into the array, pinned in the REST request-schema conformance gate. |
| 67 | +- Liveness ledger (`query.json` `aggregations.children.distinct` → `dead`, README counts), |
| 68 | + generated baselines (`authorable-surface/data.json` gains `[RETIRED]`), |
| 69 | + `spec-changes.json`, the upgrade guide and the reference docs regenerated. |
| 70 | + |
| 71 | +`count_distinct` is untouched and remains the live deduplicating spelling. |
0 commit comments