From f28c9cb9ae8ddb2918f2c9ea611c68025e6bf71a Mon Sep 17 00:00:00 2001 From: huangyiirene Date: Tue, 11 Aug 2026 05:08:22 +0000 Subject: [PATCH] fix(driver-mongodb): take a structured GroupByNode, and answer count/count_distinct like every other backend (#6850, #6814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `driver-mongodb` is now enrolled in the shared `AGGREGATION_CASES` standard. Clearing that cell fixed three divergences, all of the kind that ANSWER rather than fail — which is why none of them ever surfaced as an error: 1. #6850 — a structured `GroupByNode` had no lowering at all. `groupBy` was annotated `string[]` and `groupId[field] = '$' + field` stringified the object, so the `$group._id` key became the literal `"[object Object]"` and its value a field path matching nothing: rows grouped by a nonexistent path, under a column of that name. `MongoDBDriver.aggregate` passed the value through an `any` cast, which is why the declared union never met that annotation at `tsc`. Both sides now spell `GroupByNode[]`, and the `_id` keys on `alias ?? field` with the FIELD as its value — the #6401 rule. 2. #6814 — `count_distinct` sized a `$addToSet` that keeps explicit nulls, so a nullable column answered 3 where the standard says 2. The sizing now excludes null, matching `COUNT(DISTINCT col)` and the in-memory fallback. 3. Measured here, named by neither card — `count(col)` ignored `field` and emitted `{ $sum: 1 }`, so `count(stage)` came back 6 where the standard says 4. It now counts non-null values, a missing field reading as null. A `dateGranularity` node is refused with NOT_IMPLEMENTED/501 in the ADR-0112 envelope rather than silently ignored — the `driver-sql` / `driver-turso` refusal, first sentence for first sentence (#6212). A native `$dateTrunc` lowering needs the engine's bucket labels, a published capability record and `date-bucket-parity.test.ts`, so it is its own change. A `groupBy` entry that is neither half of the union is refused with INVALID_QUERY/400. The suite is server-free (the real-mongod halves are opt-in since #5517): it drives the EMITTED pipeline through a strict in-process evaluator that refuses every shape it does not model, and each pre-fix lowering is replayed through it and must fail the case it broke. It holds the LOWERING to the table and does not answer "does MongoDB agree?" — recorded as open rather than implied. The `driver-mongodb` x `AGGREGATION_CASES` DEBT row goes with it, in this PR, per the gate's rule. driver-memory is untouched: it stays under the #5499 freeze and keeps its row, so #6814 stays open for its half. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PeD85qA1JNLXodmYdnu4Zm --- ...b-structured-groupby-and-count-distinct.md | 66 +++ .../mongodb-aggregation-translation.test.ts | 491 ++++++++++++++++++ .../driver-mongodb/src/mongodb-aggregation.ts | 186 ++++++- .../driver-mongodb/src/mongodb-driver.ts | 13 +- .../spec/src/data/aggregation-conformance.ts | 35 +- scripts/check-driver-conformance.mjs | 59 +-- 6 files changed, 794 insertions(+), 56 deletions(-) create mode 100644 .changeset/mongodb-structured-groupby-and-count-distinct.md create mode 100644 packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts diff --git a/.changeset/mongodb-structured-groupby-and-count-distinct.md b/.changeset/mongodb-structured-groupby-and-count-distinct.md new file mode 100644 index 0000000000..80c0c13d43 --- /dev/null +++ b/.changeset/mongodb-structured-groupby-and-count-distinct.md @@ -0,0 +1,66 @@ +--- +"@objectstack/driver-mongodb": patch +"@objectstack/spec": patch +--- + +fix(driver-mongodb): take a structured `GroupByNode`, and answer `count` / +`count_distinct` the way every other backend does (#6850, part of #6814) + +`driver-mongodb` is now enrolled in the shared `AGGREGATION_CASES` standard +(`@objectstack/spec/data`), and clearing that cell fixed three divergences — all +three of the kind that ANSWER rather than fail, which is why none of them ever +surfaced as an error. + +**1. A structured `groupBy` node had no lowering at all (#6850).** +`GroupByNodeSchema` declares a union: a bare field name, or +`{ field, dateGranularity?, alias? }`. The pipeline builder annotated `groupBy` +as `string[]` and did `groupId[field] = '$' + field`, so a structured node — an +object in that loop — stringified: the `$group._id` key became the literal +`"[object Object]"` and its value the field path `"$[object Object]"`, which +matches nothing. The aggregation did not refuse and did not throw. It returned +rows grouped by a nonexistent path, under a column named `[object Object]`. +`MongoDBDriver.aggregate` passed the value through an `any` cast, which is why +the declared union never met that annotation at `tsc`. + +Both sides now spell the declared type, so the next drift between them is a +compile error. The `$group._id` keys on `alias ?? field` and its value is the +FIELD path — the projected column is renamed, the grouping does not move, which +is the rule #6401 converged the three SQL faces onto and the one +`in-memory-aggregation.ts` has always applied. The bare-string spelling emits +exactly what it emitted before. + +**2. `count_distinct` counted NULL as a distinct value (#6814).** The lowering +collects a `$addToSet` and sizes it; `$addToSet` keeps an explicit `null`, so a +nullable column answered one HIGHER than `COUNT(DISTINCT col)` — 3 where the +standard says 2. The sizing now excludes null, which is what +`COUNT(DISTINCT col)` computes on SQLite, PostgreSQL and MySQL alike and what +`objectql`'s in-memory fallback already computed. + +**3. `count(col)` counted ROWS, not values.** Measured while writing the suite +and named by neither issue: the `count` arm ignored `field` entirely and emitted +`{ $sum: 1 }` for both spellings, so `count(stage)` came back 6 — the answer +`count(*)` already has — where the standard says 4. `count(col)` now counts +non-null values, and a missing field is counted as null, the SQL reading. + +**A `dateGranularity` node is now REFUSED rather than silently ignored**, with +`NOT_IMPLEMENTED` / 501 in the ADR-0112 envelope — the same refusal, first +sentence for first sentence, that `driver-sql` and `driver-turso`'s remote +transport give for a granularity they cannot bucket (#6212). This driver +publishes no `supports.queryDateGranularity`, so the engine buckets every +granularity in memory and never pushes a bucketed node down; the refusal fires +only for a caller that reached the builder directly, which previously got a +`"[object Object]"` grouping instead. A native `$dateTrunc` lowering is +buildable and is not ruled out — it needs the engine's bucket LABELS, a +published capability record and `date-bucket-parity.test.ts`, so it is its own +change. A `groupBy` entry that is neither half of the union is refused with +`INVALID_QUERY` / 400. + +The suite that holds all of this is server-free (`mongodb-aggregation- +translation.test.ts`): this package's real-mongod suites are opt-in since #5517, +so it drives the EMITTED pipeline through a strict in-process evaluator that +refuses every shape it does not model. It holds the lowering to the shared +table; it does not answer "does MongoDB agree?", which is a real-mongod half's +question and is recorded as still open on #6814. + +`driver-memory`'s half of #6814 is untouched — it remains under the #5499 +investment freeze, and its `AGGREGATION_CASES` DEBT row stands. diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts new file mode 100644 index 0000000000..65db00839a --- /dev/null +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts @@ -0,0 +1,491 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Aggregate-vocabulary conformance for `buildAggregationPipeline` + + * `postProcessAggregation` — the shared `AGGREGATION_CASES` standard, answered + * without a server (#6850, #6814). + * + * `mongodb-aggregation.ts` is an independent lowering of `QueryAST.aggregations` + * + `QueryAST.groupBy`: it shares no line of code with the SQL compiler, with + * Turso's remote transport, or with `objectql`'s in-memory fallback. Nothing + * held it to the standard in `@objectstack/spec/data`, and all three defects the + * DEBT row recorded were the kind that ANSWER rather than fail: + * + * 1. a structured `GroupByNode` stringified into a `"[object Object]"` + * `$group._id` — rows grouped by a field path that matches nothing, under + * a column of that name (#6850); + * 2. `count_distinct` sized a `$addToSet` that CONTAINS the explicit nulls, so + * a nullable column answered one higher than `COUNT(DISTINCT col)` (#6814); + * 3. measured here and named by neither card: `count(col)` ignored `field` + * entirely and answered the ROW count, so `count(stage)` came back 6 where + * the case-set says 4 — the number `count(*)` already has. + * + * ## Why this half must always run + * + * The real-mongod suites in this package are OPT-IN since #5517 (a ~123 MB + * binary download that made green runs exit 1), so a defect only a downloadable + * binary can catch is a defect nobody catches. `buildAggregationPipeline` and + * `postProcessAggregation` are pure functions, which is what makes this half + * possible at all — the same split as `mongodb-filter-logic-translation.test.ts` + * and the shape #6814's closing conditions asked for. + * + * ## Why there is a pipeline evaluator in here + * + * The builder's output is a pipeline and the shared cases are stated as VALUES, + * so something has to bridge the two. Pinning the emitted stages literally for + * every case would pin today's spelling rather than the semantics — and it is + * the semantics that were wrong: every one of the three defects above emitted a + * perfectly well-formed pipeline. + * + * So {@link runPipeline} executes the emitted stages over + * {@link AGGREGATION_ROWS} by MongoDB's documented semantics, and is + * deliberately **strict**: every stage, accumulator and expression it does not + * model is a thrown error, never a tolerated no-op. A stand-in more permissive + * than the real engine turns a suite into a green light for broken code. Its own + * discrimination is proved at the bottom of this file — each of the three + * pre-fix lowerings is replayed through it and must FAIL the case it broke — so + * "all green" cannot mean "the evaluator says yes to anything". + * + * What it deliberately does NOT answer: whether a real mongod agrees. `$cond` / + * `$ifNull` / `$addToSet` are modelled here from the documentation, not + * observed. That is the question the opt-in half would answer, and it is + * recorded as open rather than implied — this environment cannot fetch a mongod + * binary at all, and a suite nobody has executed is a claim, not a check. + */ + +import { describe, it, expect } from 'vitest'; +import { + AGGREGATION_CASES, + AGGREGATION_ROWS, + type AggregationCase, + type GroupByNode, +} from '@objectstack/spec/data'; +import { buildAggregationPipeline, postProcessAggregation, type AggregationInput } from './mongodb-aggregation.js'; + +/** The alias every case's measure is projected under — never a fixture column. */ +const MEASURE = 'measure'; + +// ── A deliberately strict reader of the emitted pipeline ──────────────────── + +/** Thrown for any shape this evaluator does not model — never swallowed. */ +class UnsupportedShape extends Error {} + +/** A field path that resolved to nothing. MongoDB distinguishes it from `null`. */ +const MISSING = Symbol('missing'); + +type Doc = Record; + +/** Resolve a `'$a.b'` field path against a document, or {@link MISSING}. */ +function resolvePath(doc: Doc, ref: string): unknown { + if (!ref.startsWith('$')) throw new UnsupportedShape(`not a field path: ${ref}`); + let current: unknown = doc; + for (const part of ref.slice(1).split('.')) { + if (current === null || typeof current !== 'object' || Array.isArray(current)) return MISSING; + if (!Object.prototype.hasOwnProperty.call(current, part)) return MISSING; + current = (current as Doc)[part]; + } + return current; +} + +/** + * Evaluate an aggregation EXPRESSION against one document. + * + * Models only the operators this builder emits. `$$`-prefixed system variables + * are refused rather than guessed at — `$$ROOT` reaches the `array_agg` arm, and + * an evaluator that quietly resolved it would be asserting a semantics nobody + * checked. + */ +function evalExpr(doc: Doc, expr: unknown): unknown { + if (typeof expr === 'string' && expr.startsWith('$')) { + if (expr.startsWith('$$')) throw new UnsupportedShape(`system variable '${expr}' is not modelled`); + return resolvePath(doc, expr); + } + if (expr === null || typeof expr !== 'object') return expr; // literal + if (Array.isArray(expr)) return expr.map((e) => evalExpr(doc, e)); + + const keys = Object.keys(expr as Doc); + if (keys.length !== 1) throw new UnsupportedShape(`expression with ${keys.length} keys: ${keys.join(', ')}`); + const [op] = keys; + const arg = (expr as Doc)[op]; + + switch (op) { + case '$ifNull': { + if (!Array.isArray(arg) || arg.length !== 2) throw new UnsupportedShape('$ifNull takes [expr, replacement]'); + const value = evalExpr(doc, arg[0]); + return value === MISSING || value === null ? evalExpr(doc, arg[1]) : value; + } + case '$eq': { + if (!Array.isArray(arg) || arg.length !== 2) throw new UnsupportedShape('$eq takes two operands'); + // MISSING and null compare EQUAL under `$eq` in the aggregation language, + // which is why the `count` lowering routes through `$ifNull` first rather + // than relying on it. + const left = evalExpr(doc, arg[0]); + const right = evalExpr(doc, arg[1]); + const norm = (v: unknown) => (v === MISSING ? null : v); + return norm(left) === norm(right); + } + case '$cond': { + if (!Array.isArray(arg) || arg.length !== 3) throw new UnsupportedShape('$cond takes [if, then, else]'); + return evalExpr(doc, arg[0]) ? evalExpr(doc, arg[1]) : evalExpr(doc, arg[2]); + } + default: + throw new UnsupportedShape(`unsupported aggregation expression operator '${op}'`); + } +} + +/** One accumulator, folded over the documents of a single group. */ +function accumulate(rows: Doc[], acc: unknown): unknown { + if (acc === null || typeof acc !== 'object' || Array.isArray(acc)) { + throw new UnsupportedShape(`accumulator must be a one-key document, got ${JSON.stringify(acc)}`); + } + const keys = Object.keys(acc as Doc); + if (keys.length !== 1) throw new UnsupportedShape(`accumulator with ${keys.length} keys`); + const [op] = keys; + const arg = (acc as Doc)[op]; + const values = rows.map((row) => evalExpr(row, arg)); + /** MongoDB's arithmetic accumulators ignore missing and non-numeric values. */ + const numbers = values.filter((v): v is number => typeof v === 'number'); + + switch (op) { + case '$sum': + return numbers.reduce((a, b) => a + b, 0); + case '$avg': + return numbers.length === 0 ? null : numbers.reduce((a, b) => a + b, 0) / numbers.length; + case '$min': + return numbers.length === 0 ? null : Math.min(...numbers); + case '$max': + return numbers.length === 0 ? null : Math.max(...numbers); + case '$addToSet': { + // `$addToSet` skips a MISSING field and keeps an explicit `null` — the + // whole of #6814 lives in that second half. + const set: unknown[] = []; + for (const v of values) { + if (v === MISSING) continue; + if (!set.includes(v)) set.push(v); + } + return set; + } + case '$push': + return values.filter((v) => v !== MISSING); + default: + throw new UnsupportedShape(`unsupported accumulator '${op}'`); + } +} + +/** Execute an emitted pipeline over the fixture. Throws on any shape not modelled. */ +export function runPipeline(rows: readonly Doc[], pipeline: readonly Doc[]): Doc[] { + let docs: Doc[] = rows.map((row) => ({ ...row })); + + for (const stage of pipeline) { + const keys = Object.keys(stage); + if (keys.length !== 1) throw new UnsupportedShape(`pipeline stage with ${keys.length} keys`); + const [name] = keys; + const spec = stage[name]; + + switch (name) { + case '$group': { + const { _id: idSpec, ...accumulators } = spec as Doc; + const groups = new Map(); + for (const doc of docs) { + let id: unknown; + if (idSpec === null) { + id = null; + } else if (idSpec && typeof idSpec === 'object' && !Array.isArray(idSpec)) { + const key: Doc = {}; + for (const [outKey, ref] of Object.entries(idSpec as Doc)) { + const value = evalExpr(doc, ref); + key[outKey] = value === MISSING ? null : value; + } + id = key; + } else { + throw new UnsupportedShape(`$group._id must be null or a document, got ${JSON.stringify(idSpec)}`); + } + const bucket = JSON.stringify(id); + if (!groups.has(bucket)) groups.set(bucket, { id, rows: [] }); + groups.get(bucket)!.rows.push(doc); + } + docs = [...groups.values()].map(({ id, rows: groupRows }) => { + const out: Doc = { _id: id }; + for (const [alias, acc] of Object.entries(accumulators)) { + out[alias] = accumulate(groupRows, acc); + } + return out; + }); + break; + } + case '$project': { + docs = docs.map((doc) => { + const out: Doc = {}; + for (const [key, rule] of Object.entries(spec as Doc)) { + if (rule === 0 || rule === false) continue; + if (rule === 1 || rule === true) { + const value = resolvePath(doc, `$${key}`); + if (value !== MISSING) out[key] = value; + continue; + } + if (typeof rule === 'string') { + const value = resolvePath(doc, rule); + if (value !== MISSING) out[key] = value; + continue; + } + throw new UnsupportedShape(`unsupported $project rule for '${key}': ${JSON.stringify(rule)}`); + } + return out; + }); + break; + } + case '$match': + // Reachable only from `opts.where`, which no case in the shared set + // spells. Refused rather than approximated: this file's `$match` would + // be a second, weaker copy of the matcher + // `mongodb-filter-logic-translation.test.ts` already owns. + throw new UnsupportedShape('$match is not modelled here — see mongodb-filter-logic-translation.test.ts'); + case '$sort': { + const entries = Object.entries(spec as Doc); + docs = [...docs].sort((a, b) => { + for (const [field, dir] of entries) { + if (dir !== 1 && dir !== -1) throw new UnsupportedShape(`$sort direction ${String(dir)}`); + const av = a[field]; + const bv = b[field]; + if (av === bv) continue; + return ((av as never) < (bv as never) ? -1 : 1) * (dir as number); + } + return 0; + }); + break; + } + case '$skip': + docs = docs.slice(spec as number); + break; + case '$limit': + docs = docs.slice(0, spec as number); + break; + default: + throw new UnsupportedShape(`unsupported pipeline stage '${name}'`); + } + } + + return docs; +} + +// ── Driving the shared case-set ───────────────────────────────────────────── + +/** How a case's single `groupBy` column is spelled on the wire. */ +type GroupBySpelling = 'bare' | 'structured'; + +function groupByFor(c: AggregationCase, spelling: GroupBySpelling): GroupByNode[] | undefined { + if (!c.groupBy) return undefined; + if (c.groupByAlias) return [{ field: c.groupBy, alias: c.groupByAlias }]; + return spelling === 'bare' ? [c.groupBy] : [{ field: c.groupBy }]; +} + +/** Run one case end to end and return its answers, ascending by group. */ +function answer(c: AggregationCase, spelling: GroupBySpelling = 'bare'): Array<{ group: string | null; value: unknown }> { + const aggregations: AggregationInput[] = [{ function: c.function, field: c.field, alias: MEASURE }]; + const pipeline = buildAggregationPipeline({ aggregations, groupBy: groupByFor(c, spelling) }); + const rows = postProcessAggregation(runPipeline(AGGREGATION_ROWS as unknown as Doc[], pipeline), aggregations); + + // The group value is read from `groupByAlias ?? groupBy` — the rule the + // case-set states, and the one that makes the alias cases fail on a face that + // ignores `alias` instead of passing on the numbers. + const outKey = c.groupByAlias ?? c.groupBy; + return rows + .map((row) => ({ + group: outKey === undefined ? null : (row[outKey] as string | null), + value: row[MEASURE], + })) + .sort((a, b) => String(a.group).localeCompare(String(b.group))); +} + +describe('driver-mongodb — the aggregate vocabulary, without a server (#6850/#6814)', () => { + for (const c of AGGREGATION_CASES) { + it(c.name, () => { + expect(answer(c), c.note ?? '').toEqual([...c.expected]); + }); + } +}); + +describe('a PLAIN structured groupBy node answers exactly like the bare field name', () => { + // The half of the union that no capability bit guards: `{ field: 'region' }` + // with no granularity is pushed down to every driver (#6212's finding on the + // Turso remote transport), so it has to mean what `'region'` means. Before + // #6850 it meant `"[object Object]"` here. + for (const c of AGGREGATION_CASES.filter((x) => x.groupBy && !x.groupByAlias)) { + it(c.name, () => { + expect(answer(c, 'structured')).toEqual([...c.expected]); + }); + } +}); + +// ── The wire shapes the values cannot show ────────────────────────────────── + +describe('the emitted pipeline', () => { + const countAgg: AggregationInput[] = [{ function: 'count', alias: 'n' }]; + + it('keys $group._id on the ALIAS and its value on the FIELD, and projects the alias', () => { + expect(buildAggregationPipeline({ aggregations: countAgg, groupBy: [{ field: 'region', alias: 'bucket' }] })) + .toEqual([ + { $group: { _id: { bucket: '$region' }, n: { $sum: 1 } } }, + { $project: { _id: 0, bucket: '$_id.bucket', n: 1 } }, + ]); + }); + + it('leaves the bare-string spelling exactly as it was', () => { + expect(buildAggregationPipeline({ aggregations: countAgg, groupBy: ['region'] })).toEqual([ + { $group: { _id: { region: '$region' }, n: { $sum: 1 } } }, + { $project: { _id: 0, region: '$_id.region', n: 1 } }, + ]); + }); + + it('an alias equal to the field name is a no-op, not a second column', () => { + expect(buildAggregationPipeline({ aggregations: countAgg, groupBy: [{ field: 'region', alias: 'region' }] })) + .toEqual(buildAggregationPipeline({ aggregations: countAgg, groupBy: ['region'] })); + }); + + it('never emits "[object Object]" anywhere, for any spelling of the union', () => { + const spellings: GroupByNode[][] = [ + ['region'], + [{ field: 'region' }], + [{ field: 'region', alias: 'bucket' }], + ['region', { field: 'stage', alias: 'phase' }], + ]; + for (const groupBy of spellings) { + const emitted = JSON.stringify(buildAggregationPipeline({ aggregations: countAgg, groupBy })); + expect(emitted, `groupBy: ${JSON.stringify(groupBy)}`).not.toContain('[object Object]'); + } + }); + + it('counts NON-NULL values for count(col) and rows for count(*)', () => { + // The lowering, stated literally: the two spellings of `count` are different + // expressions, which is what defect 3 above collapsed. + expect(buildAggregationPipeline({ aggregations: [{ function: 'count', alias: 'n' }] })[0]) + .toEqual({ $group: { _id: null, n: { $sum: 1 } } }); + expect(buildAggregationPipeline({ aggregations: [{ function: 'count', field: 'stage', alias: 'n' }] })[0]) + .toEqual({ + $group: { + _id: null, + n: { $sum: { $cond: [{ $eq: [{ $ifNull: ['$stage', null] }, null] }, 0, 1] } }, + }, + }); + }); +}); + +// ── Refusals carry the ADR-0112 envelope ──────────────────────────────────── + +describe('a groupBy entry with no lowering is REFUSED, not ignored', () => { + const aggregations: AggregationInput[] = [{ function: 'count', alias: 'n' }]; + + it('a dateGranularity node answers NOT_IMPLEMENTED / 501', () => { + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + buildAggregationPipeline({ aggregations, groupBy: [{ field: 'closed_at', dateGranularity: 'month' }] }); + } catch (err) { + thrown = err as Error & { code?: string; status?: number }; + } + expect(thrown, 'a granularity this backend cannot bucket must not be silently dropped').toBeDefined(); + expect(thrown!.code).toBe('NOT_IMPLEMENTED'); + expect(thrown!.status).toBe(501); + expect(thrown!.message).toMatch(/Date bucketing by 'month' is not supported/); + expect(thrown!.message).toMatch(/queryDateGranularity/); + }); + + it('refuses every declared granularity, so none is half-supported', () => { + for (const g of ['day', 'week', 'month', 'quarter', 'year'] as const) { + expect(() => buildAggregationPipeline({ aggregations, groupBy: [{ field: 'closed_at', dateGranularity: g }] })) + .toThrow(new RegExp(`Date bucketing by '${g}'`)); + } + }); + + it('an entry that is neither half of the union answers INVALID_QUERY / 400', () => { + for (const node of [{}, { field: '' }, 42, null] as unknown[]) { + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + buildAggregationPipeline({ aggregations, groupBy: [node as GroupByNode] }); + } catch (err) { + thrown = err as Error & { code?: string; status?: number }; + } + expect(thrown, `groupBy entry ${JSON.stringify(node)} must be refused`).toBeDefined(); + expect(thrown!.code).toBe('INVALID_QUERY'); + expect(thrown!.status).toBe(400); + expect(thrown!.message).toMatch(/GroupByNodeSchema/); + } + }); + + it('refuses before a pipeline exists, so no stage is built from a half-read groupBy', () => { + expect(() => buildAggregationPipeline({ + aggregations, + groupBy: ['region', { field: 'closed_at', dateGranularity: 'year' }], + })).toThrow(/Date bucketing/); + }); +}); + +// ── The evaluator is not the thing being tested, so it gets tested ────────── + +describe('the in-process pipeline evaluator discriminates', () => { + const rows = AGGREGATION_ROWS as unknown as Doc[]; + + it('the pre-#6850 $group._id FAILS the alias case it broke', () => { + // What `groupId[field] = '$' + field` emitted for `{ field: 'region', + // alias: 'bucket' }`: the key is the stringified object and the value a + // field path that matches nothing. This is the finding's own description, + // executed — all six rows collapse into ONE group under a column literally + // named `[object Object]`, holding null, and nothing is projected under + // `bucket` at all. It does not throw; it answers. + const broken = runPipeline(rows, [ + { $group: { _id: { '[object Object]': '$[object Object]' }, n: { $sum: 1 } } }, + { $project: { _id: 0, '[object Object]': '$_id.[object Object]', n: 1 } }, + ]); + expect(broken).toEqual([{ '[object Object]': null, n: 6 }]); + expect(broken.every((row) => row.bucket === undefined)).toBe(true); + // The two group counts the alias case requires (east 2, west 4) are not + // merely mis-keyed here — they do not exist. + expect(broken).toHaveLength(1); + }); + + it('the pre-#6814 count_distinct sizing FAILS the null case it broke', () => { + const set = runPipeline(rows, [{ $group: { _id: null, n: { $addToSet: '$stage' } } }])[0].n as unknown[]; + // The evaluator keeps the explicit null, which is what MongoDB does — so + // the old `.length` answered 3 and the standard says 2. + expect(set).toContain(null); + expect(set.length).toBe(3); + expect(set.filter((v) => v != null).length).toBe(2); + }); + + it('the pre-#6814 count(col) lowering FAILS the case it broke', () => { + // `{ $sum: 1 }` regardless of `field`: the row count, not the value count. + expect(runPipeline(rows, [{ $group: { _id: null, n: { $sum: 1 } } }])[0].n).toBe(6); + expect(answer(AGGREGATION_CASES.find((c) => c.name.startsWith('count(stage) counts'))!)).toEqual([ + { group: null, value: 4 }, + ]); + }); + + it('computes per GROUP rather than over the whole fixture', () => { + const grouped = runPipeline(rows, [ + { $group: { _id: { region: '$region' }, n: { $sum: '$score' } } }, + { $project: { _id: 0, region: '$_id.region', n: 1 } }, + ]); + expect(grouped).toEqual([{ region: 'west', n: 100 }, { region: 'east', n: 110 }]); + }); + + it('refuses any stage, accumulator or expression it does not model', () => { + expect(() => runPipeline(rows, [{ $lookup: {} }])).toThrow(/unsupported pipeline stage/); + expect(() => runPipeline(rows, [{ $group: { _id: null, n: { $stdDevPop: '$score' } } }])) + .toThrow(/unsupported accumulator/); + expect(() => runPipeline(rows, [{ $group: { _id: null, n: { $sum: { $abs: '$score' } } } }])) + .toThrow(/unsupported aggregation expression operator/); + expect(() => runPipeline(rows, [{ $group: { _id: '$region', n: { $sum: 1 } } }])) + .toThrow(/\$group\._id must be null or a document/); + expect(() => runPipeline(rows, [{ $match: { region: 'west' } }])).toThrow(/\$match is not modelled/); + }); + + it('distinguishes a MISSING field from an explicit null, like MongoDB does', () => { + const sparse: Doc[] = [{ id: '1', stage: null }, { id: '2' }, { id: '3', stage: 'won' }]; + const set = runPipeline(sparse, [{ $group: { _id: null, n: { $addToSet: '$stage' } } }])[0].n as unknown[]; + expect(set).toEqual([null, 'won']); + // …and the `count` lowering counts neither of them. + const counted = runPipeline(sparse, [{ + $group: { _id: null, n: { $sum: { $cond: [{ $eq: [{ $ifNull: ['$stage', null] }, null] }, 0, 1] } } }, + }])[0].n; + expect(counted).toBe(1); + }); +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts index 813d7404ea..727ccfd253 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts @@ -8,6 +8,8 @@ */ import type { Document } from 'mongodb'; +import { StandardErrorCode } from '@objectstack/spec/api'; +import type { GroupByNode } from '@objectstack/spec/data'; import { translateFilter } from './mongodb-filter.js'; import type { TemporalFieldKindResolver } from './mongodb-temporal.js'; @@ -22,6 +24,110 @@ export interface AggregationInput { filter?: unknown; } +/** + * One `groupBy` entry after the declared union has been read: the field the + * `$group._id` keys on, and the name the group value is PROJECTED under. + * + * [#6850] The two are separate because `GroupByNodeSchema.alias` renames the + * projection without moving the grouping — the rule #6401 converged the three + * SQL faces onto (`alias ?? field`), and the one `in-memory-aggregation.ts` has + * always applied. + */ +interface GroupByTarget { + field: string; + outKey: string; +} + +/** + * [#6850] Read the declared `GroupByNode` union — a bare field name, or a + * structured `{ field, dateGranularity?, alias? }` node. + * + * Before this, `groupBy` was annotated `string[]` and every entry went straight + * into `groupId[field] = '$' + field`. A structured node is an OBJECT there, so + * the `$group._id` key became the literal `"[object Object]"` and its value the + * field path `"$[object Object]"`, which matches nothing: the aggregation did + * not refuse and did not throw, it ANSWERED — rows grouped by a nonexistent + * path, under a column named `[object Object]`. (`mongodb-driver.ts` passed the + * value through an `any` cast, which is why the declared union never met that + * `string[]` annotation at `tsc`.) `driver-turso`'s remote transport carried the + * same shape and at least died loudly on it (#6212); this face answered. + */ +function normalizeGroupBy(nodes: readonly GroupByNode[]): GroupByTarget[] { + return nodes.map((node) => { + if (typeof node === 'string') return { field: node, outKey: node }; + if (node && typeof node === 'object' && typeof node.field === 'string' && node.field !== '') { + // A DATE-BUCKETED node has no lowering here — refuse it rather than group + // by the raw instant, which would answer one bucket per distinct + // timestamp and look like a working query. + if (node.dateGranularity) refuseDateBucketedGroupBy(node.dateGranularity); + return { field: node.field, outKey: node.alias ?? node.field }; + } + throw malformedGroupByError(node); + }); +} + +/** + * [#6850] A `groupBy` entry asks for a date BUCKET — the twin of `driver-sql`'s + * and `driver-turso`'s `refuseDateBucketedGroupBy`, first sentence for first + * sentence, and the same NOT_IMPLEMENTED/501 class for the same reason (#5907, + * #6212, ADR-0112): `DateGranularity` declares the name, this backend emits no + * bucket expression for it, so it is a capability gap in the backend rather than + * a mistake in the query. + * + * This driver buckets NOTHING natively, which is exactly what it publishes: + * `MongoDBDriver.supports` carries no `queryDateGranularity` key at all (see the + * comment on that field), so the engine buckets every granularity in memory and + * never pushes a bucketed item down here. The refusal therefore fires only for a + * caller that went around the capability bit and reached the builder directly, + * which is the caller this message is written for. + * + * A native lowering is buildable — MongoDB has `$dateTrunc` — and this refusal + * is not a verdict that it cannot be. It is not a one-liner either: the engine's + * fallback produces LABELS (`'2026-01'`, `'2026-Q1'`, ISO `'2026-W03'` — see + * `bucketDateValue` and `SqlDriver.buildDateBucketExpr`), so a pushdown here has + * to emit those strings, publish `supports.queryDateGranularity`, and be held to + * `date-bucket-parity.test.ts`. Until then, refusing is what keeps a declared + * key from being silently ignored. + */ +function refuseDateBucketedGroupBy(granularity: string): never { + const err = new Error( + `Date bucketing by '${granularity}' is not supported by this backend. ` + + `Bucketed here: none (driver-mongodb). ` + + `The query is spelled correctly and @objectstack/spec DateGranularity declares it — this is ` + + `a capability gap in the backend, not a mistake in the query, which is why it answers ` + + `NOT_IMPLEMENTED/501 rather than a 400. A driver publishes the granularities it buckets ` + + `natively as \`supports.queryDateGranularity\`; the engine reads that record and buckets ` + + `in memory for every granularity absent from it, which is always correct (#6212). This ` + + `driver publishes none, so a bucketed groupBy reaches here only when a caller goes around ` + + `the engine (#6850).`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + throw err; +} + +/** + * [#6850] A `groupBy` entry that is neither half of the declared union. + * + * INVALID_QUERY/400 rather than 501: unlike a date bucket this is not a + * capability gap — `GroupByNodeSchema` declares a field NAME or an object + * carrying a `field`, and nothing else has a meaning to lower. Refused rather + * than skipped because dropping a grouping target silently changes which rows + * share a group. + */ +function malformedGroupByError(node: unknown): Error { + const err = new Error( + `groupBy entry ${JSON.stringify(node) ?? String(node)} is not a grouping target. ` + + `@objectstack/spec GroupByNodeSchema declares each entry as either a field NAME ` + + `('region') or an object with a non-empty 'field' ({ field: 'closed_at', dateGranularity?, ` + + `alias? }). It is refused rather than skipped because dropping a grouping target silently ` + + `changes which rows share a group.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.INVALID_QUERY; + err.status = 400; + return err; +} + /** * Build a MongoDB aggregation pipeline from QueryAST components. * @@ -35,7 +141,11 @@ export interface AggregationInput { export function buildAggregationPipeline(opts: { where?: unknown; aggregations?: AggregationInput[]; - groupBy?: string[]; + /** + * [#6850] `GroupByNode[]` — the spec's own union, not the `string[]` + * restatement that had drifted from it. See {@link normalizeGroupBy}. + */ + groupBy?: readonly GroupByNode[]; orderBy?: Array<{ field: string; order?: string }>; limit?: number; offset?: number; @@ -57,16 +167,21 @@ export function buildAggregationPipeline(opts: { } } + // [#6850] Read the declared union ONCE, before any stage is built, so the + // `$group` keys and the `$project` that flattens them cannot disagree about + // what a node means — and so a refusal happens before a pipeline exists. + const groupTargets = normalizeGroupBy(opts.groupBy ?? []); + // $group stage if (opts.aggregations && opts.aggregations.length > 0) { const groupId: Document = {}; const groupAccumulators: Document = {}; - // Build _id from groupBy fields - if (opts.groupBy && opts.groupBy.length > 0) { - for (const field of opts.groupBy) { - groupId[field] = `$${field}`; - } + // Build _id from groupBy fields. The _id key is the PROJECTED name and its + // value the FIELD path, so an alias renames the output column without + // moving the grouping — `alias ?? field`, the #6401 rule. + for (const { field, outKey } of groupTargets) { + groupId[outKey] = `$${field}`; } // Build accumulators from aggregation descriptors @@ -82,10 +197,10 @@ export function buildAggregationPipeline(opts: { }); // $project stage to flatten _id fields back to top level - if (opts.groupBy && opts.groupBy.length > 0) { + if (groupTargets.length > 0) { const project: Document = { _id: 0 }; - for (const field of opts.groupBy) { - project[field] = `$_id.${field}`; + for (const { outKey } of groupTargets) { + project[outKey] = `$_id.${outKey}`; } for (const agg of opts.aggregations) { project[agg.alias] = 1; @@ -122,7 +237,20 @@ function buildAccumulator(agg: AggregationInput): Document { switch (agg.function) { case 'count': - return { $sum: 1 }; + // [#6814] `count(*)` counts ROWS; `count(col)` counts NON-NULL VALUES of + // that column — what `COUNT(col)` does on every SQL dialect, what the + // in-memory fallback does, and what `AGGREGATION_CASES` says (4 over + // `AGGREGATION_ROWS`, beside `count(*)`'s 6 and `count_distinct`'s 2: + // three different numbers over one column, on purpose). This arm ignored + // `field` entirely and answered the row count for both spellings, so + // `count(stage)` came back 6 here and 4 on the SQL family. + // + // `$ifNull` maps a MISSING field to null as well, so an absent key and an + // explicit null are counted alike — the SQL reading, where an absent + // value is NULL and there is no third state. + return fieldRef === null + ? { $sum: 1 } + : { $sum: { $cond: [{ $eq: [{ $ifNull: [fieldRef, null] }, null] }, 0, 1] } }; case 'sum': return { $sum: fieldRef ?? 0 }; @@ -137,8 +265,9 @@ function buildAccumulator(agg: AggregationInput): Document { return { $max: fieldRef ?? 0 }; case 'count_distinct': - // Use $addToSet to collect unique values; a subsequent $project - // can use $size to get the count. We store the set here. + // Collect the distinct values here; {@link postProcessAggregation} sizes + // the set, EXCLUDING null — see the note there for why the exclusion is + // on that side rather than in this expression. return { $addToSet: fieldRef ?? null }; case 'array_agg': @@ -158,6 +287,35 @@ function buildAccumulator(agg: AggregationInput): Document { * * Handles count_distinct conversion ($addToSet → count) and * string_agg conversion ($push → joined string). + * + * ## [#6814] Why the null exclusion is HERE + * + * `count_distinct` is distinct NON-NULL values of the column — what + * `COUNT(DISTINCT col)` computes on SQLite, PostgreSQL and MySQL alike, what + * `in-memory-aggregation.ts` computes (`new Set(values.filter(v => v != null)).size`), + * and what `AGGREGATION_CASES` says (2 over `AGGREGATION_ROWS`). `$addToSet` + * adds an explicit `null` to the set, so sizing the array as it arrived answered + * one HIGHER on any nullable column — 3 where the standard says 2. (`$addToSet` + * on a MISSING field adds nothing, so the divergence showed only for an + * explicitly-null value, which is exactly what a nullable column produces.) + * + * The two server-side spellings the finding sketched were measured against this + * one and not taken: + * + * - **`$ne: null` before the `$addToSet`** — as a `$match` it drops the row from + * the WHOLE pipeline, so a `count(*)` or `sum()` sharing it would silently + * lose the null rows too. Correct only for a pipeline carrying nothing else, + * which is not a shape this builder can assume. + * - **`$size` of a `$setDifference` against `[null]`** — sound, and it would + * size server-side rather than shipping the array; it needs a `$project` stage + * this builder does not emit when there is no `groupBy`, so it is a shape + * change to the pipeline that no suite here can execute (the real-mongod + * suites are opt-in since #5517). Worth doing when this cell gains a live + * half; it would make this function's `Array.isArray` guard fall through + * harmlessly on the already-sized value. + * + * Excluding it here is exact, needs no server semantics to be true, and is + * pinned directly by `mongodb-aggregation-translation.test.ts`. */ export function postProcessAggregation( results: Document[], @@ -179,7 +337,9 @@ export function postProcessAggregation( const processed = { ...row }; for (const field of countDistinctFields) { if (Array.isArray(processed[field])) { - processed[field] = processed[field].length; + // `!= null` on purpose: it takes `undefined` with it, which is what a + // set built from a field some documents do not carry can hold. + processed[field] = processed[field].filter((v: unknown) => v != null).length; } } for (const field of stringAggFields) { diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.ts index 454891bcb7..c0c18f13ef 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.ts @@ -509,7 +509,18 @@ export class MongoDBDriver implements IDataDriver { const pipeline = buildAggregationPipeline({ where: query.where, aggregations, - groupBy: (query as any).groupBy, + // [#6850] Was `(query as any).groupBy`. `DriverQuery` declares this key as + // `GroupByNode[]` — a union of a bare field name and a structured + // `{ field, dateGranularity?, alias? }` node — and this cast is what kept + // the declaration from ever meeting the builder's `string[]` annotation at + // `tsc`, so a structured node stringified into a `"[object Object]"` + // `$group._id` rather than failing to compile. Both sides now spell the + // declared type, so the next drift between them is a type error. + // + // The `aggregations` read above keeps its cast for now: it also carries + // the undeclared `query.aggregate` limb that #6321 deleted from the SQL + // faces, which is a separate convergence and not this card. + groupBy: query.groupBy, orderBy: query.orderBy as Array<{ field: string; order?: string }>, limit: query.limit, offset: query.offset, diff --git a/packages/spec/src/data/aggregation-conformance.ts b/packages/spec/src/data/aggregation-conformance.ts index eac3aac314..8fe6dad936 100644 --- a/packages/spec/src/data/aggregation-conformance.ts +++ b/packages/spec/src/data/aggregation-conformance.ts @@ -66,7 +66,9 @@ * non-null values in PostgreSQL and MySQL alike. A backend that counted NULL as * a distinct value would answer `3` where {@link AGGREGATION_CASES} says `2`, * and that is not a theoretical failure mode: it is what `driver-mongodb`'s - * `$addToSet` → `$size` lowering does today (see the DEBT list below). + * `$addToSet` → `$size` lowering did until #6850/#6814 enrolled it — measured by + * running this table against the emitted pipeline, not predicted (see the DEBT + * list below, where `driver-memory` still carries the open half). * * The `count` cases sit beside them on purpose. `count(stage)` is `4` while * `count_distinct(stage)` is `2` and `count(*)` is `6`: three different numbers @@ -84,6 +86,15 @@ * - **`driver-sqlite-wasm`** — `sqlite-wasm-aggregation-conformance.test.ts`. It * inherits `SqlDriver`'s compiler, so what it re-checks is the sql.js dialect * the statement then has to survive, not a second lowering. + * - **`driver-mongodb`** — `mongodb-aggregation-translation.test.ts` [#6850/#6814]. + * The one enrolled face with NO engine behind it: this package's real-mongod + * suites are opt-in since #5517, so the suite drives the pipeline + * `buildAggregationPipeline` EMITS through a strict in-process evaluator of + * the stages it emits, and refuses every shape it does not model. That bounds + * what the cell claims — it holds the LOWERING to the table, and it does not + * answer "does MongoDB agree?", which is the question a real-mongod half would + * own. Recorded here rather than left to be discovered, because a green cell + * reads as more than that. * - **`objectql`'s in-memory fallback** — `in-memory-aggregation-conformance.test.ts` * in `packages/objectql`. [#6401] Not a SQL face and not a driver, which is * why #6409 left it out; enrolled here because it is the face that has always @@ -109,15 +120,23 @@ * |---|---|---| * | `driver-memory` (data face) | **RED** — answers `null` | `MemoryDriver.computeAggregate` has no `count_distinct` arm; the `switch` falls to `default: return null`, so the aggregation resolves with no value and no error. | * | `driver-memory` (analytics face) | **agrees** | `memory-analytics.ts` collects `$addToSet` and sizes it — the same NULL question as MongoDB below; not executed against this table. | - * | `driver-mongodb` | **RED** — counts NULL | `count_distinct` lowers to `$addToSet` and `postProcessAggregation` takes the array's `.length`, so an explicit `null` is one of the distinct values. Read from the source; not executed. | + * | ~~`driver-mongodb`~~ | **CLEARED** [#6850/#6814] | Was RED and under-stated: the `count_distinct` null (3 for the standard's 2), the `"[object Object]"` `$group._id` below, AND a third divergence neither row named — `count(col)` ignored `field` and answered the ROW count (6 for the standard's 4). All three fixed and enrolled; see the list above. | * | `driver-memory` — the #6401 alias cases | **agrees** | `MemoryDriver.performAggregation`'s `normalizeGroupBy` (`memory-driver.ts:1066-1068`) already returns `{ field, alias: node.alias ?? node.field }` and projects the group value under `alias`. It reached the enforce answer independently, so the alias leg needed NO mechanical alignment here — measured, not assumed. | - * | `driver-mongodb` — the #6401 alias cases | **RED** — and wider than alias | `buildAggregationPipeline` types `groupBy` as `string[]` and does `groupId[field] = '$' + field` (`mongodb-aggregation.ts:66-69`, mirrored in the `$project` at `:85-88`). A STRUCTURED node — with or without an alias — is an object there, so the `$group._id` key becomes the literal `"[object Object]"` and its value `"$[object Object]"`. The alias is not so much ignored as unreachable: this face cannot take a structured `GroupByNode` at all. `mongodb-driver.ts:512` passes `(query as any).groupBy`, which is why the declared union never met the `string[]` annotation at `tsc`. Read from the source; not executed. | + * | ~~`driver-mongodb` — the #6401 alias cases~~ | **CLEARED** [#6850] | Was RED and wider than the alias: `buildAggregationPipeline` typed `groupBy` as `string[]` and did `groupId[field] = '$' + field`, so a STRUCTURED node — aliased or not — stringified into a `"[object Object]"` `$group._id` keyed on a field path that matches nothing. The alias was unreachable rather than ignored. It now reads the union, keys `_id` on `alias ?? field`, and refuses a `dateGranularity` node with NOT_IMPLEMENTED/501 rather than dropping a declared key; `mongodb-driver.ts` spells the declared type instead of `(query as any).groupBy`, so the next drift is a `tsc` error. | * - * Both packages are inside the **#5499 investment freeze**, which is why these - * are DEBT rows and not fixes: #6409's ruling put them explicitly out of scope - * and left their partial implementations untouched. Enrolling either means - * lifting the freeze for it first — the row is here so that decision is made - * against a measured verdict instead of an assumption that they already agree. + * `driver-memory` is inside the **#5499 investment freeze**, which is why its + * row is a DEBT row and not a fix: #6409's ruling put it explicitly out of scope + * and left its partial implementation untouched. Enrolling it means lifting the + * freeze for it first — the row is here so that decision is made against a + * measured verdict instead of an assumption that it already agrees. The + * maintainer lifted the freeze for `driver-mongodb` alone on 2026-08-11, which + * is why the two rows above are struck through and this one is not. + * + * What the strike-throughs are worth keeping for: every one of those verdicts + * was reached by READING, and when the suite finally executed the case-set it + * found a divergence none of the readings had (`count(col)`). The rows were + * right about what they measured and incomplete about what they had not — which + * is the argument for the suite rather than against the ledger. * * `objectql`'s in-memory fallback (`in-memory-aggregation.ts`) is a fourth * lowering and is NOT frozen: it computes `count_distinct` as diff --git a/scripts/check-driver-conformance.mjs b/scripts/check-driver-conformance.mjs index 923f1ee307..5ef431853f 100644 --- a/scripts/check-driver-conformance.mjs +++ b/scripts/check-driver-conformance.mjs @@ -340,7 +340,7 @@ const CASE_SETS = [ // refusal face — objectql's `having` (`having-filter.ts`) — was outside // that scope and kept its bare `new Error` until #7047. // -// ## AGGREGATION_CASES: two DEBT rows on arrival, and they are the same pair +// ## AGGREGATION_CASES: two DEBT rows on arrival, and they were the same pair // // The column arrived with #6409, which lowered `count_distinct` to // `COUNT(DISTINCT x)` on the SQL family — the ENFORCE leg of #6188's split @@ -350,14 +350,30 @@ const CASE_SETS = [ // whose suite pins the inherited statement surviving a different ENGINE, the // same judgement #4405 recorded for its filter-logic cell. // -// The two open cells are `driver-memory` and `driver-mongodb` — the #5499 frozen -// family, and open by that decision rather than by difficulty. #6409's ruling -// put both explicitly out of scope and left their partial implementations -// untouched, so the rows below record what each ANSWERS today, read from the -// source on this branch. Neither is a prediction, and neither is a permission -// slip: the cell clears when a suite runs the case-set, not when someone argues -// the driver would pass it. Both would go RED as they stand, which is the -// reason the rows exist rather than a reason to omit them. +// The two open cells were `driver-memory` and `driver-mongodb` — the #5499 +// frozen family, and open by that decision rather than by difficulty. #6409's +// ruling put both explicitly out of scope and left their partial +// implementations untouched, so the rows recorded what each ANSWERS, read from +// the source. Neither was a prediction, and neither was a permission slip: the +// cell clears when a suite runs the case-set, not when someone argues the +// driver would pass it. +// +// [#6850/#6814] `driver-mongodb`'s cell is now CLEARED, and its row is gone +// with the suite that replaced it (`mongodb-aggregation-translation.test.ts`) +// — the maintainer unfroze this package on 2026-08-11 (#5499). What the row +// predicted held, and it under-counted: on top of the `count_distinct` null +// (3 where the standard says 2) and the `"[object Object]"` `$group._id`, the +// suite measured a THIRD divergence neither card named — `count(col)` ignored +// `field` and answered the ROW count, so `count(stage)` came back 6 where the +// case-set says 4. That is what a cell clears against: an executed case-set, +// not a re-reading. The suite is the SERVER-FREE half #5517 requires, driving +// the emitted pipeline through an in-process evaluator; the real-mongod half +// is still absent and is recorded as such on #6814 rather than implied here. +// +// `driver-memory`'s row stands. The 2026-08-11 ruling lifted the freeze for +// `driver-mongodb` alone, so that cell — a missing `count_distinct` arm and the +// two-face divergence beside it — stays open by the same decision as before, +// and #6814 stays OPEN for it. const LEDGER = [ { @@ -443,31 +459,6 @@ const LEDGER = [ + 'NO mechanical alignment here. The cell stays open on `count_distinct` alone.', issue: 'https://github.com/objectstack-ai/objectstack/issues/6814', }, - { - driver: 'driver-mongodb', - marker: 'AGGREGATION_CASES', - kind: 'DEBT', - why: - 'Measured on this branch by reading `mongodb-aggregation.ts`: `count_distinct` lowers to ' - + '`{ $addToSet: fieldRef ?? null }` and `postProcessAggregation` takes the array\'s `.length`. ' - + '`$addToSet` adds an explicit `null` to the set, so a nullable column sizes ONE HIGHER than ' - + '`COUNT(DISTINCT col)` does — 3 where the case-set says 2 over `AGGREGATION_ROWS`. `$addToSet` on a ' - + 'MISSING field adds nothing, so the divergence shows only for an explicitly-null value, which is ' - + 'exactly what the fixture seeds and what a nullable column produces in practice. Not executed — this ' - + 'package has no suite for the cell, which is the debt. #5499 freezes it; the fix is a `$ne: null` ' - + 'before the `$addToSet` (or sizing a `$setDifference` against `[null]`). Tracked as #6814. Note the ' - + 'real-mongod suites are opt-in since #5517, so whatever clears this cell needs a server-free half ' - + 'like `mongodb-filter-logic-translation.test.ts` has. ' - + '[#6401] Re-measured when the case-set gained its `groupByAlias` axis, and the finding is WIDER than ' - + 'the alias: `buildAggregationPipeline` annotates `groupBy` as `string[]` and builds ' - + '`groupId[field] = \'$\' + field` (`mongodb-aggregation.ts:66-69`, mirrored in the `$project` at ' - + '`:85-88`). A STRUCTURED `GroupByNode` — aliased or not — is an object there, so the `$group._id` key ' - + 'becomes the literal `"[object Object]"` and its value `"$[object Object]"`. This face cannot take ' - + 'the structured half of the declared union at all, so the alias is unreachable rather than ignored. ' - + '`mongodb-driver.ts:512` passes `(query as any).groupBy`, which is why the union never met that ' - + '`string[]` annotation at `tsc`. Read from the source; not executed. Same #6814 home.', - issue: 'https://github.com/objectstack-ai/objectstack/issues/6814', - }, ]; // ── Discovery ───────────────────────────────────────────────────────────────