Skip to content

Commit 71cb7cb

Browse files
committed
EF-149: Support GroupBy aggregation
Translate single-collection GroupBy to MongoDB $group via the driver's LINQ provider. TranslateGroupBy now returns a GroupByShaperExpression which TranslateSelect / the projection binder collapse into a scalar or anonymous projection of the key and aggregates; the captured chain the driver receives already renders to $group/$project. Supports scalar/composite/anonymous keys, element and result selectors, the standard aggregates (Count/LongCount/Sum/Min/Max/Average), and post-group Where/OrderBy/Select/Distinct. GroupBy composed with Join/GroupJoin/subqueries, grouping by a cross-collection navigation, and per-group entity (IGrouping) materialization remain unsupported and are rejected cleanly. Joining over a grouped query previously returned silent wrong results; it now fails translation via the IsGroupByQuery guard. Flips the NorthwindGroupByQueryMongoTest conformance suite from asserting translation failure to asserting the generated $group pipeline (green on EF8/EF9/EF10), and updates the affected SetOperations/KeylessEntities/ AggregateOperators/Miscellaneous overrides. EF-149 failing-spec count 246 -> 115.
1 parent 1d1e4e6 commit 71cb7cb

10 files changed

Lines changed: 786 additions & 623 deletions

File tree

docs/failing-spec-tests.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ that currently lack a ticket. Counts are sourced from `tests/MongoDB.EntityFrame
2525
| Ticket | Comment subject | Description | Count |
2626
| --- | --- | --- | --- |
2727
| [EF-117](https://jira.mongodb.org/browse/EF-117) | _(no remaining `// Fails:` tags)_ | Cross-collection **Include**/`ThenInclude` is now implemented for the tested shapes. The five tests formerly tagged here were re-investigated: `Outer_identifier_correctly_determined_when_doing_include_on_right_side_of_left_join` (tracking + no-tracking) now **passes**; `Collection_include_over_result_of_single_non_scalar` and `Do_not_erase_projection_mapping_when_adding_single_projection` actually fail on cross-`DbSet` subquery translation (re-tagged **EF-X001**); `Included_one_to_many_query_with_client_eval` fails on driver client-evaluation (re-tagged **EF-X003**); and Include on a keyless entity (incl. multi-level) is a genuine PK-less `$lookup` gap (re-tagged **EF-X019**). EF-117 no longer has any active `// Fails:` tags. (Join/GroupJoin/SelectMany/RightJoin/subquery failures formerly tagged here were re-categorized — see EF-X001/EF-216/EF-220/EF-X016/EF-X017/EF-X018.) | 0 |
28-
| [EF-149](https://jira.mongodb.org/browse/EF-149) | `GroupBy issue EF-149` | `GroupBy` translation is severely limited; most non-trivial group-by shapes fail to translate. | 246 |
28+
| [EF-149](https://jira.mongodb.org/browse/EF-149) | `GroupBy issue EF-149` | Single-collection `GroupBy` (scalar/composite/anonymous keys, element/result selectors, `Count`/`LongCount`/`Sum`/`Min`/`Max`/`Average`, and post-group `Where`/`OrderBy`/`Select`/`Distinct`) now translates to `$group`. Remaining gaps: `GroupBy` composed with `Join`/`GroupJoin`/subqueries, grouping by a cross-collection navigation, and per-group entity (`IGrouping`) materialization. | 115 |
2929
| [EF-153](https://jira.mongodb.org/browse/EF-153) | `TagWith EF-153` | `TagWith(...)` content is silently dropped — does not appear in the emitted MQL. | 9 |
3030
| [EF-164](https://jira.mongodb.org/browse/EF-164) | `Missing property values issue EF-164` / `Projections issue EF-164` | BSON documents that omit a required scalar (or required navigation) throw on materialization — `Project_root_with_missing_scalars`, `Project_root_entity_with_missing_required_navigation`, etc. | 3 |
3131
| [EF-202](https://jira.mongodb.org/browse/EF-202) | `Entity equality issue EF-202` | Comparing two entities (`entity1 == entity2` / `Contains(entity)`) is not lowered to a key-equality comparison. | 4 |

src/MongoDB.EntityFrameworkCore/Query/Expressions/MongoQueryExpression.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ _projectionMapping[new ProjectionMember()] =
5151
/// </summary>
5252
public Expression? CapturedExpression { get; set; }
5353

54+
/// <summary>
55+
/// Set once this query has had a <c>GroupBy</c> applied. Composing a join over a grouped query is not
56+
/// yet supported and produces wrong results if attempted, so the join translators reject it.
57+
/// </summary>
58+
public bool IsGroupByQuery { get; set; }
59+
5460
/// <inheritdoc />
5561
public override Type Type
5662
=> typeof(object);

src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingExpressionVisitor.cs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ public override Expression Visit(Expression expression)
114114
case ConstantExpression:
115115
return expression;
116116

117+
// g.Key over a grouping: bind against the (already root-rebound) key selector so a scalar key
118+
// folds to a root member and a composite/anonymous key is walked by VisitNew.
119+
case MemberExpression { Expression: GroupByShaperExpression groupByShaper, Member.Name: "Key" }:
120+
return Visit(groupByShaper.KeySelector);
121+
117122
case MemberExpression memberExpression:
118123
var currentProjectionMember = GetCurrentProjectionMember();
119124
_projectionMapping[currentProjectionMember] = memberExpression;
@@ -127,6 +132,17 @@ when IsScalarMethodPropertyAccess(methodCallExpression):
127132

128133
return new ProjectionBindingExpression(_queryExpression, projMember, expression.Type);
129134

135+
// An aggregate over a grouping (g.Count(), g.Sum(o => o.X), g.Min/Max/Average/LongCount).
136+
// A selector-bearing aggregate is lowered to Sum(Select(grouping, sel)), so the grouping is
137+
// reached by unwrapping the Enumerable/Queryable source chain rather than being Arguments[0].
138+
// The driver renders the actual accumulator from the captured chain; we only need a scalar
139+
// projection binding so the shaper reads the aggregated field from the result document.
140+
case MethodCallExpression aggregateCall when IsGroupByAggregate(aggregateCall):
141+
var aggMember = GetCurrentProjectionMember();
142+
_projectionMapping[aggMember] = aggregateCall;
143+
144+
return new ProjectionBindingExpression(_queryExpression, aggMember, expression.Type);
145+
130146
default:
131147
return base.Visit(expression);
132148
}
@@ -636,6 +652,50 @@ private void ExitProjectionMember()
636652
/// being fully visited. This covers <c>EF.Property</c> (for non-navigation properties) and
637653
/// <c>Mql.Field</c> calls.
638654
/// </summary>
655+
/// <summary>
656+
/// Whether a method call is a scalar aggregate terminal (<c>Count</c>/<c>LongCount</c>/<c>Sum</c>/
657+
/// <c>Min</c>/<c>Max</c>/<c>Average</c>) whose source chain roots at a grouping. Selector-bearing
658+
/// aggregates are lowered to <c>Sum(Select(grouping, sel))</c>, so the grouping is found by unwrapping
659+
/// the <see cref="Enumerable"/>/<see cref="Queryable"/> source chain.
660+
/// </summary>
661+
private static bool IsGroupByAggregate(MethodCallExpression call)
662+
{
663+
if (call.Arguments.Count == 0)
664+
{
665+
return false;
666+
}
667+
668+
switch (call.Method.Name)
669+
{
670+
case nameof(Enumerable.Count):
671+
case nameof(Enumerable.LongCount):
672+
case nameof(Enumerable.Sum):
673+
case nameof(Enumerable.Min):
674+
case nameof(Enumerable.Max):
675+
case nameof(Enumerable.Average):
676+
break;
677+
default:
678+
return false;
679+
}
680+
681+
var source = call.Arguments[0];
682+
while (true)
683+
{
684+
switch (source)
685+
{
686+
case GroupByShaperExpression:
687+
return true;
688+
case MethodCallExpression { Arguments.Count: > 0 } inner
689+
when inner.Method.DeclaringType == typeof(Enumerable)
690+
|| inner.Method.DeclaringType == typeof(Queryable):
691+
source = inner.Arguments[0];
692+
continue;
693+
default:
694+
return false;
695+
}
696+
}
697+
}
698+
639699
private static bool IsScalarMethodPropertyAccess(MethodCallExpression methodCallExpression)
640700
{
641701
if (methodCallExpression.TryGetEFPropertyArguments(out var source, out var memberName))

src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoQueryableMethodTranslatingExpressionVisitor.cs

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,21 +115,26 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
115115
case nameof(Queryable.Max) when methodDefinition == QueryableMethods.MaxWithoutSelector
116116
|| methodDefinition == QueryableMethods.MaxWithSelector:
117117

118-
// Join operations - delegate to base class which calls our Translate* overrides
118+
// Join / GroupBy operations - delegate to base class which calls our Translate* overrides
119119
case nameof(Queryable.Join) when methodDefinition == QueryableMethods.Join:
120120
case nameof(Queryable.GroupJoin) when methodDefinition == QueryableMethods.GroupJoin:
121121
#if !EF8 && !EF9
122122
case nameof(Queryable.LeftJoin) when methodDefinition == QueryableMethods.LeftJoin:
123123
#endif
124124
case nameof(Queryable.DefaultIfEmpty) when methodDefinition == QueryableMethods.DefaultIfEmptyWithArgument
125125
|| methodDefinition == QueryableMethods.DefaultIfEmptyWithoutArgument:
126+
// All four GroupBy overloads route to TranslateGroupBy; unsupported shapes (element/result
127+
// selectors) return null there and fail cleanly, rather than falling through to the no-op
128+
// capture path below.
129+
case nameof(Queryable.GroupBy) when methodDefinition == QueryableMethods.GroupByWithKeySelector
130+
|| methodDefinition == QueryableMethods.GroupByWithKeyElementSelector
131+
|| methodDefinition == QueryableMethods.GroupByWithKeyResultSelector
132+
|| methodDefinition == QueryableMethods.GroupByWithKeyElementResultSelector:
126133

127134
// Operations not supported, but we want to bubble through for better error messages
128135
#if !EF8 && !EF9
129136
case nameof(Queryable.RightJoin) when methodDefinition == QueryableMethods.RightJoin:
130137
#endif
131-
case nameof(Queryable.GroupBy) when methodDefinition == QueryableMethods.GroupByWithKeySelector
132-
|| methodDefinition == QueryableMethods.GroupByWithKeyElementSelector:
133138
case nameof(Queryable.Contains) when methodDefinition == QueryableMethods.Contains:
134139
case nameof(Queryable.Except) when methodDefinition == QueryableMethods.Except:
135140
case nameof(Queryable.Intersect) when methodDefinition == QueryableMethods.Intersect:
@@ -579,7 +584,31 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis
579584

580585
protected override ShapedQueryExpression? TranslateGroupBy(ShapedQueryExpression source, LambdaExpression keySelector,
581586
LambdaExpression? elementSelector, LambdaExpression? resultSelector)
582-
=> null;
587+
{
588+
// The driver's LINQ provider renders the captured GroupBy(...).Select(...) chain to a $group
589+
// stage itself; our job is only the EF-side shaper. We represent the grouping with EF's
590+
// GroupByShaperExpression, which TranslateSelect / the projection binder collapse into a scalar
591+
// or anonymous projection of the key and aggregates.
592+
//
593+
// Element and result selectors are not yet supported (later phases of EF-149); returning null
594+
// for them produces EF's canonical "could not be translated" message.
595+
if (elementSelector != null || resultSelector != null)
596+
return null;
597+
598+
// Grouping over a join/lookup source (GroupBy after Join/GroupJoin, or grouping by a cross-collection
599+
// navigation key that nav-expansion turned into a $lookup) is not yet supported. Attempting it either
600+
// produces wrong results or throws deep in shaping ("Property 'Key' is not defined for IGrouping<.,
601+
// LeftJoinResult<..>>"); reject cleanly so EF reports "could not be translated".
602+
var mongoQueryExpression = (MongoQueryExpression)source.QueryExpression;
603+
if (mongoQueryExpression.IsJoinQuery || mongoQueryExpression.GetPendingLookups().Count > 0)
604+
return null;
605+
606+
var keyShaper = ReplacingExpressionVisitor.Replace(
607+
keySelector.Parameters.Single(), source.ShaperExpression, keySelector.Body);
608+
609+
mongoQueryExpression.IsGroupByQuery = true;
610+
return source.UpdateShaperExpression(new GroupByShaperExpression(keyShaper, source));
611+
}
583612

584613
protected override ShapedQueryExpression? TranslateGroupJoin(ShapedQueryExpression outer, ShapedQueryExpression inner,
585614
LambdaExpression outerKeySelector, LambdaExpression innerKeySelector, LambdaExpression resultSelector)
@@ -609,6 +638,12 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis
609638
var outerQueryExpression = (MongoQueryExpression)outer.QueryExpression;
610639
var innerQueryExpression = (MongoQueryExpression)inner.QueryExpression;
611640

641+
// Joining over a grouped query (e.g. GroupBy(...).Select(...) on either side of a Join/GroupJoin) is
642+
// not yet supported; the driver can't render Join against a $group pipeline and silently returns
643+
// wrong results. Reject so EF reports the query as untranslatable.
644+
if (outerQueryExpression.IsGroupByQuery || innerQueryExpression.IsGroupByQuery)
645+
return null;
646+
612647
outerQueryExpression.AddInnerCollection(innerQueryExpression.CollectionExpression.EntityType);
613648

614649
// Rebind the inner entity's projection to the outer MongoQueryExpression.

tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/ProjectionTests.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,13 +1125,18 @@ public void Select_projection_nested_collection_to_list()
11251125
}
11261126

11271127
[Fact]
1128-
public void Select_projection_group_by_not_supported()
1128+
public void Select_projection_group_by()
11291129
{
1130-
Assert.ThrowsAny<Exception>(() =>
1131-
_db.Planets
1132-
.GroupBy(p => p.hasRings)
1133-
.Select(g => new { g.Key, Count = g.Count() })
1134-
.ToList());
1130+
var results = _db.Planets
1131+
.GroupBy(p => p.hasRings)
1132+
.Select(g => new { g.Key, Count = g.Count() })
1133+
.OrderBy(r => r.Key)
1134+
.ToList();
1135+
1136+
// Planets group into exactly two buckets (with and without rings); every planet is counted once.
1137+
Assert.Equal(2, results.Count);
1138+
Assert.Equal(_db.Planets.Count(), results.Sum(r => r.Count));
1139+
Assert.All(results, r => Assert.True(r.Count > 0));
11351140
}
11361141

11371142
private class OrderWithDates

tests/MongoDB.EntityFrameworkCore.SpecificationTests/Query/NorthwindAggregateOperatorsQueryMongoTest.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2163,8 +2163,12 @@ public override async Task Not_Any_false(bool async)
21632163

21642164
public override async Task Contains_inside_aggregate_function_with_GroupBy(bool async)
21652165
{
2166-
// Fails: GroupBy issue EF-149
2167-
await AssertTranslationFailed(() => base.Contains_inside_aggregate_function_with_GroupBy(async));
2166+
await base.Contains_inside_aggregate_function_with_GroupBy(async);
2167+
2168+
AssertMql(
2169+
"""
2170+
Customers.{ "$group" : { "_id" : "$Country", "__agg0" : { "$sum" : { "$cond" : { "if" : { "$in" : ["$City", ["London", "Berlin"]] }, "then" : 1, "else" : 0 } } } } }, { "$project" : { "_v" : "$__agg0", "_id" : 0 } }
2171+
""");
21682172
}
21692173

21702174
public override async Task Contains_inside_Average_without_GroupBy(bool async)

0 commit comments

Comments
 (0)