Repository navigation
Commit d98bf24
fix(objectql)!: engine aggregate judges the sum row too — sum over a refused type answers INVALID_FIELD / 400 on every driver (#21103)
Fixes #20914
Clause-②: no (narrowing)
The follow-up round of this card, after PR #21037 landed as `a75311dd2`
(`Part of`, the `sum` row held). It executes triage's answer 5924445782
(A), whose execution line reads: "`'sum'` leaves `ROWS_HELD_FOR_TRIAGE`,
and the held pins flip. If the constant is then empty, delete it." With
this PR the engine's aggregate door asks every row of
`AGGREGATE_FIELD_TYPE_COMPATIBILITY`, so the card's direction (the whole
table) is complete.
## What this changes
- `packages/objectql/src/aggregate-field-type-door.ts`: `'sum'` leaves
`ROWS_HELD_FOR_TRIAGE`. The set is then empty, so the constant is
deleted with its one read. `assertAggregationFieldTypesAccepted` now
judges all six rows (`count` refuses no type). ⛔ No second table and no
per-pair exemption: the door still carries no accept list of its own. In
the module header, the section "The row the census held back" becomes
"Every row is judged — `sum` included" and records the release. It also
gains the measured `sum` table, and "a held row" leaves the not-judged
list.
- The refusal words are the door's existing machinery
(`FUNCTION_WORDS.sum`: "sums" / "does not sum"). The route is read off
the table's `sum` row: `sum accepts a field of type number, currency,
rating, slider, progress, summary, boolean or toggle: aggregate a field
of one of those types, or count the rows with count.` It sits inside the
500 characters the REST door keeps.
- `packages/spec/src/data/aggregate-field-type-compatibility.ts`, TSDoc
only. Review 5924038187 ① 7 named three published sentences; each now
states the rows the engine door judges:
- the module header: "on every row: `count_distinct` since #20808 …, and
`sum`, `avg`, `min` and `max` since #20914 (`count` refuses no type, so
its row never refuses there)". The pointer to an objectql source file a
spec consumer cannot see is gone.
- the predicate's TSDoc: "on every row of the table".
- the "does NOT do" section: "at query time, on every row too".
- `radio` joins the multi-capable list in the JSON-stored paragraph
(`MULTI_CAPABLE_TYPES` holds it).
- A comment-stripped compare (the TypeScript printer with
`removeComments`) of the file at base and head is byte-identical, sha256
`22d8df93…` on both sides: ⛔ the table and the predicate did not move.
The new clause is in the built `dist/data/index.d.ts`, and "other rows
since #20914" occurs in 0 built `.d.ts`.
- `.changeset/20914-release-sum-row.md`:
- `@objectstack/objectql: minor` with a BREAKING banner and `Clause-②:
no (narrowing)`. It carries the FROM → TO per type class, what to write
instead, and who is affected.
- Exactly one ADR-0087 marker: `not-required (already-registered
dataset-measure-aggregate-field-type-refused)`. That registered id's
prescription already covers `sum` / `avg` over every class the table
refuses; the selecting id covers only `min` / `max`.
- It names the earlier entry's `radio` omission (review ① 3) in its own
text, and that entry's "Not judged yet: `sum`" paragraph as superseded.
⛔ The landed file is not edited.
- `@objectstack/spec: patch` for the TSDoc.
## FROM → TO, measured on three drivers (H2)
Through `engine.aggregate`, two rows, on the real `InMemoryDriver`, on
`SqlDriver` over SQLite, and on a private PostgreSQL 16.14. Before =
base `2821e9f15`; after = this branch's built `dist`.
| `sum` over | before: memory / SQLite / PostgreSQL | after, all three |
|:--|:--|:--|
| `json` | `0` / `0` / **500** `function sum(json) does not exist` | 400
`INVALID_FIELD` |
| `text` | `0` / `0` / **500** `function sum(text) does not exist` | 400
`INVALID_FIELD` |
| single-value `select` | `0` / `0` / **500** `function sum(character
varying) does not exist` | 400 `INVALID_FIELD` |
| `formula` | `0` / 400 `INVALID_FIELD` (driver: no column) / 400
(driver: no column) | 400 `INVALID_FIELD`, the engine's words |
| `tags` | `0` / `0` / **500** | 400 `INVALID_FIELD` |
| `datetime` | `0` / `4052` (the years added) / **500** | 400
`INVALID_FIELD` |
| `percent` (refused by the table: a rate does not add) | `30` / `30` /
`30` | 400 `INVALID_FIELD` |
| controls: `currency`, `number`, `boolean`; `max` `number`; `count`
`json` | `30`, `3`, `1`, `2`, `2` on all three | unchanged |
The `percent` row is the one whose old answer agreed across drivers. The
table refuses it on the semantic ground its TSDoc names
(`isIncoherentAggregate`), and it is in the changeset's FROM → TO with
its route (`avg`).
## The census, re-run before the flip (H1)
Query: every line carrying a `'sum'` / `"sum"` literal, plus the
unquoted and backtick spellings. Each hit was resolved by hand to its
object and the field's declared type, then asked of the table.
- **`examples/**` at `2821e9f15`: 17 lines, 0 refused pairs.** They are
dataset measures, roll-up `summaryOperations`, a cube measure, a `kpi`
metric and an `ObjectChart` aggregate. Every operand is a `currency`,
`number` or `summary` field (11 distinct object.field pairs).
- **hotcrm (`objectstack-ai/hotcrm` cloned at its head `fb408a73`): 33
lines in `src`.** One is a comment (a `derived` op), which leaves 32
authored `sum` aggregations over 18 distinct object.field pairs. 31 are
over `currency` or `number`. **1 refused pair:**
`src/sales/views/forecast.view.ts:28`, `crm_forecast.expected_amount`
(`Field.formula`), the list-column `summary: 'sum'`. That is the known
hit: triage-answered and carried by objectstack-ai/hotcrm#1980, which
has not changed it yet. **No other hit, so the flip stands.** Outside
`src`: 2 lines in `test`, and 3 in docs and the changelog (prose).
- The query is its own positive control: it finds the known hotcrm hit.
## Pins: the held `sum` pins flipped, ⛔ not deleted
- `packages/objectql/src/engine-aggregate-field-type-door.test.ts`:
- The CONTROL's two held cases (`sum meta`, `sum title`, which reached
the driver while the row was held) move into a new refusal test. It
covers `sum` over `json`, `text`, `select`, `formula`, `tags`,
`percent`, `datetime` and a `select` with `multiple: true`, with
envelope `INVALID_FIELD` / 400 / `httpStatus` 400, `field` / `fields` /
`object` / `param`, the message head and the route, and no read.
- The CONTROL gains `sum` over a `currency` and a `boolean`.
- The GUARD that asserted "no verdict for `sum` × every type" now walks
every row read off the table, so a row the door skipped would turn it
red. It asserts the key set is the six functions, with a `sum` floor
beside the others.
- The fail-closed GUARD covers `sum` too.
- `packages/rest/src/data-aggregate-field-type-door.test.ts`: a `sum`
refusal cell over `json`, `text`, `select`, `tags` and `formula` at
`POST /api/v1/data/:object/query` (SQLite always; PostgreSQL where
`OS_TEST_POSTGRES_URL` is set). The CONTROL gains `sum` over a
`currency` (60) and a `boolean` (2). 1 parent 32d3b3c commit d98bf24
7 files changed
Lines changed: 183 additions & 62 deletions
File tree
- .changeset
- packages
- objectql/src
- rest/src
- spec/src/data
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
39 | 49 | | |
40 | 50 | | |
41 | 51 | | |
| |||
46 | 56 | | |
47 | 57 | | |
48 | 58 | | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
57 | 67 | | |
58 | | - | |
| 68 | + | |
59 | 69 | | |
60 | 70 | | |
61 | 71 | | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
68 | 81 | | |
69 | 82 | | |
70 | 83 | | |
| |||
79 | 92 | | |
80 | 93 | | |
81 | 94 | | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
88 | 100 | | |
89 | 101 | | |
90 | 102 | | |
| |||
107 | 119 | | |
108 | 120 | | |
109 | 121 | | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | | - | |
116 | | - | |
117 | 122 | | |
118 | 123 | | |
119 | 124 | | |
| |||
152 | 157 | | |
153 | 158 | | |
154 | 159 | | |
155 | | - | |
156 | 160 | | |
157 | 161 | | |
158 | 162 | | |
| |||
0 commit comments