fix(analytics): the native-SQL strategy applies the engine's aggregate policies — double accumulation, the PostgreSQL boolean cast and the empty-sum fold, hoisted into core (#21042) - #21209
Conversation
…L face (red), and the driver-sql statement the move must keep The pins this card's fix turns green, committed first: - core aggregate-answer.test.ts: the operand policies the hoist exports (red: the exports do not exist yet); - service-analytics native-sql-aggregate-policies.test.ts: each measure on both faces at the cube and dataset doors, held to the engine's number (red on SQLite: the all-NULL and measure-scoped sum fold at the cube door; red on PostgreSQL: double accumulation, the boolean cast, the fold); - cube-measure-field-type-door.test.ts: the PostgreSQL native boolean cell runs (red on PostgreSQL: max(boolean) does not exist); - rest analytics-dataset-aggregate-policies-door.test.ts: the dataset route on both strategies (red on PostgreSQL); - driver-sql sql-driver-21042-aggregate-policy-move.test.ts: the move proof, captured from the driver at the base, green before and after the move. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…s with the base count, so every group is reported A selection of measure-scoped measures alone reports only the groups their filter admits, which is the executor's row set and not this card's question. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…ompile and shaping point apply them Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…r-sql and service-analytics patch Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…tive-aggregate-policies
…tive-aggregate-policies
…tive-aggregate-policies
…tive-aggregate-policies
📓 Docs Drift CheckThis PR changes 3 package(s): 13 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 36 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 c96a80761eb220dbd4eefd0c3dff224950794d46 && git checkout c96a80761eb220dbd4eefd0c3dff224950794d46
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5e5ce48cef6d1a05714a9fda7eeaed6df5c072b8 ef1f9d84846c5c30d7753d727a4c8a3d65ff8b59 && git checkout -B drift-repro 5e5ce48cef6d1a05714a9fda7eeaed6df5c072b8 && git merge --no-ff ef1f9d84846c5c30d7753d727a4c8a3d65ff8b59
node scripts/docs-audit/affected-docs.mjs --json 5e5ce48cef6d1a05714a9fda7eeaed6df5c072b8
|
Contract reviewServed-tier: Inputs, and nothing else: card #21042 (its body and all eight comments: triage ① Derived judgmentsPublic surface.
Accept set. Served values that move, each to declared text.
The compile wraps only what the policies name. The fold. At the The move is a move. The Tests and the lifted skip. ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
Fixes #21042
Clause-②: yes (widening)
What this changes
The analytics native-SQL strategy (
NativeSQLStrategy, the default on a SQL driver) skipped three aggregate policies thatSqlDriver.aggregate()applies. So one route answered different numbers, or a500, depending on which strategy served it. This PR follows the route ruling on #21042 (comment5925613967): the operand policies are hoisted into@objectstack/core, besideAGGREGATE_ANSWER_KIND, and both faces read them from there.packages/core/src/utils/aggregate-answer.ts(@objectstack/core,minor). It takes:AGGREGATE_ACCUMULATION, moved fromdriver-sqlwith its docblock (the docblock and the table are byte-identical to the base, apart from theexportkeyword);aggregandColumnClass({ type, multiple }), the one column-class predicate ('fractional','integral','boolean', or none);POSTGRES_BOOLEAN_AGGREGAND_CAST, the driver-sql: boolean aggregands need a lowering cast on PG (+ a MySQL min/max presentation check) — the ruledfalse/true+ arithmetic answers are unproducible on the PG face #11635 cast, as aRecordoverAggregationFunction:sum/avg/min/maxare cast, the counts never are;doubleAccumulationOperand(operand, dialect), the double operand with the dialect as a parameter;aggregandOperandSql(func, columnClass, dialect, operand), the one composition both faces emit (the cast inside, the double operand around it).No
index.tsline was added: the module is already exported.packages/drivers/driver-sql/src/sql-driver.ts(patch), in the ruled regions only. TheAGGREGATE_ACCUMULATIONtable becomes a pointer plus an import.isFractionalNumericTypeis deleted. The two registry fills fillfractionalNumericFieldsthrough the predicate.accumulatesInDouble/doubleAccumulationOperandare replaced byaggregandColumnClassOf, which maps the driver's registries onto the predicate's classes. Inaggregate(), the private boolean-cast condition and the accumulation call become oneaggregandOperandSql(funcName, class, this.dialectName, '??').packages/services/service-analytics/src/strategies/native-sql-strategy.ts(patch), in two places.resolveMeasureSqlwraps the column it handsAGGREGATE_SQL/CONDITIONAL_AGGREGATE_SQLinaggregandOperandSql. The column class comes from the declaration the host already relays (declaredValueShape), on the object the column lives on (columnObjectOf, the one hop resolver). The dialect comes fromsqlDialect, which the strategy already reads.executeshaping point that PR fix(core,driver-sql,service-analytics): the analytics native-SQL path answers measures declared number as numbers (#20889) #21040 added now folds anullmeasure answer toemptyGroupValueFor(measure.type)(@objectstack/spec). It does this for every measure, measure-scoped ones included, before the number presenter, indriver-sql's order. The dataset door'sDatasetExecutorfill stays, and it is idempotent on a folded row.generateSqlcall site, which now passesctxtoresolveMeasureSql.canHandle,buildFieldMeta, the hop-object sites, the filter / text-match rendering,analytics-service.tsandfield-read-admission.tsare untouched.The card's table, before and after
Measured through
AnalyticsService.query(the cube door, whichPOST /api/v1/analytics/queryrelays verbatim) andAnalyticsService.queryDataset(the dataset door), onAnalyticsServicePluginover a real ObjectQL engine andSqlDriver. Native is the plugin's own composition (NativeSQLStrategyanswered, with one raw statement and no engine aggregate). ObjectQL is the same composition narrowed toengine.aggregate. "Before" is the strategy file at the based34aa58a2a; "after" is this branch at61aab5013a. Neither merge since then touches the aggregate code paths, and the pins below are green atef1f9d8484.Fixture:
f:frac(anumbercolumn) holds 0.1 and 0.2, andflagholds true and false;i:stars(aratingcolumn) holds seven 1s and two 2s, andflagholds 7 trues and 2 falses;n: every aggregand is NULL in all three rows.The measure-scoped measures filter on
tag = x, which only groupfholds.PostgreSQL 16.13 (a private local server; the ObjectQL column is the same before and after):
sum(frac)0.30.300000000000000040.30000000000000004avg(frac)0.150.150000000000000020.15000000000000002avg(stars), an integer column1.2222222222222221.22222222222222231.2222222222222223sum(flag)500 DATABASE_ERROR77avg(flag)500 DATABASE_ERROR0.77777777777777780.7777777777777778min(flag)/max(flag)500 DATABASE_ERROR0/10/1sum(frac), all-NULL groupnull00sum(frac), all-NULL group0(executor fill)00sum(flag), all-NULL group500 DATABASE_ERROR00avg(frac), all-NULL groupnullnullnullsum(frac), no admitted rownull00sum(frac), no admitted row0(executor fill)00avg(frac), no admitted rownullnullnullcountcontrol333countcontrol000SQLite (better-sqlite3): accumulation and the boolean answers already agreed on every face (
0.30000000000000004,0.15000000000000002,1.2222222222222223,7,0.7777777777777778,0/1). The fold is the policy that diverged there:sum(frac)/sum(stars)/sum(flag), all-NULL groupnull00sum(frac)/sum(stars)/sum(flag), all-NULL group0(executor fill)00sum(frac), no admitted rownull00sum(frac), no admitted row0(executor fill)00After the fix, the native and ObjectQL faces differ in 0 of 144 cells (2 drivers × 2 doors × 12 measures × 3 groups).
MySQL is NOT MEASURED: there is no MySQL server in this container. The MySQL operand text is pinned offline: by
core'saggregate-answer.test.ts, and by thedriver-sqlmove proof for the driver's own statements.The move proof
driver-sql's aggregate statements were dumped at the base, before any consumer changed. The dump coveredSqlDriver.aggregate()for every function (count,count_distinct,sum,avg,min,max, andcount(*)), aliased and unaliased, over 23 columns: every fractional, integral and boolean type, thefloat/integer/intaliases, multi-valued and untyped columns, and text / date / lookup / formula. It ran on SQLite, PostgreSQL and MySQL, through both registration paths (registerObjectMetadataandregisterExternalObject), offline (knextoSQL()).The policies were then hoisted,
driver-sqlwas switched to the imports, and the same dump was run again:8eee668372a28a7568f3eb1cc5a2bc9b;bc8aa0cc2f): 1668 entries, md58eee668372a28a7568f3eb1cc5a2bc9b.cmpprinted nothing: the two dumps are byte-identical.sql-driver.tsandaggregate-answer.tsare unchanged betweenbc8aa0cc2fand61aab5013a.The committed move-proof pin,
packages/drivers/driver-sql/src/sql-driver-21042-aggregate-policy-move.test.ts, holds the captured expressions for one column of each class, on each dialect and through each registration path. It passed at the base (192fc0010b: 54 / 54) and passes after (54 / 54).Pins (committed red first, then the fix)
192fc0010b, base code)coreaggregate-answer.test.tsservice-analyticsnative-sql-aggregate-policies.test.ts(each measure on both faces at both doors, against the engine's arithmetic; SQLite and live PostgreSQL cells)500, folds redservice-analyticscube-measure-field-type-door.test.ts, the lifted skipmax(boolean)red (500)restanalytics-dataset-aggregate-policies-door.test.ts(the route, both strategies)driver-sqlmove proofThe lifted skip:
it.skipIf(cell.id === 'pg' && face === 'native')incube-measure-field-type-door.test.ts(from PR #21128) is gone. Its comment now says why the cell runs on every cell and face. The PostgreSQL nativemax_flagcell answers1.native-sql-measure-number-presentation.test.tsgets a comment-only edit: its header said the native statement does not carry #20387's accumulation, and it now points at the new pin.Ablations
There was one ablation per policy, each predicted in writing before it ran. Each mutation was planted through
scripts/ablation-replace.mjs, which checks that the anchor hit and that the blob changed. Each mutation was confirmed in the builtdist/(ablation-dist-preflight.mjs: marker present). Each restore ran by absolute path (git checkout HEAD), and the file's blob was proven equal to itsHEADblob withgit diff HEADempty. After each restore the package was rebuilt, and the marker was proven absent fromdist/with the tree clean. Every prediction held exactly.coreaccumulatesInDouble's dialect gate never admits PostgreSQL or MySQLcore3;driver-sqlmove proof 24 (pg and mysql × both fills × the six numeric / boolean columns);service-analytics10, PostgreSQL only (both doors ×sum/avg(frac),avg(stars), measure-scopedsum/avg);rest3, PostgreSQL onlyavg(flag)green as predictedcoreaggregandOperandSqlnever castscore1; move proof 4 (pg × both fills ×boolean/toggle);service-analytics8, PostgreSQL only; the lifted cell, PostgreSQL native and ObjectQL, 2;rest4, PostgreSQL onlyservice-analytics8, cube door only (SQLite and PostgreSQL × the three all-NULLsums and the measure-scopedsum); everything else green, therestdataset-door route included, because the executor fill folds thererest19 / 19 greenA1 and A2 show one policy reaching both faces. Each one turned
driver-sql's own statements red. Under A2 the ObjectQL face'smax(boolean)cell failed too, with the driver's refusal. Under A1 the ObjectQL face answered the same exact decimal as the native face for the plain measures: the failing assertion was the engine's number, while native and ObjectQL still agreed.A reverse type check also ran. Passing a dialect the new type rejects (
'oracle') toaggregandOperandSqlturnedservice-analytics' typecheck red (TS2345 ... not assignable to parameter of type 'AggregandSqlDialect'), which shows the rebuiltcore.d.tswas read. The file was restored byte-identical.Verification (at
ef1f9d8484, after mergingorigin/mainatcb45469e67, which carries PR #21170 and PR #21173)Everything below ran as one locked script, at
ef1f9d8484, with each exit code captured before any pipe. The live PostgreSQL 16.13 server ran attimezone = Asia/Shanghai, and thedriver-sqlsuite ran underTZ=America/New_York, which are its own non-vacuity preconditions.pnpm turbo run build --filter='!@objectstack/docs' --concurrency=1exit 0, andpnpm --filter @objectstack/spec check:generatedexit 0.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 67 commands from the real diff (10 paths). All 67 exited 0. Reconciliation with--ranand the recorded exit codes printed:Run reconciliation — 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.pnpm --filter … typecheckexit 0 for@objectstack/core,@objectstack/driver-sql,@objectstack/service-analyticsand@objectstack/rest.vitest run --maxWorkers=2, withOS_TEST_POSTGRES_URLset so the live cells ran):corelocalcorerepodriver-sqlservice-analyticsrestlocalrestrepocore22 / 22, policies 49 / 49 (24 SQLite + 24 PostgreSQL + the oracle), field-type door 23 / 23,restroute 19 / 19 (9 SQLite + 9 PostgreSQL + the oracle).eslint --no-inline-config --format jsonover the 9 changed code files answered 9 results, 0 errors and 0 warnings. The population is read from eslint's own config:ESLint.isPathIgnoredanswersfalsefor each of the 9. The narrowing excludes nothing that could move, becauseeslint.config.mjsnever enables type-aware linting (noparserOptions.project, no typed rules), so this diff cannot change the verdict on an untouched file. The repo-widepnpm lintis CI's.driver-sqllive preconditions: the first gate run used a private server at UTC, and the suite's four timezone non-vacuity cells failed by design (… start it with timezone=Asia/Shanghai). With the server atAsia/Shanghaiand the process atAmerica/New_Yorkthe suite is green, as listed above.mainafter the final merge:origin/mainmoved 4 commits pastcb45469e67before this PR opened (chore(objectui): bump the console pin to 31971ff1e28f (one zod instance in the vendored Console), add a single-zod canary to build-console.sh, and key the release console cache on the spec zod range #21149, fix(rest): REST refusals, notes and the OpenAPI text state each decision in words instead of a tracker number (stage 2) #21188, fix(plugin-auth): the admin identity rows on the compliance ledger record the admin's decisions, never a value of a user field (#21174) #21195, pm-dispatch: owe the isolated contract review on three contract faces; changeset and docs prose move to the seat's ACCEPT #21192). None of them touchescore,driver-sql,service-analyticsor the analyticsresttests. The onepackages/specfile in the analytics area,ui/dataset.zod.ts, changes a comment only. They are not merged here; CI runs on the merge ref.Acceptance notes
coreand in thedriver-sqlmove proof.OS_TEST_POSTGRES_URLforservice-analyticsorrest. The cells above ran against a private PostgreSQL 16.13 started for this run and removed afterwards. In CI the SQLite cells run, and so do the offlinecore/ move-proof pins.avgis absent from a row its supplementary query reported no row for. That isx_avg_fracfor groupsiandn, on both strategies and before and after. It is notnull. The new service pin holds this cell only to "both faces agree", not to a value. Relatedly, a dataset-door selection made only of measure-scoped measures reports only the groups their filter admits, so the pin asks each one beside the base count.declaredValueShape), or names no SQL dialect, gets no column class or no policy. It keeps the native arithmetic it had, and a PostgreSQL booleansumthere still answers500. The plugin's own composition wires both.driver-sqlreads the class: it reads its own registries rather than calling the predicate per column.fractionalNumericFieldsis filled by the predicate.booleanFieldsandnumericFieldsare filled by the driver's coercion rules, whose populations equal the predicate's'boolean'and'fractional'∪'integral'classes. The move proof pins that equality per column class. Asking the predicate per column through the driver'svalueShapeFieldswould retirefractionalNumericFields, but its declaration and shard-alias regions are outside the ruled surface, so this PR does not do it.AGGREGATE_ACCUMULATION's docblock), so three or more fractions can still differ in the last place from SQLite and the rows path. The pins use two addends.min/max) is not addressed here.Generated by Claude Code