The seven reachable failures of M2, and the first commit of the milestone to
move the scorecard: 194/97/196 -> 201/90/196, with `aggregate-*.json` going
9 pass / 13 fail to 16 pass / 6 fail. The seven that moved are exactly the
seven priced as reachable, and the six that remain are exactly the six
attributed to M2.5 ($addFields, the expression engine), M4 ($listLocalSessions)
and M8 (collation).
Both stages write to a collection the pipeline is not reading, and three things
stood against doing that in the handler: `aggregate` is a `.read` command,
dispatch takes locks from a static table keyed on the command name before the
handler runs, and `Collection.lock` allows exactly one collection lock at a
time. So the handler computes the output under the locks it has and leaves it
in `Context.pending_write`; the epilogue applies it with nothing held, beside
the commit and the checkpoint already there. The `.read`/`.write` contract is
amended in its own comment rather than quietly broken.
`pending_write` is cleared at the top of every dispatch, so a handler that
errors before setting one cannot leave the previous command's write to fire. A
failed write replaces the pipeline's `ok: 1` with the failure, because a client
told the aggregation succeeded would believe the collection had been written.
What the stages do not implement is refused, not ignored: `$merge`'s
`whenMatched`, `whenNotMatched`, `on` and `let` all select behaviour this
server does not have, and a `whenMatched: "fail"` that silently merged would be
the same lie Tier 0 spent three commits removing. Codes measured against
mongod 8.3.7. `$out` and `$merge` answer byte-identically to it on both the
replace and the upsert case.
NOT ATOMIC, and said out loud in the code rather than left to be discovered.
mongod replaces an `$out` target atomically; this engine has no
cross-collection atomicity and no rename to build one from, so a crash between
the drop and the last insert leaves the target holding part of the new output
where MongoDB would leave the whole of the old. The fix is
write-to-temp-and-rename and rename is a command that does not exist here.
The test's mutation is the argument for the epilogue in one line: apply the
write inside the `$out` branch and it deadlocks rather than fails.
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86.
Priced the 13 aggregate failures one by one instead of counting them. Six
cannot turn green in M2 whatever is built: three need $listLocalSessions (M4)
*and* $addFields (M2.5), two need the expression engine, one needs a collation
(M8). So the gate reads '0 fail among the seven reachable', with the other six
named and attributed.
Note what the three db.aggregate() cases actually need. Implementing
{aggregate: 1} moves none of them, because each then fails on
$listLocalSessions instead. The review priced that line 'orthogonal, and
cheap'; it was cheap and worth nothing.
The seven that remain are $out and $merge, and they need a decision the review
had no authority to take. They write to a collection the pipeline is not
reading, and three things stand against that: aggregate is declared .read and
the dispatch contract says only .write commands may mutate; dispatch acquires
locks from a static table keyed on the command name, before the handler runs;
and Collection.lock's own comment states the invariant that decides it -- never
more than one collection lock at a time.
That is the same objection that kept $lookup out of M2.5's first cut, and the
review failed to apply it to the write stages in the same breath. Recorded as
the review's own error rather than quietly corrected. Two options set out in
§7, plus the durability question neither of them answers: mongod's $out
replaces the target atomically and this engine has no cross-collection
atomicity.
Nothing past Tier 0 is implemented until one is chosen.
It set a variable that the emit applied once. Three consequences, all measured
on a live server before this changed rather than read off the code:
- only the *last* `$project` in a pipeline had any effect;
- a `$match` after one still matched on the field it had removed;
- `{y: {$literal: 5}}` read as falsy, which flipped the whole projection into
its exclusion branch and returned every document minus `y`, where mongod
adds a computed `y`.
Now it transforms the stream where it stands, materializing into the same tree
form `$group` already produced. `$match`, `$sort` and `$group` after it read
what it left, which is what "stage" means.
What it cannot do is refused rather than mis-read. A computed field needs the
expression evaluator M2.5 brings, and a nested spec (`{a: {b: 1}}`) needs a
narrowing `query.project` does not do -- both were falsy, and falsy is the
answer that quietly returned the whole document. Codes read off mongod 8.3.7,
and the replies now match it on code, codeName and message text for the empty,
the mixed and the computed forms:
projection must have at least one field 51272 Location51272
cannot mix inclusion and exclusion 31254 Location31254
unknown expression in $project 31325 Location31325
Recorded, not fixed here: `query.project` is shared with `find`'s `projection`,
where a nested inclusion has the same falsy reading and the same wrong answer.
That is a real gap rather than an unimplemented feature -- narrowing is
something this projection should do -- so it is its own fix, not a refusal, and
it belongs with whichever milestone takes `find`'s projection seriously.
Scorecard unchanged again at 194/97/196. The corpus has no `$project` stage
tests either; A6 said so, and this is the second commit to confirm it.
Mutations: drop the refusals and the empty projection answers `ok: 1`; put the
projection back on the emit and the `$match`-after-`$project` case goes red.
190/190 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86.
A remote crash, present before this milestone and reachable by any client with
a two-stage pipeline:
aggregate: [{$group: {_id: "$k", n: {$sum: 1}}},
{$group: {_id: null, g: {$sum: 1}}}]
thread panic: index out of bounds: index 2, len 0
`$group` took `[]const u64` and was handed `offs.items[start..end]` whatever
the stream was made of. A pipeline starts as slab offsets -- matched and
reordered in place, never materialized -- and flips to generated documents the
moment a stage produces something the slab does not hold. After that `offs` is
empty while `start`/`end` count trees, so the slice ran off an empty list and
took the server thread down. There is no authentication in front of it.
Found while making `$project` a real stage, which reaches the same branch;
confirmed against the binary at the previous commit rather than assumed, so
this is a pre-existing defect and not a regression of that work. It is
committed on its own for that reason.
`Stream` names the two forms the rest of the pipeline had been carrying
implicitly in `in_trees`, and `$group` now reads whichever is live. The slab
side stays byte-walked -- grouping a million documents does not build a million
trees to read one field -- and `path_in_pairs` is the tree counterpart of
`query_path_value_bytes` for the other half.
The test groups by a key and then counts the groups, and sums `$n`, a path that
only resolves against the generated document. Its mutation -- hand `run_group`
the offsets unconditionally -- aborts the run rather than failing it, which is
why this wanted a test and not a code read.
Answers byte-identically to mongod 8.3.7 on the pipeline above.
189/189 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16,
e2e4 17, e2e6 72, e2e7 86.
M2 Tier 0, and the first commit of the milestone: stop answering wrong numbers
with `ok: 1`.
`$group` had one accumulator, `$sum`, and one expression vocabulary, `$field`
paths and constants -- open-coded twice and applied to whatever arrived.
Everything outside that fell through to a zero. Measured on a live server
before this commit: `{$avg: "$x"}` answered `0`, and so did `$max` and `$push`;
a compound `_id` collapsed every document into one group keyed by the
unevaluated expression; `{$literal: 1}` came back echoed; `{$sum: {$multiply:
[...]}}` was `0`. Six of eight probed pipelines succeeded with a wrong result.
That is a worse failure than an unimplemented one. An unrecognised stage is a
bug report; an `$avg` that returns `0` is a corrupted report nobody files. It
is also invisible to the gate: the corpus this project runs has no aggregation
stage tests at all, which is what PLAN amendment A6 is about.
So the vocabulary is now named -- `GroupExpr` is a path or a constant, and
`classify_expr` is the single place the `$`-prefix distinction is made -- and
every accumulator is validated before a document is read. A pipeline that
cannot be answered is refused whole rather than half-answered.
Five error codes added, every one read off mongod 8.3.7 rather than recalled,
and the replies now match it on code and codeName for all six `$group` shapes
probed:
unknown group operator 15952 Location15952
a group specification needs _id 15955 Location15955
accumulator is not an object 40234 Location40234
two operators in one 40238 Location40238
unrecognized expression 168 InvalidPipelineOperator
`$avg` and friends are reported as *unknown* operators, which is the choice
`location_unrecognized_stage` already made for `$addFields`: this server
reports what it does not implement using MongoDB's own code for "no such
thing", because a code MongoDB never emits would break the error-code parity
every milestone is held to. The message names the construct.
`$sum` over a non-number still contributes nothing -- that is MongoDB's rule,
not a stand-in for an unimplemented one, and the comment says so where it
would otherwise read as another silent zero.
The scorecard does not move: 194/97/196 before and after, byte-identical. That
is not a disappointment, it is the design review's central finding arriving on
schedule -- the corpus cannot see any of this, which is why M2.5 has to bring
its own.
Mutations: drop the accumulator-name check and `$avg` answers `ok: 1` again;
drop the `_id` classification and the compound `_id` does.
188/188 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86.
The scorecard collapses a whole-file skip into a single `*` line, so counting
the reason lines undercounted the cases they cover. Every one of the 19 is
version- or topology-gated, which is why the gate reads *0 fail* rather than
*all pass*.
The design review found the milestone's gate names a corpus that is not there:
`mongodb/specifications` has no aggregation suite, and the thirteen
`aggregate-*.json` files this project runs live inside `crud` and test the
aggregate *command*, not stages. One name was covering two milestones.
M2 becomes the command surface -- $out, $merge, db.aggregate(), collation,
let -- gated on the corpus that already exists, at 0 fail. M2.5 becomes the
engine: expression evaluator, per-stage document iterator, accumulators,
$unwind; $lookup and $facet explicitly out of the first cut, gated on a
corpus this project writes with every expectation measured against mongod.
Command surface first, engine second, chosen with the cost stated: the engine
is what gives wrong answers now -- six of eight probed pipelines answer ok:1
with a wrong result -- so this order leaves them alive a milestone longer.
Which is why M2 carries the refusals: every unimplemented construct stops
answering 0 and starts answering an error with a measured code, the same move
expectEvents made before the free list in M1. It will push the scorecard down,
and that is the point.
The review keeps its pre-decision recommendation verbatim, so the argument the
decision overrode stays legible.
PLAN §3 gives M2 one line of scope and one line of gate, and §6 lists the three
questions it deferred. This answers them, and reports one finding that has to
be settled before the rest is worth discussing: the gate named in the plan does
not exist. `mongodb/specifications` has no aggregation suite -- the thirteen
`aggregate-*.json` files we run live inside `crud` and test the aggregate
*command* surface, not stages. $lookup, $unwind, $facet, $addFields and
$replaceRoot appear nowhere in the pinned corpus.
Measured, not recalled. Seven stages exist, not the nine an earlier note in
this project claimed -- $set and $unset are update operators. There is no
expression engine: $field paths are open-coded in two places inside run_group
and $sum is the only accumulator. Probed on a live server, six of eight
pipelines answer ok:1 with a wrong result -- $avg, $max and $push all return
0, a compound _id groups everything into one bucket keyed by the unevaluated
expression, $literal is echoed back, and a $match after a $project still sees
the projected-away field. Only $addFields fails honestly.
That drives the tiering. M2 is not "add stages to the chain": the stream is a
materialized window and each stage moves its bounds, which cannot express
1->N ($unwind), a second collection ($lookup), or sub-pipelines ($facet), and
$project is not a stage transform at all. Six tiers proposed, four gate options
priced, and a recommendation: Tier 0 (refuse what is not implemented) alone
first, for the same reason expectEvents preceded the free list in M1 -- it will
move the scorecard down, and that is the point.
No decision is recorded in PLAN.md yet; that is what the review is for.
