Repository navigation
Commit 15bf186
fix(driver-sql): aggregate count / count_distinct / sum / avg answer numbers on PostgreSQL and MySQL (#20372)
Fixes #20335
Clause-②: no
## What changed
`SqlDriver.aggregate` handed the SQL client's answer straight through.
node-postgres parses `bigint` (OID 20) and `numeric` (OID 1700) to
strings, and mysql2 does the same for `DECIMAL`. So `count` /
`count_distinct` / `sum` / `avg` reached the engine's `having` and the
REST response as strings on the native path of PostgreSQL (all four) and
MySQL (`sum` / `avg`), while SQLite and the rows path of every dialect
answered numbers.
The fix is one presentation step at the end of the native path, in
`packages/drivers/driver-sql/src/sql-driver.ts`:
- `AGGREGATE_ANSWER_KIND` is a `Record` over the spec's
`AggregationFunction`. `count`, `count_distinct`, `sum` and `avg` are
`'number'`; `min` and `max` are `'column'`. A function added to the enum
without an entry fails `tsc`.
- In `aggregate()`, an aliased aggregation whose function is `'number'`
registers its alias with the existing `'number'` presenter
(`presentReadValue`, the one `formatOutput` applies to a numeric field
on a `find()` row). `min` / `max` keep the column's own presentation,
unchanged.
- It is keyed on the function the query asked for, never on what a value
looks like. It is not gated by dialect: the presenter rewrites only a
string, so SQLite's answers (and mysql2's `COUNT`) pass through
untouched.
- No connection-level type parser is touched. `find()`, `distinct()` and
the `where` comparand path are unchanged. `packages/objectql` is
untouched.
## Precision policy (H3)
The answer is one JS number, an IEEE-754 double, on every dialect. A
`sum` / `avg` whose exact value needs more than a double's 15 to 17
significant digits, or an integer total at or above 2^53, is rounded to
the nearest double. The policy is stated on `AGGREGATE_ANSWER_KIND` and
in the changeset.
Options weighed:
- A. A JS number, with the loss beyond double precision undeclared.
Rejected, because the loss must be written down.
- B. A string only when the value is unsafe as a double, a number
otherwise. Rejected. The value's type would depend on its size, so the
defect would come back for large totals only: a `having` `$in`, or a
chart reading the column, would silently stop matching for exactly those
groups. That is also the harder shape for an AI author to get right.
- C. **Chosen:** always a number, with the loss declared. It is the same
bound `find()` already puts on a read of the same exact-decimal column,
since `formatOutput`'s numeric pass (`valueSchemaFor` gives the numeric
class `z.number().finite()`, ADR-0104 D1). It is also the bound the rows
path has always had: `in-memory-aggregation.ts` sums JS doubles.
What the spec says an aggregate's value type is:
`packages/spec/src/data/aggregation-conformance.ts` types
`AggregationExpectation.value` as `number` and states it "stays a
`number` for every case", across every enrolled face.
`service-analytics`' `measure-result-type.ts` publishes `number` as the
result type of `count` / `count_distinct` / `sum` / `avg` measures.
Before this change, PostgreSQL delivered strings under that declaration.
## Measured (H1, H2)
Base `26daf0b036` and head `f3b9e1390c`, on SQLite (better-sqlite3),
live PostgreSQL 16.13 (server zone Asia/Shanghai) and live MySQL 8.0.46
(`+08:00`). Each cell went through `SqlDriver.aggregate`,
`engine.aggregate` and `POST /api/v1/data/:object/query` (JSON
round-trip). The fixture is `groupBy` customer, four groups, over
`number` fields (one integer-valued, one decimal-valued), `currency`,
`percent` and `rating`. The three native doors answered identically at
base and at head, on every dialect.
| face | base | head |
|:--|:--|:--|
| PostgreSQL native: `count`, `count_distinct` | string `"2"` | number
`2` |
| PostgreSQL native: `sum` / `avg` over number, currency, percent |
string `"500.000…"` (30-digit scale) | number `500` |
| PostgreSQL native: `sum` / `avg` over rating (int4) | string `"7"` /
`"3.5000000000000000"` | number `7` / `3.5` |
| MySQL native: `count`, `count_distinct` | number | number, unchanged |
| MySQL native: `sum` / `avg` over number, currency, percent, rating |
string (`DECIMAL`) | number |
| `min` / `max` over any of the five, native, PG and MySQL | number |
number, unchanged |
| SQLite native, every cell | number | byte-identical |
| rows path, every dialect, every cell | number | byte-identical |
H2: a raw query through `pg` answers field OIDs `n:20 total:1700
mean:1700 mn:1700 mx:1700 ss:20 sa:1700 smin:23`. Every value is a
string except `smin` (int4). The same query through `mysql2` answers
column types `n:8` (LONGLONG, a number), `246` (NEWDECIMAL, a string)
for `sum` / `avg` / `min` / `max` of the decimal column and for `sum` /
`avg` of the int column, and `3` for `min` of the int column. So `min` /
`max` over a `numeric` column is a string at the client too. It already
left the driver as a number, because a declared numeric field takes the
`'number'` column presentation since the numeric-representation change.
The rows path yields numbers because `find()` presents the numeric
column as a number and `in-memory-aggregation.ts` computes `count` as
`rows.length` and `sum` / `avg` in JS arithmetic.
The card's `having` table, engine and REST doors (`having` is evaluated
by the engine on both paths):
| `having` | base: PG native | base: MySQL native | head: every dialect
× path |
|:--|:--|:--|:--|
| `{ n: { $in: [2] } }` | no group | c1, c2 | c1, c2 |
| `{ total: { $in: [500, 20] } }` | no group | no group | c1, c4 |
| `{ mean: { $in: [250, 600] } }` | no group | no group | c1, c2 |
| `{ avg_rate: { $in: [0.375] } }` | no group | no group | c1 |
| `{ n: { $lt: 'not-a-date' } }` | c1–c4 | no group | no group |
| `{ total: { $lt: 'not-a-date' } }` | c1–c4 | c1–c4 | no group |
| `{ n: { $gt: '+010000-01-01T00:00:00.000Z' } }` | c1–c4 | no group |
no group |
| `$eq` on count / sum / avg, `$gt` / `$gte` numbers | as elsewhere | as
elsewhere | unchanged |
At base, SQLite (both paths) and the rows path of PostgreSQL and MySQL
already answered the head column.
## Collateral (H4)
- `find()` and `distinct()` over the five numeric columns are
byte-identical base to head, on all three dialects (compared as typed
JSON).
- SQLite: every aggregate cell is byte-identical base to head, on every
door.
- No connection-level parser changed. The temporal `min` / `max`
presentation is untouched: the `'column'` arm is the pre-existing
branch. `sql-driver-aggregate-temporal-output.test.ts` and
`sql-driver-13973-canonical-iso-read-door.test.ts` pass in the live
suite below.
## Consumers (H5)
These read the changed values and now receive a number:
- `@objectstack/objectql` `aggregateSummaryValue`: roll-up summaries
write the value into the parent's `summary` field. That value is now a
number where PostgreSQL (`count`, `sum`, `avg`) and MySQL (`sum`, `avg`)
handed back a numeric string. `summary-backfill.ts` counts `nonEmpty`
for a `count` / `sum` roll-up only when `typeof computed === 'number'`,
so by reading, its report under-counted those roll-ups on PostgreSQL
(`count`, `sum`) and MySQL (`sum`) before; it counts them now.
- `service-analytics` `ObjectQLStrategy`: passes values through into
responses that declare `number`, and now delivers one.
`cross-object-rebucket.ts` and `dataset-executor.ts` coerce with
`Number(...)`, which is the identity on a number.
- `metadata-protocol` (the REST query route), `runtime`
`action-execution` `aggregate`, `mcp` `stdio-data-bridge`, and the
client SDK: pass-through, no coercion.
- objectui, read by code search: `ObjectChart` `readValue`,
`ObjectMetricWidget` and `MetricWidget` read these values through
`Number(...)`, which is the identity on a number.
None of the consumers above compares these values as strings or branches
on `typeof value === 'string'`; the one `typeof` test
(`summary-backfill.ts`) asks for `'number'`.
## Tests
- New:
`packages/drivers/driver-sql/src/sql-driver-20335-aggregate-numeric-presentation.test.ts`.
Seven cases per dialect cell of the live matrix (`declareDialectCell`,
so the PostgreSQL and MySQL cells run in `Temporal Conformance (live PG
+ MySQL)`):
- `count` / `count_distinct` / `sum` / `avg` over number, currency,
percent and rating, and `sum` / `avg` over a boolean, asserted with
`toBe` against the rows' own JS arithmetic;
- the all-NULL fold;
- `min` / `max` presentation unchanged;
- the precision policy: SQL-written `9007199254740993` and
`12345678901234567.123456789` answer `Number(literal)`, never a string,
and equal what `find()` reads for the same row.
- New: `packages/rest/src/rest-aggregate-numeric-having.test.ts`. Engine
and REST doors, native and rows paths: `having` `$in` / `$eq` on `count`
/ `count_distinct` / `sum` / `avg`, the string-comparand rows of the
card, and the response's JSON numbers. The SQLite cell always runs. The
PostgreSQL and MySQL cells run when `OS_TEST_POSTGRES_URL` /
`OS_TEST_MYSQL_URL` are set and are otherwise a named skip.
- Reverse verification: the fix was committed first, then the one
`presentedOutput.set(agg.alias, 'number')` line was deleted through
`scripts/ablation-replace.mjs` (anchor 1 → 0, blob `8dc157d535` →
`c247bcff26`). driver-sql was rebuilt, and `ablation-dist-preflight
--absent` confirmed the marker absent from all six built files. Result:
- driver-sql test: 7 failed, 14 passed. The failures are every PG and
MySQL string cell (for example `c1 count(*): expected '2' to be 2`).
Every SQLite case and MySQL `count` stayed green.
- REST test: 13 failed, 23 passed. The failures are the PG and MySQL
cells the base table marks. SQLite stayed green.
- The ablated native answers were byte-identical to the base on all
three dialects.
- Restore: blob equals HEAD, `git diff HEAD` empty. After a rebuild the
marker is present in 2 built files, and `git status --porcelain` is
empty.
- Whole suites, at `06349dc973` (identical code to `f3b9e1390c`, which
only corrects a docblock). PostgreSQL and MySQL were live, with
`TZ=America/New_York` and `OS_EXPECT_LIVE_DIALECT_MATRIX=1`.
- `@objectstack/driver-sql`: 204 files passed, 4690 tests passed, 1
skipped. The reporter confirmed that all 3 dialects were exercised.
- `@objectstack/rest` `local` project, with no live URL, as in CI: 210
files passed, 3817 tests passed, 26 skipped (24 of them this file's
PostgreSQL and MySQL cells).
- `@objectstack/objectql` aggregate, `having`, in-memory aggregation and
summary files: 19 files, 612 tests passed.
- Typecheck: `@objectstack/driver-sql` exit 0 and `@objectstack/rest`
exit 0 (including `check:test-typecheck`). Both new files are in a
typecheck program (`--listFiles`).
## Gates
At head `f3b9e1390c`, after a full workspace build (`turbo run build`
over `./packages/*` and `./packages/*/*`, 71 of 71 tasks):
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 63 commands, the same 63 as the
dispatch list. All 63 ran and exited 0. `--ran` reconciliation: 63
derived, 63 run, 0 NOT-MEASURED, 0 UNRUN.
- `origin/main` moved during the run, so the three `--base` gates
(`check-adr-0087-registration`, `check-changeset-no-major`,
`check-empty-changeset`) and `check-issue-citations` were also run with
`--base 26daf0b` (the merge base). Each exited 0.
`check-changeset-no-major` reads the level from the PR, so its local run
has no PR payload to judge.
- Lint, a proved narrowing: `eslint --no-inline-config --format json`
over the three touched TypeScript files reports 3 files, 0 errors, 0
warnings. `eslint --print-config` resolves a config for each, so none is
ignored. The config enables no type-aware linting (`parserOptions` is
`ecmaVersion` / `sourceType` only, with no `project`), so this diff
cannot move the verdict on an untouched file. The repo-wide `pnpm lint`
is CI's.
- NOT MEASURED locally, and owned by CI: the type-check lanes over the
whole workspace, `Test Core` shards, `Dogfood`, `Build Core`, and
`Temporal Conformance` as CI spells it. The driver-sql suite was run
locally against live PostgreSQL and MySQL as above.
## Acceptance notes
- The rows path and the exact-decimal native path can differ in the last
place of a double. A `number` column holding 0.1 and 0.2 sums to `0.3`
on PostgreSQL and MySQL native (exact `numeric` / `DECIMAL` arithmetic)
and to `0.30000000000000004` on the rows path and on SQLite (double
arithmetic). So `having { s: { $eq: 0.3 } }` keeps that group on PG /
MySQL native and on no other face. Measured identical at base and at
head: this PR moves the type, not the arithmetic. Reported to the seat
as a separate finding.
- `readPresentationKind`'s `'number'` kind (used by `min` / `max` and
`distinct()`) reads `numericFields`, which includes the driver-internal
`integer` / `int` / `float` aliases, on every dialect. `formatOutput`
narrows its row pass to `numericValueFields` on PostgreSQL and MySQL, to
keep an introspected external `bigint` above 2^53 as a string. Not
measured, not changed here.
- The REST test's PostgreSQL and MySQL cells are provisioned by no CI
job today; the live servers are attached to `driver-sql`'s suite. CI
runs their SQLite cell. The PostgreSQL and MySQL value pins run in CI
through the driver-sql file. Carrier: none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 17e4f52 commit 15bf186
4 files changed
Lines changed: 529 additions & 5 deletions
File tree
- .changeset
- packages
- drivers/driver-sql/src
- rest/src
| 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 | + | |
Lines changed: 208 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1476 | 1476 | | |
1477 | 1477 | | |
1478 | 1478 | | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
| 1487 | + | |
| 1488 | + | |
| 1489 | + | |
| 1490 | + | |
| 1491 | + | |
| 1492 | + | |
| 1493 | + | |
| 1494 | + | |
| 1495 | + | |
| 1496 | + | |
| 1497 | + | |
| 1498 | + | |
| 1499 | + | |
| 1500 | + | |
| 1501 | + | |
| 1502 | + | |
| 1503 | + | |
| 1504 | + | |
| 1505 | + | |
| 1506 | + | |
| 1507 | + | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
| 1512 | + | |
| 1513 | + | |
| 1514 | + | |
| 1515 | + | |
| 1516 | + | |
| 1517 | + | |
| 1518 | + | |
| 1519 | + | |
| 1520 | + | |
| 1521 | + | |
| 1522 | + | |
| 1523 | + | |
| 1524 | + | |
| 1525 | + | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
| 1531 | + | |
| 1532 | + | |
| 1533 | + | |
| 1534 | + | |
| 1535 | + | |
| 1536 | + | |
1479 | 1537 | | |
1480 | 1538 | | |
1481 | 1539 | | |
| |||
9858 | 9916 | | |
9859 | 9917 | | |
9860 | 9918 | | |
9861 | | - | |
9862 | | - | |
| 9919 | + | |
| 9920 | + | |
| 9921 | + | |
| 9922 | + | |
9863 | 9923 | | |
9864 | 9924 | | |
9865 | 9925 | | |
| |||
10009 | 10069 | | |
10010 | 10070 | | |
10011 | 10071 | | |
| 10072 | + | |
| 10073 | + | |
| 10074 | + | |
| 10075 | + | |
| 10076 | + | |
| 10077 | + | |
| 10078 | + | |
| 10079 | + | |
| 10080 | + | |
| 10081 | + | |
| 10082 | + | |
10012 | 10083 | | |
10013 | 10084 | | |
10014 | 10085 | | |
10015 | 10086 | | |
10016 | 10087 | | |
10017 | 10088 | | |
10018 | 10089 | | |
10019 | | - | |
| 10090 | + | |
10020 | 10091 | | |
10021 | 10092 | | |
10022 | 10093 | | |
| |||
15128 | 15199 | | |
15129 | 15200 | | |
15130 | 15201 | | |
15131 | | - | |
| 15202 | + | |
| 15203 | + | |
| 15204 | + | |
15132 | 15205 | | |
15133 | 15206 | | |
15134 | 15207 | | |
| |||
15196 | 15269 | | |
15197 | 15270 | | |
15198 | 15271 | | |
15199 | | - | |
| 15272 | + | |
| 15273 | + | |
| 15274 | + | |
15200 | 15275 | | |
15201 | 15276 | | |
15202 | 15277 | | |
| |||
0 commit comments