fix(objectql): the rows path adds sum / avg with compensated summation, as SQLite does - #20543
Conversation
…mpensation, as SQLite does The engine's rows path (in-memory-aggregation.ts) folded sum and avg naively, while SQLite 3.43+ compensates, so one query answered two doubles on SQLite depending on the path engine.aggregate took (0.1 + 0.2 + 0.3: native 0.6, rows 0.6000000000000001). Both arms now add through one compensated fold transcribed from SQLite's kahanBabuskaNeumaierStep and its finalizers' overflow guard. Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude <noreply@anthropic.com>
… residual it leaves Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check2 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1b2e388b6b50f2fc4e8f967879943a2ddcac84ec && git checkout 1b2e388b6b50f2fc4e8f967879943a2ddcac84ec
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ba5927f714af7516105706b36a05cedf34d5fa1b 1c476788382b5de4e5aec4c5cebb71fe14452f53 && git checkout -B drift-repro ba5927f714af7516105706b36a05cedf34d5fa1b && git merge --no-ff 1c476788382b5de4e5aec4c5cebb71fe14452f53
node scripts/docs-audit/affected-docs.mjs --json ba5927f714af7516105706b36a05cedf34d5fa1b |
Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN Co-authored-by: Claude <noreply@anthropic.com>
…te and its comments
The rows path now adds with compensated summation, so the residual the
pending driver-sql double-accumulation note, the AGGREGATE_ACCUMULATION
comment and the driver-sql test header stated ("SQLite's native face
alone", "every other face 0.6000000000000001") no longer held. Each now
states it as PostgreSQL / MySQL native against SQLite and the rows path.
Text only; no logic moves.
Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN
Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Inputs read: card #20489 (body; comments 5875711059 triage, 5881410875 claim, 5881969729 os-dev-report round 1, 5881988201 seat answer, 5882195153 os-dev-report patch 1), PR #20543 (body, 7-file list, net diff against merge-base ① Derived judgments1. 2. The pins: cover what the card and the brief ask, and weaken nothing. The objectql unit file (11 cases): the card's fixture with the discriminating 3. The DELIBERATE CORRECTION of 4. The driver-sql edits: comment-only, and each now TRUE. Re-derived from the net diff: filtering every changed line under 5. The new driver-memory split: stated honestly, and #20544 is the right carrier. The 20489 changeset says the in-memory driver "still adds naively in its own ② Semver level
The 20489 changeset, each sentence: the title, TRUE; ③ Boundary flagsOpen question 1 (land as is, or widen to the hoist): the seat ruled A, and A is right. Triage scoped the card to the rows path; the claim's ⛔ forbids editing driver-memory; the family close-out is on #20544, which reads "serial after PR #20543" so the helper lands first. This PR lands. Round 1 deviations 1-4: accepted, each verified. (1) The three branch commits carry Patch-round deviations 1-3. (1) The Out-of-scope findings. Round 1 [0], the driver-memory family: filed as #20544, the right carrier (① item 5). Round 1 [1], the 20387 note's false sentence: done in the patch round as the DELIBERATE CORRECTION (① item 3). Patch round [0], the Added by this review, carrier the next touch of that file, not a FAIL: the 20387 test header's line 26 still says PR-body sentences: TRUE, with three notes. (i) "a true merge of Governed surface: none (no Check-runs on Implemented-by: VERDICT: PASS |
Fixes #20489
Clause-②: no
What changed
The engine's rows path (
packages/objectql/src/in-memory-aggregation.ts,applyInMemoryAggregation) now addssum/avgwith Kahan-Babuska-Neumaier compensation. That is the direction triage 5875711059 ruled: fix the one face the platform owns, so the rows path agrees with SQLite native on both SQLite paths and gives the more accurate answer.compensatedSum, is a transcription of SQLite'skahanBabuskaNeumaierStepplus its finalizers' overflow guard: when the error term is non-finite, the plain running sum is returned. Thesumarm and theavgarm both call it. Before, each arm had its own naivereduce.toNumber, the null / non-numeric handling, the empty group (sum0,avgnull) and the answer's type (a JS number) are untouched.packages/spec.Head measured:
8ad4a9d4a. It is a true merge oforigin/mainat03b19d9cf, on top of the two commits of this branch.H1: the defect, before and after
Base
b2b6a0643, SQLite 3.53.4 (better-sqlite3),driver-sql. Onenumbercolumn. The rows path is forced by a filtered siblingcount. Bothengine.aggregateandPOST /api/v1/data/:object/querywere read, and they gave the same values in every cell:sum/avg8ad4a9d4a0.1, 0.2, 0.30.6/0.199999999999999980.6000000000000001/0.200000000000000040.6/0.199999999999999981e16, 1, -1e161/0.33333333333333330/01/0.33333333333333331e16, 0.5, -1e160.5/0.166666666666666660/00.5/0.166666666666666660.1, 0.20.30000000000000004/0.150000000000000021, 2, 3, 40, 500546/109.2having { s: { $eq: 0.6 } }, engine and REST, and{ a: { $eq: 0.19999999999999998 } }, engine:acase through REST, on both paths.H1 holds as the card stated it.
H3: the compensated fold against SQLite native
SQLite's native
sum/avgwas measured on three engines: better-sqlite3 3.53.4 (driver-sql), sql.js 3.49.1 (driver-sqlite-wasm) and @libsql/client 3.45.1 (driver-turso). better-sqlite3 and sql.js were read over aREALand aNUMERICcolumn, libsql over aREALcolumn. The JS fold was run over the same doubles. The2^53row was read on better-sqlite3 alone.0.1, 0.2, 0.30.6000000000000001/0.200000000000000040.6/0.199999999999999980.6/0.199999999999999981e16, 1, -1e160/01/0.33333333333333331/0.33333333333333331e16, 0.5, -1e160/00.5/0.166666666666666660.5/0.166666666666666661e20, 1, -1e200/01/0.33333333333333331/0.33333333333333330.1, 0.2(two-addend control)0.30000000000000004/0.150000000000000021, 2, 3, 40, 500(integers)546/109.22^53, 1, 1900719925474099290071992547409949007199254740994sumandavgmatched SQLite on all 5,000 groups; the naive fold disagreed on 1,601 of them.NUMERIC, SQLite stores1e16as an INTEGER and sums it in exact int64. The answer is the same1.1s (last row).H2: census of rows folds of
sum/avginpackages/**0.1, 0.2, 0.3, head8ad4a9d4aapplyInMemoryAggregation,sumandavgarmsreducecopies in one function, now one helper0.6/0.19999999999999998filterhavingsummary(summary-aggregate.ts), and service-analyticsObjectQLStrategyengine.aggregatememory-driver.ts,sum/avgarm)0.6000000000000001/0.20000000000000004memory-analytics.ts, mingo$sum/$avg)0.6000000000000001/0.20000000000000004preview-evaluator.ts,sum/avgarms)0.6000000000000001/0.20000000000000004dataset-executor.tscomputeDerivedsumNativeSQLStrategy, driver-mongodb$sumThree sites keep their own fold. They are measured and reported, not edited, as the dispatch requires. One reading needs stating plainly:
engine.aggregateon driver-memory answers0.6000000000000001on its native path and0.6on the rows path.having { s: { $eq: 0.6 } }keeps the group on the rows path only.@objectstack/core, asbucketDateKeywas, and adopt it on driver-memory's two faces and the preview.H4: what stays as it was (pinned)
nulladds nothing tosumand is left out of theavgcount.0and counts inavg. A numeric string reads as its number. A boolean reads as 0 / 1.sum0,avgnull.Infinity,-Infinity, overflow,NaN) isObject.is-equal to the naive answer.H5: the residual
Residual, stated. PostgreSQL and MySQL add
sum/avgnatively in double without compensation. That arithmetic is the database's own, and this PR does not wrap it. So over three or more fractions, their native path can still differ from the rows path in the last place. For0.1 + 0.2 + 0.3, #20387's measurement on live PostgreSQL 16.13 and MySQL 8.0.46 gave native0.6000000000000001; the rows path is now0.6. An exact$eqon a fractional sum compares doubles, and a cross-dialect last-place difference remains there: compare with a range.find()returns, and aggregatesum/avg: PostgreSQL and MySQL native answer exact decimal (0.1 + 0.2 = 0.3) while SQLite and the engine rows path answer a double (0.30000000000000004), sohaving { s: { $eq: 0.3 } }keeps the group on PG / MySQL native only #20387's pin reads those as[0.1, 0.2]on both servers.Tests
New pins
packages/objectql/src/in-memory-aggregation-compensated-sum.test.ts: 11 cases, covering the H3 fixtures, groupBy plus a per-aggregation filter, and the H4 cases.packages/rest/src/rest-aggregate-compensated-sum.test.ts: 4 cases on SQLite, through the engine and REST. It proves each path really ran (a spy ondriver.aggregate), checks thatfind()reads back the doubles, pins native equal to rows for every group, and pinshaving$eq/$inon both paths.Reverse verification, from the committed fix
scripts/ablation-replace.mjs(wrap mode). It swappedreturn Number.isFinite(c) ? s + c : s;forconst ablation20489 = s; return ablation20489;, which is the naive running sum.ff8d2385a69fto89ea11d3bc18.pnpm --filter @objectstack/objectql build, andablation-dist-preflight.mjs @objectstack/objectql ablation20489found the marker in 4 built files.git diff HEADempty).--absentfound the marker absent from all 14 built files, with a clean tree.Suites
pnpm --filter @objectstack/objectql testdb9d27230pnpm --filter @objectstack/rest testdb9d27230typecheckfor both packages,check:test-typecheckincludeddb9d27230tsconfig.test.jsonprograms (--listFilesOnly)rest-aggregate-numeric-having16 passed / 24 skipped8ad4a9d4aThe merge brought driver-sql, cli, lint, platform-objects and spec changes, and none in objectql or rest. After it, the
@objectstack/rest^...closure was rebuilt before the targeted re-run.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat8ad4a9d4aprinted 63 commands. All 63 exit 0, and each exit code was written to a file before any pipe.check:dual-build-cjs-loadsandcheck:type-check-debtfirst exited 3 (PREREQUISITE NOT MET: nodist/).pnpm exec turbo run build --concurrency=2 --filter='./packages/*' --filter='./packages/*/*'(71 tasks), and both passed.--ran: 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN.dispatch-gatesnames.Lint
pnpm lintis CI's.eslint --no-inline-config --format jsonover the three changed TS files counted 3 files, 0 errors and 0 warnings.eslint.config.mjsfiles: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']; the changeset is not a linted extension.parserOptions.project), so this diff cannot move an untouched file's verdict.Acceptance notes
All three notes from the first round were corrected in patch round 1, text only (below):
AGGREGATE_ACCUMULATIONresidual comment inpackages/drivers/driver-sql/src/sql-driver.ts("as the rows path adds them", "on SQLite's native face alone", "every other face0.6000000000000001").sql-driver-20387-aggregate-double-accumulation.test.tsthat calledrowsPathSumthe rows path's arithmetic..changeset/20387-aggregate-one-double.mdresidual paragraph, as a DELIBERATE CORRECTION.Still standing, not edited (outside the admitted text): the
aggregate()inline comment insql-driver.ts("so one query answers one number on every face", beside theaccumulatesInDoublecall). It was already an overclaim before this PR, since SQLite's native sum compensated. It points atAGGREGATE_ACCUMULATION, which now states the residual. Carrier: none.DELIBERATE CORRECTION
Note:
.changeset/20387-aggregate-one-double.md. It is pending onmain, so it ships in the same release as this PR.Sentence corrected: the "Residual, stated." paragraph, and only that paragraph. It read:
0.1 + 0.2 + 0.3: SQLite0.6, every other face0.6000000000000001). This was already true of SQLite's two paths before this change."What changed under it: this PR makes the engine's rows path add with compensated summation, as SQLite 3.43+ does. The rows path now answers
0.6, not0.6000000000000001, so "every other face" and "SQLite's native face alone" are false once this PR lands.It now reads: PostgreSQL and MySQL add without compensation, while SQLite and, since #20489, the rows path compensate. So the PostgreSQL / MySQL native answer can differ from SQLite's and the rows path's in the last place (
0.1 + 0.2 + 0.3: PostgreSQL / MySQL native0.6000000000000001, SQLite and the rows path0.6). Before #20489, SQLite's own two paths differed there too. Two addends cannot differ.Every other sentence was checked against this PR and left unchanged:
11 / 9answered1.2222222222222222, where every other face answers1.2222222222222223", stays true. Integer addends within 2^53 sum exactly under both folds. Measured through the built rows path at1c4767883:applyInMemoryAggregationover1,1,1,1,1,1,1,2,2answerssum11andavg1.2222222222222223, and over1, 2, 2answers1.6666666666666667.0.1 + 0.2statements (base lines 11-12, 38-40 and 48-50) are two addends, which no compensation moves.Gate:
check-empty-changesetis red on exactly this one name, by design ("DELIBERATE CORRECTION ... do NOT restore it"). ⛔ Noskip-changeset; the note is not restored. The at-tier review is the written confirmation, as the seat answer asks.Patch round 1
New head:
1c4767883. One push, carrying a true merge oforigin/mainat288611e3e(spec-only commits) as925368f00, then one text-only commit. Seat answer 5881988201 ruled the open question A.Edits:
.changeset/20387-aggregate-one-double.md: the DELIBERATE CORRECTION above.packages/drivers/driver-sql/src/sql-driver.ts: theAGGREGATE_ACCUMULATIONresidual comment now states PostgreSQL / MySQL native against SQLite and the rows path.packages/drivers/driver-sql/src/sql-driver-20387-aggregate-double-accumulation.test.ts: the header sentence says the expected values arefind()'s rows added in row order, which is the rows path's compensated sum too over this file's fixtures (two addends, and integers within 2^53). "a compensated sum ... fail" is removed, since it was never true for two addends. The one-line docs ofrowsPathSum/rowsPathAvgmake the same statement, because the header links them..changeset/20489-rows-path-compensated-sum.md: it no longer says it "replaces" the driver-sql residual. It says the driver-sql entry states the same residual, and it keeps the driver-memory split and the PostgreSQL / MySQL residual.Comment-only proof for items 2 and 3: every changed line under
packages/drivers/driver-sql/is a comment line (git difffiltered to non-comment lines is empty).No
@objectstack/driver-sqlline added to this PR's changeset. Why none is needed:fixedgroup in.changeset/config.json, so the@objectstack/objectqlpatch releases driver-sql in lockstep anyway.@objectstack/driver-sql's builtdist/index.{js,mjs,d.ts,d.mts}carry 0 hits for "Kahan", "20489" or "every other face". The positive control,AGGREGATE_ACCUMULATION, is found in those same four files.Checks at
1c4767883, each exit code recorded before any pipe:node scripts/check-empty-changeset.mjs --base origin/main.changeset/20387-aggregate-one-double.md(one::errorline), by design; the empty-frontmatter half is greennode scripts/check-changeset-fixed.mjsfixedgroup in sync with 69 public packagesnode scripts/check-changeset-no-major.mjs --base origin/mainmajorbumpnode scripts/check-adr-0087-registration.mjs --base origin/mainpnpm check:nul-bytesnode scripts/check-issue-citations.mjspnpm --filter @objectstack/driver-sql typecheck--listFilesOnly)vitest run src/sql-driver-20387-aggregate-double-accumulation.test.ts(driver-sql)pnpm check:driver-conformance,check:object-def-param-keys,check:tenant-chokepointdispatch-gatesnewly derives for the driver-sql pathsThe other 63 derived families were run at
8ad4a9d4a(above). This round changes only text on top of a spec-only merge, and CI re-runs them.Generated by Claude Code