Skip to content

Commit 6d4d5d3

Browse files
os-muskclaude
andauthored
fix(driver-sql): an all-NULL sum answers 0 on every face — fold at the aggregate door, conformance cell on every enrolled face (#15546) (#16720)
* fix(driver-sql): fold an all-NULL sum to 0 in the aggregate presentation, and pin it on every face SQL SUM skips NULLs and answers NULL once it has skipped every row of a group, so driver-sql answered null where the engine's in-memory tier, driver-memory and driver-mongodb answer 0 and emptyGroupValueFor rules 0. Measured null on better-sqlite3, live PostgreSQL 16.13 and live MySQL 8.0.46. Ruled option A on #15546 (maintainer, 2026-09-07): the SQL face moves. The fold reads the identity from emptyGroupValueFor at the aggregate door; the statement is unchanged. The conformance fixture gains a nullable numeric column, amount, NULL in every east row, and three cases pin the ruled answer on every enrolled face; every harness that seeds the rows declares the column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(conformance): keep a NULL aggregate answer as null in the SQL-family and in-memory harnesses Number(null) is 0 — the ruled answer for the all-null sum cell — so the unconditional Number(r.n) coercion made that cell green with the driver-sql fold ablated (measured 85/85 green against a driver answering null). The harnesses, not the faces, were holding the observable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * fix(driver-turso): the remote transport folds an all-NULL sum to 0 like the local face TursoDriver picks the remote compiler or SqlDriver's from url, so without this the same driver answered the all-NULL sum 0 locally and null remotely. Measured null on the enrolled remote face with a null-preserving harness. Same fold, same policy read (#15546). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ed7243d commit 6d4d5d3

9 files changed

Lines changed: 260 additions & 18 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/driver-turso": minor
4+
"@objectstack/spec": minor
5+
---
6+
7+
`SqlDriver.aggregate` answers `0` — not `null` — for a `sum` over a group whose aggregand is NULL in every row, matching the engine's in-memory aggregate tier and the identity `emptyGroupValueFor` already declares (#15546; maintainer ruling 2026-09-07, option A: a non-empty group whose aggregand is absent and an empty group are the SAME case for `sum`, and the SQL face is the one that moves).
8+
9+
SQL `SUM` skips NULLs and answers NULL once it has skipped everything, so on every dialect this driver targets (measured on better-sqlite3, live PostgreSQL 16.13 and live MySQL 8.0.46) a grouped list view with a `sum` summary on a nullable number or currency column rendered a BLANK total for a group whose column was empty in every row — while the same view on a deployment whose query took the engine's in-memory path rendered `0`. Which path answered was decided by a driver capability bit the caller never sees. The fold is part of the driver's aggregate presentation (`foldEmptyAggregateAnswers`): the compiled statement is unchanged (no `COALESCE`), the answer is the JS number `0` on every dialect, and `avg`/`min`/`max` — which have no identity over nothing — still answer `null`. The identity is read from `emptyGroupValueFor` rather than restated, so the two faces cannot drift apart on it again.
10+
11+
`@objectstack/driver-turso`: the REMOTE transport's `aggregate` carries the same fold (`RemoteTransport.foldEmptyAggregateAnswers`). `TursoDriver` picks the remote compiler or the local `SqlDriver` one from `url`, so without it the same driver would have answered the all-NULL `sum` as `0` locally and `null` remotely — one query, two answers, decided by a connection string, the seam the shared conformance table exists to close. Measured `null` on the enrolled remote face before the fold.
12+
13+
`@objectstack/spec`: the aggregate-vocabulary conformance fixture gains a NULLABLE numeric column. `AggregationRow.amount` (`number | null`) is NULL in every row of the `east` group and in two of the four `west` rows, and `AGGREGATION_CASES` gains the three cases that pin the ruled answer on every enrolled face — `sum(amount)` grouped by region (`east` 0 / `west` 40), its `count(amount)` reachability control (`east` 0 / `west` 2, which is what proves the nulls were stored as nulls), and the ungrouped partial-null control (40). A harness that runs the table MUST declare `amount` as a nullable numeric column and seed its nulls AS nulls, exactly as it already must for `stage`; a `0` written in place of a null turns the cell green for the wrong reason.

packages/drivers/driver-sql/src/sql-driver-11635-boolean-aggregand-answers.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ describe(`[#11635] driver-sql — boolean aggregands answer the ruled values (${
8787
region: { type: 'string' },
8888
stage: { type: 'string' },
8989
score: { type: 'number' },
90+
// [#15546] The fixture rows carry a nullable `amount` too — declared
91+
// so the verbatim seed below lands every column it carries.
92+
amount: { type: 'number' },
9093
flag: { type: 'boolean' },
9194
},
9295
},

packages/drivers/driver-sql/src/sql-driver-aggregation-conformance.test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ const CONFORMANCE_OBJECT = {
179179
// Nullable, and it must stay that way — see `AggregationRow.stage`.
180180
stage: { type: 'text', name: 'stage' },
181181
score: { type: 'number', name: 'score' },
182+
// [#15546] Nullable, and it must stay that way — see `AggregationRow.amount`:
183+
// the `east` group is NULL in every row, which is the cell the ruled
184+
// `sum` → `0` fold is pinned on. A `NOT NULL` column, or a `0` seeded in
185+
// place of a null, turns that cell green for the wrong reason.
186+
amount: { type: 'number', name: 'amount' },
182187
// [#11152] Declared `type: 'boolean'` on purpose — see `AggregationRow.flag`:
183188
// the ruled point of the boolean cases is that aggregation answers NUMBERS
184189
// (min=0/max=1) even where the declared type would present a row read as a
@@ -211,8 +216,14 @@ const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) =>
211216
// `groupByAlias ?? groupBy`. Reading `c.groupBy` unconditionally is the bug
212217
// this axis exists to catch: it is green on a face that ignores the alias.
213218
const groupKey = c.groupByAlias ?? c.groupBy;
219+
// [#15546] A NULL answer is kept as `null`, never coerced: `Number(null)` is
220+
// `0`, which is the ruled answer for the all-null `sum` cell — so the
221+
// unconditional `Number(r.n)` this read as before made that cell green with
222+
// the fold ABLATED (measured: 85/85 green against a driver answering
223+
// `null`). The harness, not the driver, was holding the observable. The
224+
// string-typed answers node-pg hands back (`"6"` for `COUNT`) still coerce.
214225
return rows
215-
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: Number(r.n) }))
226+
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: r.n === null ? null : Number(r.n) }))
216227
.sort((x, y) => String(x.group).localeCompare(String(y.group)));
217228
};
218229

@@ -247,13 +258,18 @@ describe(`[#6409] SqlDriver — aggregate vocabulary conformance (${cell.label})
247258
expect(rows.map((r: any) => String(r.id))).toEqual(['1', '2', '3', '4', '5', '6']);
248259
for (const r of rows as any[]) {
249260
const seeded = AGGREGATION_ROWS.find((s) => s.id === String(r.id))!;
250-
expect([r.region, r.stage ?? null, Number(r.score)], r.id)
251-
.toEqual([seeded.region, seeded.stage, seeded.score]);
261+
expect([r.region, r.stage ?? null, Number(r.score), r.amount ?? null], r.id)
262+
.toEqual([seeded.region, seeded.stage, seeded.score, seeded.amount]);
252263
}
253264
// The property the null cases hang off, asserted directly: an empty string
254265
// in place of a null would keep every `count_distinct` case green at the
255266
// wrong number.
256267
expect((rows as any[]).filter((r) => r.stage === null)).toHaveLength(2);
268+
// [#15546] The property the all-null `sum` cell hangs off: four NULL
269+
// amounts, both `east` rows among them. A `0` seeded in place of a null
270+
// answers the ruled `0` without the fold ever running.
271+
expect((rows as any[]).filter((r) => r.amount === null), 'null amounts').toHaveLength(4);
272+
expect((rows as any[]).filter((r) => r.region === 'east' && r.amount === null), 'east all-null').toHaveLength(2);
257273
// [#11152] The property the boolean cases hang off: 3 true / 3 false. A
258274
// seed that folded the flags turns every boolean case into a test of the
259275
// wrong table. `Boolean(...)` because the ROW read is presentation-shaped

packages/drivers/driver-sql/src/sql-driver.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readA
2020
// The DECLARED aggregate vocabulary (#5907). Read from the spec so this driver's
2121
// "the protocol has no such function" refusal cannot drift from what
2222
// `AggregationNodeSchema.function` actually admits.
23-
import { AggregationFunction } from '@objectstack/spec/data';
23+
import { AggregationFunction, emptyGroupValueFor } from '@objectstack/spec/data';
2424
import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
2525
// [#5659] The Filter Protocol's boolean identity reduction — `$and: []` is TRUE,
2626
// `$or: []` is FALSE, `{}` is a TRUE disjunct, `$not: {}` is FALSE. One
@@ -8569,6 +8569,12 @@ export class SqlDriver implements IDataDriver {
85698569
// See {@link presentReadColumns}.
85708570
const presentedOutput = new Map<string, ReadPresentationKind>();
85718571

8572+
// [#15546] Result columns whose NULL answer folds to the identity the
8573+
// platform declares for that aggregate over NOTHING (`emptyGroupValueFor`,
8574+
// spec `data/aggregation-policy.ts`), keyed like `presentedOutput` by the
8575+
// column name the caller will read. See {@link foldEmptyAggregateAnswers}.
8576+
const foldedOutput = new Map<string, number>();
8577+
85728578
if (query.groupBy) {
85738579
// groupBy items may be plain strings ('region') or structured objects
85748580
// ({ field: 'closed_at', dateGranularity: 'quarter' }). For structured
@@ -8698,6 +8704,12 @@ export class SqlDriver implements IDataDriver {
86988704
} else {
86998705
builder.select(this.knex.raw(`${rawFunc} as ${this.aliasIdentifierSql(agg.alias)}`, [fieldExpr]));
87008706
}
8707+
// [#15546] What this aggregate answers over NOTHING, read from the
8708+
// policy rather than restated: `sum` (and the two counts, which never
8709+
// arrive as NULL) fold to `0`; `avg`/`min`/`max` have no identity and
8710+
// their NULL passes through. See {@link foldEmptyAggregateAnswers}.
8711+
const identity = emptyGroupValueFor(funcName);
8712+
if (identity !== undefined) foldedOutput.set(agg.alias, identity);
87018713
// `min`/`max` are the only supported functions that hand back a value
87028714
// OF the column rather than a count/total derived from it, so they are
87038715
// the only ones whose result still needs the column's presentation.
@@ -8789,7 +8801,60 @@ export class SqlDriver implements IDataDriver {
87898801
// {@link SqlDriver.aggregateBackendFault}.
87908802
throw this.aggregateBackendFault(object, query, error);
87918803
}
8792-
return this.presentReadColumns(rows, presentedOutput);
8804+
return this.presentReadColumns(this.foldEmptyAggregateAnswers(rows, foldedOutput), presentedOutput);
8805+
}
8806+
8807+
/**
8808+
* [#15546] Fold the NULL a SQL aggregate answers over an all-NULL aggregand
8809+
* to the identity the platform declares for that aggregate over NOTHING.
8810+
*
8811+
* SQL `SUM` skips NULLs, and once it has skipped every row of a group it
8812+
* answers NULL — on every dialect this driver targets. Measured 2026-09-07
8813+
* on better-sqlite3, live PostgreSQL 16.13 and live MySQL 8.0.46: `sum` over
8814+
* a group of three rows whose column is NULL in each of them is `null` on
8815+
* all three, for `number` and `currency` columns alike. The engine's
8816+
* in-memory aggregate tier (`objectql`'s `in-memory-aggregation.ts`) answers
8817+
* `0` for the same rows, as do `driver-memory` and `driver-mongodb`'s
8818+
* lowering, and `emptyGroupValueFor` (spec `data/aggregation-policy.ts`)
8819+
* rules that summing nothing is `0` — a measured fact, not missing data.
8820+
* Which face answered was decided by a driver capability bit the caller
8821+
* never sees (`engine.ts`'s `typeof drv.aggregate === 'function'` fork), so
8822+
* one grouped list view rendered a blank total on one deployment and `0` on
8823+
* another. Maintainer ruling 2026-09-07 on #15546 (option A): three rows
8824+
* whose aggregand is absent and zero rows are the SAME case for `sum` — the
8825+
* addend set is empty either way — and this face is the one that moves.
8826+
*
8827+
* The identity is READ from the policy rather than restated here, so the
8828+
* other half of the same rule holds by construction: an aggregate whose
8829+
* `emptyGroupValueFor` is `undefined` (`avg`/`min`/`max`) has no answer over
8830+
* nothing, is never registered, and its NULL reaches the caller untouched.
8831+
* `count`/`count_distinct` register too but never arrive as NULL — `COUNT`
8832+
* answers `0` on its own — so the entry is inert for them, deliberately
8833+
* rather than special-cased away.
8834+
*
8835+
* Presentation, not compilation. The statement is unchanged — no `COALESCE`
8836+
* — so the emitted-SQL pins and the per-dialect result-type parsing above
8837+
* are untouched, and the answer is the JS number `0` on every dialect, the
8838+
* same value the in-memory tier produces. Only `null` folds: an `undefined`
8839+
* would mean the column was never projected, a different defect that must
8840+
* stay visible. Rows are mutated in place, as {@link presentReadColumns}
8841+
* does. The unaliased branch of {@link SqlDriver.aggregate} is not tracked,
8842+
* for the reason `presentedOutput` gives: `alias` is required by
8843+
* `AggregationNodeSchema`, and that branch lands under a dialect-dependent
8844+
* column name.
8845+
*
8846+
* Pinned on every enrolled face by the `sum(amount)` cases of
8847+
* `AGGREGATION_CASES` (spec `data/aggregation-conformance.ts`).
8848+
*/
8849+
protected foldEmptyAggregateAnswers(rows: any, identities: Map<string, number>): any {
8850+
if (identities.size === 0 || !Array.isArray(rows)) return rows;
8851+
for (const row of rows) {
8852+
if (!row || typeof row !== 'object') continue;
8853+
for (const [column, identity] of identities) {
8854+
if (row[column] === null) row[column] = identity;
8855+
}
8856+
}
8857+
return rows;
87938858
}
87948859

87958860
/**

packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-aggregation-conformance.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) =>
4848
// this axis exists to catch: it is green on a face that ignores the alias.
4949
const groupKey = c.groupByAlias ?? c.groupBy;
5050
return rows
51-
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: Number(r.n) }))
51+
// [#15546] A NULL answer stays `null` — `Number(null)` is `0`, the ruled
52+
// answer for the all-null `sum` cell, so coercing would let a face that
53+
// hands SQL's NULL through pass as if it had folded.
54+
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: r.n === null ? null : Number(r.n) }))
5255
.sort((x, y) => String(x.group).localeCompare(String(y.group)));
5356
};
5457

@@ -66,6 +69,10 @@ describe('[#6409] driver-sqlite-wasm — aggregate vocabulary conformance', () =
6669
// column nullable, which is what the null-bearing rows need.
6770
stage: { type: 'string' },
6871
score: { type: 'number' },
72+
// [#15546] Nullable, like `stage` — see `AggregationRow.amount`: the
73+
// `east` group is NULL in every row, the cell the ruled `sum` → `0`
74+
// answer is pinned on.
75+
amount: { type: 'number' },
6976
// [#11152] Declared `type: 'boolean'` on purpose — see
7077
// `AggregationRow.flag`: the ruled boolean cases answer NUMBERS
7178
// (min=0/max=1) over the 0/1 INTEGER storage.

packages/drivers/driver-turso/src/remote-transport.ts

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { resolveFilterSubtreeProvenance } from '@objectstack/spec/data';
3434
// The DECLARED aggregate vocabulary (#5907) — read from the spec so this
3535
// transport's "the protocol has no such function" refusal cannot drift from what
3636
// `AggregationNodeSchema.function` admits, nor from the local driver's twin.
37-
import { AggregationFunction } from '@objectstack/spec/data';
37+
import { AggregationFunction, emptyGroupValueFor } from '@objectstack/spec/data';
3838
import type { DriverQuery } from '@objectstack/spec/contracts';
3939
// [#8413] What a `unique: true` FIELD becomes, from the one place that decides
4040
// it. `uniqueIndexesFromFields`' own contract is that it is "the ONLY place
@@ -1369,6 +1369,10 @@ export class RemoteTransport {
13691369
this.assertSafeIdentifier(object);
13701370

13711371
const selectParts: string[] = [];
1372+
// [#15546] Result columns whose NULL answer folds to the identity the
1373+
// platform declares for that aggregate over NOTHING — the twin of
1374+
// `SqlDriver.aggregate`'s `foldedOutput`. See {@link foldEmptyAggregateAnswers}.
1375+
const foldedOutput = new Map<string, number>();
13721376

13731377
// [#6212] `groupBy` is `GroupByNode[]` — a UNION of a bare field name and a
13741378
// structured `{ field, dateGranularity?, alias? }` entry — so reading it as
@@ -1501,6 +1505,13 @@ export class RemoteTransport {
15011505
const alias = agg.alias || `${func}_${field === '*' ? 'all' : field}`;
15021506
const argSql = lowering.distinct ? `distinct ${fieldSql}` : fieldSql;
15031507
selectParts.push(`${lowering.sql}(${argSql}) AS ${this.aliasIdentifierSql(alias)}`);
1508+
// [#15546] What this aggregate answers over NOTHING, read from the policy
1509+
// rather than restated: `sum` (and the two counts, which never arrive as
1510+
// NULL) fold to `0`; `avg`/`min`/`max` have no identity and their NULL
1511+
// passes through. Keyed by the OUTPUT column — every aggregation here
1512+
// has one, defaulted or caller-supplied.
1513+
const identity = emptyGroupValueFor(func);
1514+
if (identity !== undefined) foldedOutput.set(alias, identity);
15041515
}
15051516

15061517
if (selectParts.length === 0) selectParts.push('*');
@@ -1524,7 +1535,7 @@ export class RemoteTransport {
15241535

15251536
try {
15261537
const result = await this.client!.execute({ sql, args });
1527-
return this.mapRows(result);
1538+
return this.foldEmptyAggregateAnswers(this.mapRows(result), foldedOutput);
15281539
} catch (error: any) {
15291540
if (
15301541
error.message &&
@@ -1537,6 +1548,38 @@ export class RemoteTransport {
15371548
}
15381549
}
15391550

1551+
/**
1552+
* [#15546] Fold the NULL SQL answers for an aggregate over an all-NULL
1553+
* aggregand to the identity the platform declares for that aggregate over
1554+
* NOTHING — the twin of `SqlDriver.foldEmptyAggregateAnswers`, and the reason
1555+
* it is here: `TursoDriver` picks this compiler or the local one from `url`,
1556+
* so without it the SAME driver answered `sum` over a group whose column is
1557+
* NULL in every row as `0` locally and `null` remotely — the #5907/#6203
1558+
* shape, one query, two answers, decided by a connection string. Measured
1559+
* on the enrolled remote face (libsql IS SQLite) before this fold: `null`.
1560+
*
1561+
* `emptyGroupValueFor` (spec `data/aggregation-policy.ts`) is READ rather
1562+
* than restated, so `avg`/`min`/`max` — no identity over nothing — are never
1563+
* registered and their NULL reaches the caller untouched; `count` and
1564+
* `count_distinct` register but never arrive as NULL. Presentation, not
1565+
* compilation: the statement is unchanged. Only `null` folds — an
1566+
* `undefined` would mean the column was never projected, a different defect
1567+
* that must stay visible. Rows are mutated in place, as `mapRows` builds
1568+
* them. Pinned by the `sum(amount)` cases of `AGGREGATION_CASES`.
1569+
*/
1570+
private foldEmptyAggregateAnswers(
1571+
rows: Record<string, unknown>[],
1572+
identities: Map<string, number>,
1573+
): Record<string, unknown>[] {
1574+
if (identities.size === 0) return rows;
1575+
for (const row of rows) {
1576+
for (const [column, identity] of identities) {
1577+
if (row[column] === null) row[column] = identity;
1578+
}
1579+
}
1580+
return rows;
1581+
}
1582+
15401583
async create(object: string, data: Record<string, unknown>): Promise<Record<string, unknown>> {
15411584
await this.ensureConnected();
15421585

packages/drivers/driver-turso/src/turso-remote-aggregation-conformance.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ const CONFORMANCE_OBJECT = {
7373
// Nullable, and it must stay that way — see `AggregationRow.stage`.
7474
stage: { type: 'string' },
7575
score: { type: 'number' },
76+
// [#15546] Nullable, like `stage` — see `AggregationRow.amount`: the `east`
77+
// group is NULL in every row, the cell the ruled `sum` → `0` answer is
78+
// pinned on.
79+
amount: { type: 'number' },
7680
// [#11152] Declared `type: 'boolean'` on purpose — see `AggregationRow.flag`.
7781
// SQLite stores it 0/1 INTEGER, and the ruled boolean cases (min=0/max=1,
7882
// sum=3, avg=0.5) are answered in exactly that numeric domain.
@@ -97,8 +101,12 @@ const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) =>
97101
// `groupByAlias ?? groupBy`. Reading `c.groupBy` unconditionally is the bug
98102
// this axis exists to catch: it is green on a face that ignores the alias.
99103
const groupKey = c.groupByAlias ?? c.groupBy;
104+
// [#15546] A NULL answer is kept as `null`, never coerced: `Number(null)` is
105+
// `0`, the ruled answer for the all-null `sum` cell, so an unconditional
106+
// `Number(r.n)` reads a face that hands SQL's NULL through as if it had
107+
// folded — the harness holding the observable instead of the face.
100108
return rows
101-
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: Number(r.n) }))
109+
.map((r) => ({ group: groupKey ? String(r[groupKey]) : null, value: r.n === null ? null : Number(r.n) }))
102110
.sort((x, y) => String(x.group).localeCompare(String(y.group)));
103111
};
104112

0 commit comments

Comments
 (0)