Skip to content

[drivers] driver-memory / driver-mongodb disagree with the SQL family on count_distinct — the two AGGREGATION_CASES cells #6409 left open #6814

Closed

Description

@os-zhuang

Filed unassigned from the #6409 lane (the enforce half of #6188). Recording, not claiming — both packages are inside the #5499 investment freeze and #6409's ruling put them explicitly out of scope. This issue exists so the two open cells of the new AGGREGATION_CASES matrix column have a measured verdict behind them instead of an omission.

Context

#6409 lowered count_distinct to COUNT(DISTINCT x) on the SQL family and introduced AGGREGATION_CASES / AGGREGATION_ROWS (@objectstack/spec/data) — the shared standard the aggregate lowerings are now held to. Three of the five drivers were enrolled by that PR (driver-sql, driver-turso REMOTE, driver-sqlite-wasm). The other two carry DEBT rows in scripts/check-driver-conformance.mjs pointing here.

The standard's answer for count_distinct is distinct NON-NULL values of the target column — what COUNT(DISTINCT col) computes on SQLite, PostgreSQL and MySQL alike, and what objectql's in-memory fallback (in-memory-aggregation.ts, new Set(values.filter(v => v != null)).size) and service-analytics's AGGREGATE_SQL already give.

What each package answers today

Read from the source on origin/main @ 6de592c; not executed against the fixture, since neither cell has a suite (that is the debt).

1. driver-memory — the DATA face answers null, silently

MemoryDriver.computeAggregate (packages/drivers/driver-memory/src/memory-driver.ts) has arms for count, sum, avg, min, max and then:

default:
    return null;

There is no count_distinct arm. So aggregate() with { function: 'count_distinct', field: 'stage', alias: 'n' } resolves with { n: null } — no error, no log, no refusal. Against AGGREGATION_ROWS the standard says 2.

This is the worse half of the pair: a wrong ANSWER rather than a wrong number. It is also the default:-arm shape #4157 and the aggregation-lockstep guard in service-analytics exist to prevent one layer up, arriving here through a different door.

Note the package is partial in the way #6409's ruling described: its ANALYTICS face (memory-analytics.ts) does implement count_distinct ($addToSet, then sized). It is only the data-driver aggregate() path that has no arm — so this package answers one declared function two ways depending on which face you enter, the divergence class #5374 fixed for $contains in the same package.

2. driver-mongodb — counts NULL as a distinct value

mongodb-aggregation.ts lowers count_distinct to { $addToSet: fieldRef ?? null }, and postProcessAggregation takes the array's .length. $addToSet adds an explicit null to the set, so a column with rows won, won, lost, null, won, null sizes to 3 where the standard says 2.

$addToSet on a MISSING field adds nothing, so the divergence only shows for an explicitly-null value — which is exactly what AGGREGATION_ROWS seeds and exactly what a nullable column produces in practice.

Same file still carries a string_agg arm and a string_agg branch in postProcessAggregation; string_agg was retired from AggregationFunction at #6188, so that code is no longer reachable through a spec-valid request. Not a defect — noted so it is not rediscovered as one — and untouched for the same freeze reason.

Disposition

Deliberately none proposed. Both packages are #5499-frozen, so this is a decision about the freeze, not about the code: the fix on driver-memory is a count_distinct arm beside its neighbours, and on driver-mongodb a $ne: null filter before the $addToSet (or sizing a $setDifference against [null]). Neither is difficult; both are inside the boundary.

Clearing a cell means writing the suite that runs AGGREGATION_CASES and deleting the ledger row in the same PR — the gate fails either half alone. driver-mongodb's real-mongod suites are opt-in since #5517, so whatever clears its cell needs a server-free half in the shape of mongodb-filter-logic-translation.test.ts.

Refs: #6409 (the lane that measured this), #6188 (the split ruling), #5499 (the freeze), #6682 / #6520 (the same pair's other two open cells).

Activity

  1. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    ContributorAuthor

    Triage: finding + domain:drivers (labels and this comment land as a pair).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-zhuang commented on Aug 9, 2026

    @os-zhuang
    ContributorAuthor

    Findings triage round: held (finding stays) — first formal grading; the 08-08 comment above was the triage-time classification.

    Premises re-verified on origin/main @ 3e8e669: memory-driver.ts still has zero count_distinct hits while memory-analytics.ts has five (the two-face divergence intact); mongodb-aggregation.ts:142 still lowers via $addToSet: fieldRef ?? null. The governing hold reason is unchanged: #5499 (investment freeze on driver-memory / driver-mongodb) is still open and standing, and the divergence is already declared machine-readably as DEBT rows in scripts/check-driver-conformance.mjs — nothing is silently wrong-looking-right.

    Restart conditions (unchanged from the triage comment): the #5499 freeze lifts, or the maintainer green-lights clearing the two AGGREGATION_CASES cells — at which point this promotes with the card's own fix sketch, and the clearing PR must pair suite + ledger-row deletion per the gate's rule.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. self-assigned this
    on Aug 11, 2026
  4. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Claim: PM loop round 2 (drivers seat, post-unfreeze batch 1) — mongodb half only
    Session: session_013ZU38vNzLXCscgdFZjKd53
    Branch: claude/issue-6850-mongodb-structured-groupby (shared with #6850 — same file, same new server-free suite, same AGGREGATION_CASES conformance cell; PR carries Fixes #6850 + Part of #6814)
    Worktree: objectstack-issue-6850
    Domain: domain:drivers
    File surface: same as #6850's claim comment (one card).
    Container & model: M, mode:cloud, model: opus
    Serial constraints cleared: same as #6850's claim comment.

    Scope split under the 2026-08-11 partial unfreeze (verbatim ruling on #5499, comment 5249019855): the driver-mongodb cell (null counted as a distinct value by $addToSet: fieldRef ?? null) dispatches now; the driver-memory cell (missing count_distinct arm, two-face divergence) stays under the #5499 freeze — driver-memory's restart condition is untouched by the ruling. This issue therefore stays OPEN after the PR lands, returning to pm:on-hold for its memory half.


    Generated by Claude Code

  5. removed their assignment
    on Aug 11, 2026
  6. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Returned to pm:on-hold — 2026-08-11, the driver-mongodb half is DONE, the driver-memory half is the card's remaining scope.

    Claim released (assignee removed); the dispatch is complete for the half that was authorized.


    Generated by Claude Code

  7. self-assigned this
    on Aug 11, 2026
  8. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Claim: PM loop round 6 (drivers seat, memory-unfreeze DEBT-clearing card) — driver-memory half, the card's LAST half
    Session: session_013ZU38vNzLXCscgdFZjKd53
    Branch: claude/issue-6814-6682-memory-debt-rows (shared with #6682's memory half — one card: both remaining DEBT rows live in the same file surface, memory-driver.ts + adjacent ledger rows in scripts/check-driver-conformance.mjs; splitting them guarantees a merge conflict)
    Worktree: objectstack-issue-6814
    Domain: domain:drivers
    File surface: packages/drivers/driver-memory/src/memory-driver.ts (regions: computeAggregate count_distinct arm; the $contains-family fold sites ~:808-830, ~:985-994, :1353), memory-analytics.ts (count_distinct consistency + its fold sites), memory-matcher.ts (verify-only expected), memory test files + new/extended conformance suites, scripts/check-driver-conformance.mjs (BOTH driver-memory DEBT rows, :410/:441), .changeset/* (stop on breach; explain in the report)
    Container & model: M, mode:cloud, model: opus
    Serial constraints cleared: lane in-flight ZERO at selection (the mongodb batch fully landed); hot-file sql-driver.ts untouched; #7117 (memory-analytics LIKE rendering) deliberately SERIALIZED behind this card — same memory-analytics.ts surface, returns to queue separately.

    Authorization: maintainer fully lifted the #5499 freeze 2026-08-11 (verbatim on #5499, comment 5252526378:「driver-memory 也解冻,继续清剩下两行 DEBT」). This dispatch IS that mandate. PR carries Fixes #6814 + Fixes #6682 — both cards' mongodb halves already landed (#7550 / #7625), so these memory halves are the last open scope of each and merging correctly closes both.

    Premises re-verified by the PM on origin/main @ 79c3145: count_distinct has ZERO hits in memory-driver.ts (the missing arm) vs 5 in memory-analytics.ts (the two-face divergence intact); NINE 'i'-flagged RegExp sites across three clusters in memory-driver.ts (more than the card's framing — enumerate all); memory-matcher.ts is String.includes case-exact; DEBT rows at :410/:441.


    Generated by Claude Code

  9. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    os-dev report — the driver-memory halves of #6814 + #6682 · session session_01NuzQE844ut8m8vypXcdBYD · branch claude/issue-6814-6682-memory-debt-rows · draft PR #7723 (Fixes #6814 + Fixes #6682)

    Headline

    Both driver-memory DEBT rows are gone, and the ledger is now EMPTY — check-driver-conformance reads 40 covered cell(s), 0 in the DEBT ledger, 0 exempt, the first time since the gate was written.

    Premises verified before the first edit — all live, one FALSIFIED

    The PM's measurements on origin/main @ 79c3145 all held at 21888ab: zero count_distinct hits in memory-driver.ts vs 5 in memory-analytics.ts; nine 'i'-flagged RegExp sites in three clusters; memory-matcher.ts case-exact.

    Verified by EXECUTION, not reading — both suites written first and run against unmodified main:

    suite pre-fix post-fix
    memory-filter-text-conformance.test.ts 9 failed / 43 passed (52) 52/52
    memory-aggregation-conformance.test.ts 11 failed / 23 passed (34) 34/34

    Every text-suite failure was a $contains-family row. Every $icontains row, literal-comparand row, refusal row and reference-matcher row passed before the fix — so the dispatch's suggested check is answered: requirements 1 and 3 were already fully closed on this package, requirement 2 was the whole gap, and nothing was widened to reach green.

    One premise is DEAD, and it is the card's own. #6814, its ledger row and aggregation-conformance.ts's DEBT table all say the ANALYTICS face "DOES implement count_distinct". It does not. buildAggregator emitted { $addToSet } under a comment reading "Will need post-processing for count" — and no post-processing existed. The measure answered the raw array ['won','lost',null], under a field measureTypeToFieldType types as number. So the package answered one declared function three ways: null on the data face, an array on the analytics face, the standard's number nowhere. Wrong on the null question the row anticipated and on a shape it did not. Both fixed here; the DEBT table's row is corrected to what execution showed rather than just struck through.

    The :985-994 cluster — the dispatch's open question, answered

    Not an $in-like path. It is normalizeFieldOperators — the same four $contains-family operators reached through the FilterCondition door instead of the AST door. Squarely inside #4706 Q2 = A; nothing was widened beyond the family the ruling names. The third cluster (filterSubstringPattern, :1353) is the #5374 shared rule the analytics face borrows rather than re-derives, so one line fixed that face — #5374 working in the direction it was built for.

    Multi-face declaration

    face $contains family count_distinct
    query path (both spellings, both doors of find() / aggregate(AST)) fixed here — 8 sites fixed here — the missing arm
    analytics face fixed here — via the shared rule fixed here — $addToSet sized, null-excluded
    reference matcher already compliant — String.includes; verified by 13 rows, file untouched n/a

    Cross-backend scoreboard cited (#6518 / #7625), not re-proved.

    Out of scope, with reason

    Pins flipped, not deleted (5)

    Two in memory-driver.test.ts (the case pin flipped plus a new $icontains sibling, so the pair keeps covering both behaviours; and the $and row where the comparand moved rather than the expectation, since combination is that row's subject), three in memory-driver-filter-logic-conformance.test.ts (plus a positive control so they cannot be passed by a predicate that stopped matching anything).

    Reverse verification — direction predicted BEFORE it was run

    • (A) count_distinct arm removed: predicted failures on the VALUE null, never a throw, arithmetic green. Measured 7 failed / 27 passed — exactly that.
    • (B) arm present but new Set(values).size: predicted 3-for-2 ungrouped, east 2 / west 3 grouped, count_distinct(score) staying GREEN at 6. Measured 6 failed / 28 passed — exactly that.

    Prose made false by the diff, corrected (4 files, zero schema lines)

    filter-text-conformance.ts, filter.zod.ts, aggregation-conformance.ts, and content/docs/protocol/objectql/query-syntax.mdx (two warn callouts saying one driver still folds → info, naming the row-set change). Per the #7625 / #5586 precedent; PM may want cross-seat declarations for the spec-surface and devx seats.

    Gates

    gate result
    check:driver-conformance OK — 40 covered, 0 DEBT, 0 exempt
    --self-test OK
    driver-memory suite 664 passed (22 files)
    spec suite 9883 passed (377 files)
    objectql suite 3228 passed (182 files)
    runtime suite 2018 passed (127 files)
    service-datasource / http-conformance / verify 326 / 72 / 23 passed
    tsc --noEmit (driver-memory, spec) clean
    build (dependency closure) clean

    Downstream selection measured, not assumed: ...@objectstack/driver-memory lists 16 dependents and objectql is not among them — it depends the other way — so its suite was run on its own account and is green.

    Changeset: patch for @objectstack/driver-memory, pointing users who relied on the fold at $icontains. No content/docs/releases/ edit. Heavy runs held flock /tmp/os-heavy-verify.lock.

    CI convergence, ready-flip and landing are the PM's — not polled here.


    Generated by Claude Code

  10. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    os-dev patch round — PR #7723, ESLint red · commit 2a8c150 · same branch, no new PR

    Reproduced locally, and the ratchet named the count but not the sites

    node scripts/check-query-options-erasure-ratchet.mjs reproduced it exactly: test surface grew 242 → 245 site(s). The script reports an aggregate, so I measured the per-site list by driving its own measure() approach (the real ESLint config with the rule block's ignores lifted, matched on QUERY_OPTIONS_RULE_ID) over driver-memory/src/*.test.ts.

    All three sites were in ONE file — memory-aggregation-conformance.test.ts:143:45, :154:50, :168:45. memory-filter-text-conformance.test.ts contributed zero: its { where: filter as any } is a cast on the filter value, not on the options position the rule matches. Worth recording, since the dispatch expected sites in both files.

    Per-site choice: TYPED at all three, as unknown as at none

    The three sites were the same expression — queryFor(c) as any — at the three places the case-set query is handed to the driver: find(), aggregate(AST) (the door objectql's engine uses), and the never-answers-null property row.

    Fixed once, at the source rather than three times at the call sites: queryFor now declares DriverQuery as its return type (import type { DriverQuery } from '@objectstack/spec/contracts' — the same type MemoryDriver.find takes, import-reachable as the dispatch said). All three casts are gone and all three arguments are now checked by tsc.

    The as unknown as EngineQueryOptions spelling would have been wrong at every one of these sites, and the reason is the file's own subject: every case here is deliberately ON contract — the point of the suite is that the shared standard's own vocabulary reaches the driver unmodified. There is no bypassed contract to name, so naming one would have been a false label on a passing type. That spelling is for a rejection test building input tsc should refuse; nothing here is that.

    The baseline is NOT raised. The count returned to the ceiling rather than the ceiling moving to the count.

    Gates re-run

    gate result
    check-query-options-erasure-ratchet ✓ holds — test surface 242 site(s) in 48 files, at the ceiling; 67 unswept non-test sites, none new; baseline key set verified against 21888ab, no files added
    tsc --noEmit (driver-memory) clean — the typed literal compiles against DriverQuery with no widening
    driver-memory suite 664 passed (22 files) — unchanged
    check:driver-conformance OK — 40 covered, 0 DEBT, 0 exempt

    Diff for this round is one file, +14 / −5, test-only — no source, no baseline, no other file touched. Everything else in the PR is as reviewed.

    CI convergence and the ready-flip remain the PM's.


    Generated by Claude Code

  11. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    ACCEPT — PR #7723 (head 2a8c150 after the ratchet patch round; covers this card and #6682, verdict recorded here).

    Flipping ready + queueing. On merge both cards close (Fixes #6814 + Fixes #6682) — the #4706/#6188 conformance programme's driver matrix is complete.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions