M2.5: the aggregation engine, gated by a corpus recorded from mongod #3
Reference in New Issue
Block a user
Delete Branch "m2.5-aggregation-engine"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
M2.5's first cut, as scoped by amendment A6: the aggregation engine itself.
$lookupand$facetstay out, deliberately.The corpus is 70 pass / 0 fail — every answer byte-identical to mongod
8.3.7 across the accumulators, the expressions and the document stages.
The gate had to be built first, because it did not exist
mongodb/specificationshas no aggregation suite (amendment A6), so thismilestone brings its own. The hazard in a corpus you author yourself is fatal
and obvious: it can encode your own bugs as expectations and then agree with
you forever. So the split is enforced by tooling —
sources/*.jsonholdsdocuments and pipelines and nothing else, and
record.jsasks a real mongodwhat each pipeline answers. Inputs authored, expectations measured.
No second runner:
run.js --suite-dirpoints the existing one elsewhere, sothe entity model, the matchers and
expectEventscome for free.--scorecardis refused with
--suite-dir, becausescorecard.txtis the crud corpus'srecord and the milestones are compared against it.
Each tier was recorded before it was written, and the starting numbers are in
the history: accumulators 9/10, expressions 1/26, document stages 0/24.
What the recording caught that guessing would not
$avgover a group with no numeric value is null, not0. A divisorcounting documents rather than numbers passes every hand-written test and is
wrong on the one group that matters.
$ifNulltreats them alike,$pushdoes not. Hence?bson.Valuethroughout the evaluator, where theobvious shortcut folds absent into
.nullat the boundary and loses thedistinction for good.
null— not an error, not zero — and overa string is an error, 7157723.
$group's 40238.mongod distinguishes an expression from an accumulator there.
$unwinddrops an empty array, keeps a non-array whole, and numbersincludeArrayIndexfrom zero: three behaviours where one guess covers allthree wrongly.
$modfollows the dividend's sign;-5is truthy;0.0is not.Two places the design review was wrong, and the corpus settled both
Tier 2 did not need a stage iterator. It was scoped as one because
$unwindis 1→N and "there is no way to express that in a window over theinput". True of the window as it stood; false the moment
$projectwas made torebuild the stream instead of moving bounds over it. All five document stages
share one shape: read the window, build a new list, replace the stream.
The accumulators did not need the evaluator. The tier order put them after
it; the corpus showed nine of its ten failures were reachable without one,
because every accumulator takes a single value per document.
$projectneeded nothing fromquery.projectWhich I had expected to rewrite, and which
findshares. A nested spec is adotted path —
{n: {x: 1}}and{"n.x": 1}are the same projection — anddotted paths already narrow correctly. Both the nested and the computed form
used to read as falsy, flipping the whole projection into its exclusion
branch and returning the entire document minus the field. That was recorded
during M2 as broken rather than unimplemented; this is the fix it was waiting
for, and the same flattening would fix
find.One case that projection genuinely cannot express is built directly: a
projection that only computes keeps
_idand nothing else, and with nonon-
_idflag it reads the spec as an exclusion and returns everything. Costtwo failures to find, which is what a recorded corpus is for.
Verification
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86. The crud corpus is unchanged
throughout at 201/90/196 — this milestone adds no CRUD semantics, and the
number says so.
M2.5's gate, built before the milestone it gates -- the same order that put `expectEvents` before the free list in M1 and Tier 0 before everything in M2. `mongodb/specifications` has no aggregation suite, which is amendment A6's central finding, so this milestone has to bring its own. The hazard in a corpus we author is obvious and fatal: it can encode our own bugs as expectations and then agree with us forever. So the split is enforced by the tooling. `sources/*.json` holds documents and pipelines and nothing else; `record.js` asks a real mongod 8.3.7 what each pipeline answers and writes the unified- format file from the reply. Inputs authored, expectations measured -- the discipline that corrected three assumptions in M1's session work and every error code in M2, where the alternative would have shipped both times. No second runner. `run.js --suite-dir` points the existing one somewhere else, so the entity model, the matchers, the skip accounting and `expectEvents` come for free; a second runner would drift from the first exactly where it mattered. `--scorecard` is refused with `--suite-dir`, because `scorecard.txt` is the crud corpus's record and the milestones are compared against it -- writing it from an unrelated run would replace that record silently. Errors record the code and not the message: message text is mongod's to change between releases. Group pipelines end in a `$sort`, because group output order is unspecified and a case depending on it would fail for the wrong reason on either server. The first source covers `$group`: nine accumulators including the edge cases that decide an implementation -- `$avg` over a group whose values are not numbers, `$min` of a field no document has, `$push` skipping a missing field, `$first`/`$last` against input order, grouping on an array, a compound `_id`. Where it starts, run against the M2 tip: group-accumulators.json 9 pass 10 fail 0 skip The nine include the four refusals M2 added, which answer with mongod's own codes -- so the corpus already confirms that half. The ten are the milestone. The crud corpus is unchanged at 201/90/196.Nine of them -- `$sum`, `$avg`, `$min`, `$max`, `$first`, `$last`, `$push`, `$addToSet`, `$count` -- and the corpus goes 9 pass / 10 fail to 18 pass / 1 fail. The one left is the compound `_id`, which needs the expression evaluator. Landed before that evaluator, against the tier order the design review set out, and the corpus is why: every one of these takes a single value per document, a path or a constant, so nine of its ten failures turned out to be reachable without one. `classify_expr` already produced exactly that value. What the recording caught, which is the argument for measuring expectations rather than writing them: - `$avg` over a group with no numeric value is **null**, not `0`. A divisor that counted documents rather than numbers would pass every test anybody would think to write by hand, and be wrong on the one group that matters. - `$min`/`$max` compare across types in canonical BSON order, so the maximum of `30`, `7` and `"not a number"` is the string. - `$push` skips an absent field but would push an explicit null, so "resolved to nothing" and "resolved to null" cannot be the same value internally -- which is why the accumulators take `?bson.Value` and not `.null`. - `$first`/`$last` follow input order, including when the value is absent: `$last` of a missing field is null, not the last present one. `AccState` is one struct rather than a union: the fields are small and every site already switches on the kind, so a union would add a tag test where a switch was going to be anyway. Its arrays are the gpa's, the values inside them the reply arena's -- they outlive the group and travel with the documents. `numeric_value` is the int32-or-double narrowing MongoDB reports, shared now between the accumulators and `cmd_aggregate`'s count fast path. It was written twice before; a divergence between them would make `countDocuments` disagree with the pipeline it is a shortcut for. `$avg` and `$push` came out of the Tier 0 refusal test, replaced by `$stdDevPop` and `$mergeObjects`. The refusal is a property of what is missing rather than of a list, and the test should read that way. 191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at 201/90/196.27 cases, recorded before a line of the evaluator is written, which is the whole point of having built the recorder first: the expectations come from mongod 8.3.7 rather than from what the implementation is about to do. The corpus reads 1 pass / 26 fail, and the one pass is the unknown-operator refusal M2 already answers correctly. Expressions are exercised through `$group` because `_id` and the accumulator arguments are the only expression positions that exist until `$addFields` and `$project`'s computed fields land; testing them anywhere else would be testing a stage that is not there. Ten things the recording settled that guessing would have got wrong: $add over a missing field or null null -- not an error, and not 0 $add over a string error 7157723 $divide by zero error 4848401 $mod of -5 by 4 -1, the dividend's sign $lt of a number and a string true, canonical type order $and over -5 truthy $not of a missing field true $switch, no branch and no default error 40069 two operators in one expression error 15983, *not* $group's 40238 $subtract with one operand error 16020 The six error codes are in `ErrorCode` already, so the evaluator's refusals and its runtime failures have somewhere measured to land. The evaluator itself is the next commit and is not started. 191/191 unit tests, crud corpus unchanged at 201/90/196.The whole corpus is green: 46 pass, 0 fail, byte-identical to mongod 8.3.7 on every case including all 27 expressions and the compound `_id` that was the last accumulator failure. Expressions are *compiled once per pipeline and evaluated per document*, and that split is the point rather than an optimisation: it keeps the property M2's refusals bought, which is that a pipeline that cannot be answered is refused before a single document is read instead of half way through with part of the work already reported. `Expr` is the compiled tree, `compile_expr` reports, `eval_expr` cannot. Nineteen operators: `$literal`, the five arithmetic ones, seven comparisons, `$and`/`$or`/`$not`, `$cond` in both its forms, `$ifNull` and `$switch`. Plus the two shapes that are not operators at all -- a compound document, which is what a `$group` `_id` usually is, and an array. Everything the corpus recorded, and none of it guessable: - absent and a present null are *different* internally, because `$ifNull` treats them alike and `$push` does not. Hence `?bson.Value` throughout, where the obvious shortcut is to fold absent into `.null` at the boundary and lose the distinction for good. - arithmetic over absent or null is `null` -- not an error, not zero -- and over a string is an error, 7157723. - `$divide` by zero is 4848401, `$switch` with no branch and no default is 40069, and both are failures only a document can produce, so `EvalError` exists and `report_eval_error` maps it. - two operators in one expression document is 15983 and *not* `$group`'s 40238: mongod distinguishes an expression from an accumulator there. - truthiness is MongoDB's, so `-5` is true and `0.0` is false. - `$mod` follows the dividend's sign, so -5 mod 4 is -1. - `$not` takes a bare argument as readily as a one-element array. `compile_expr` and `compile_operator` call each other, so their error set is written out rather than inferred -- Zig cannot infer a cycle, and the failure mode is a "dependency loop" message that says nothing about expressions. Three cases left the Tier 0 refusal test, because a compound `_id`, `$literal` and a `$multiply` argument all work now. What is refused should be what is missing, so an unknown operator and a wrong operand count took their place. 191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2 concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at 201/90/196.24 cases for `$addFields`/`$set`, `$unset`, `$replaceRoot`, `$unwind` and `$project`'s computed fields, recorded from mongod 8.3.7 before any of them is written. The file starts at 0 pass / 24 fail, which is the honest number for five stages this server does not have. Eight things the recording settled, none of them guessable from the manual: $addFields whose expression is missing the field is not added at all $addFields: {"n.z": 1} sets the nested path, keeps siblings $replaceRoot of missing or non-document error 40228 $unwind of an empty array or missing the document is dropped $unwind of a non-array the document is kept whole $unwind path without a $ error 28818 includeArrayIndex 0-based $project: {n: {x: 1}} narrows it; a document without `n` keeps only `_id` That last one is the `query.project` gap recorded during M2 as broken rather than unimplemented -- a nested inclusion currently reads as falsy and returns the whole document minus the field. It now has a measured expectation to be fixed against, in the corpus, rather than a note in a commit message. Worth stating before the implementation: the design review expected Tier 2 to need a per-stage iterator, on the grounds that `$unwind` is 1->N and "there is no way to express that in a window over the input". That was true of the window as it stood, and stopped being true when `$project` was made to rebuild the stream -- a stage that rebuilds can emit any number of documents it likes. So Tier 2 looks like five stages rather than a rewrite. The corpus is what will say whether that holds.`$addFields`, `$set`, `$unset`, `$replaceRoot` and `$unwind`. The corpus goes 0 pass / 24 fail to 21 / 3, and the three left are `$project`'s computed fields, renames and nested inclusions, which want the `query.project` that `find` shares and are their own change. **The design review was wrong about what this needed, and the corpus is what settled it.** Tier 2 was scoped as a per-stage iterator on the grounds that `$unwind` is 1->N and "there is no way to express that in a window over the input". That was true of the window as it stood, and stopped being true the moment `$project` was made to rebuild the stream instead of moving bounds over it -- a stage that rebuilds can emit as many documents as it likes, or none. So all five share one shape: read the window, build a new list, replace the stream. No iterator, no rewrite. What the recording settled, and what a hand-written test would have got wrong: - `$addFields` whose expression resolves to nothing leaves the field out entirely rather than setting it to null -- so `set_path` is only reached when there is a value, and `eval_expr`'s absent/null distinction earns its keep a second time. - `$addFields: {"n.z": 1}` sets the nested path and keeps its siblings, and an existing field is replaced *where it stands*, which is what makes the stage "add or overwrite" rather than "append". - `$unwind` drops a document whose field is missing or an empty array, keeps one whose field is not an array *whole*, and numbers `includeArrayIndex` from zero. Three separate behaviours where one guess would have covered them all wrongly. - `$replaceRoot` of a missing path and of a non-document are the same error, 40228. `ReplaceRootNotDocument` joins `EvalError` rather than being reported at the stage: it is a failure only a document can produce, which is the line that set already draws. 191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at 201/90/196.The last three cases of the corpus, which is now 70 pass / 0 fail -- every answer byte-identical to mongod 8.3.7 across the accumulators, the expressions and the document stages. The fix turned out to need nothing from `query.project`, which `find` shares and which I had expected to have to rewrite. A nested spec *is* a dotted path: `{n: {x: 1}}` and `{"n.x": 1}` are the same projection, and dotted paths are something the existing projection already narrows correctly. So `$project` is flattened into inclusion/exclusion flags plus a list of computed fields, and both halves reuse machinery that was already there -- `query.project` for the flags, `set_path` from the document stages for the computed fields. A bare path (`{value: "$a"}`) is a rename, which is a computed field like any other. Both shapes used to read as *falsy*, which flipped the whole projection into its exclusion branch and returned the entire document minus the field. That was recorded during M2 as broken rather than unimplemented; this is the fix it was waiting for. One case `query.project` genuinely cannot express, so it is built directly: a projection that only computes keeps `_id` and nothing else, and with no non-`_id` flag that function reads the spec as an exclusion and returns everything. It cost two failures and a `id_only` flag to find, which is what a recorded corpus is for -- the answer is obvious once seen and not before. `$project` now goes through the same `Rewrite` path as `$addFields`, `$unset`, `$replaceRoot` and `$unwind`, so its own branch is gone. Its refusal shrank to the one shape mongod also refuses, mixing inclusion with exclusion, judged on the flattened flags so a nested spec is treated like a dotted one. 191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2 concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at 201/90/196.