Native GroupBy refactors: post-group guard choke-point + single-source projection grammar (from EF-344 design review)

XMLWordPrintableJSON

    • Type: Task
    • Resolution: Unresolved
    • Priority: Unknown
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • None
    • Dotnet Drivers
    • None
    • None
    • None
    • None
    • None
    • None

      Architectural refactors deferred from the EF-344 pass-4 design review (native LINQ query rewrite, epic EF-322). Non-blocking; EF-344 shipped with per-site guards + a documented invariant. These touch the translation hot path, so they warrant their own reviewed change rather than being folded into the converged EF-344 branch.

      1. Collapse the three-site IsGroupBy guard into one choke point. Post-group fallback is currently enforced by three independent if (select.IsGroupBy) … guards — NativeSlotPopulator.PopulateNativeSlots and NativeCardinalityBinder.TryBindAggregate/TryBindReducer — plus a hand-maintained Query/AGENTS.md invariant ("every post-group operator entry point must gate on IsGroupBy"). That documented "remember to add this everywhere" rule is a missing-abstraction smell — and it is exactly what let the pass-1 (HAVING wrong-data) and pass-2 (aggregate KeyNotFound crash) bugs through. The only post-group operator that should stay native is a Select whose source shaper is GroupByShaperExpression; everything else after IsGroupBy should fall back. A single guard at the QMTEV VisitMethodCall fall-through could subsume all three sites (and the IsPostGroupSlotOperator list) and delete the invariant from the docs. Do it only if the choke-point placement lands cleanly against the existing Translate-override vs slot-populator delegation split; otherwise keep the explicit guards and keep the AGENTS.md invariant prominent.
      2. Single-source the group projection grammar. NativeGroupByBinder.TryGetProjectionBindings (builds accumulators + the flattening $project) and MongoQueryableMethodTranslatingExpressionVisitor.TryBuildGroupResultShaper/BindGroupMember (builds the shaper bindings) each independently walk the same anonymous/DTO projection grammar and must agree on the member set and the alias==member-name convention. If they drift, a $project emits an alias the shaper doesn't read → silent wrong results. Have the binder expose the parsed (memberName, valueExpr) list (or a shared mini-parser) that both the flatten-projection and the shaper consume, so membership/alias has one source of truth.

      Optional polish (batch if convenient): consolidate the "_id" literal (NativeGroupByBinder declares GroupIdFieldName but then hardcodes "_id"/"_id." in TryGetKeyMemberPath, and MongoPipelineFactory.RenderKeyedGroup uses a raw "_id"); a $sum/$avg/$min/$max accumulator-operator enum ONLY if the pre-existing scalar MongoGroupAccumulatorStage path adopts it at the same time (else two conventions); extract the classify/flatten loop out of TryBindGroupProjection (mixes parse + mutate).

      Context: design docs/superpowers/specs/2026-07-08-native-groupby-design.md; as-built in src/MongoDB.EntityFrameworkCore/Query/AGENTS.md (EF-SP6 GroupBy note). Related coverage/nit follow-ups: EF-345.

            Assignee:
            Unassigned
            Reporter:
            Arthur Vickers
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: