Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: Expand navigations in GroupBy case #17356

Merged
merged 1 commit into from
Aug 21, 2019
Merged

Query: Expand navigations in GroupBy case #17356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ...RelationalShapedQueryCompilingExpressionVisitor.CustomShaperCompilingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,11 @@ protected override Expression VisitExtension(Expression extensionExpression)
Expression.Constant(((LambdaExpression)Visit(collectionShaper.InnerShaper)).Compile()));
}

if (extensionExpression is GroupByShaperExpression)
{
throw new InvalidOperationException("Client side GroupBy is not supported.");
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
}

return base.VisitExtension(extensionExpression);
}

Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ private void AddJoin(
}

// Verify what are the cases of pushdown for inner & outer both sides
if (Limit != null || Offset != null || IsDistinct || GroupBy.Count > 1)
if (Limit != null || Offset != null || IsDistinct || GroupBy.Count > 0)
{
var sqlRemappingVisitor = new SqlRemappingVisitor(PushdownIntoSubquery(), (SelectExpression)Tables[0]);
innerSelectExpression = sqlRemappingVisitor.Remap(innerSelectExpression);
Expand All @@ -1105,7 +1105,7 @@ private void AddJoin(
|| innerSelectExpression.IsDistinct
|| innerSelectExpression.Predicate != null
|| innerSelectExpression.Tables.Count > 1
|| innerSelectExpression.GroupBy.Count > 1)
|| innerSelectExpression.GroupBy.Count > 0)
{
joinPredicate = new SqlRemappingVisitor(
innerSelectExpression.PushdownIntoSubquery(), (SelectExpression)innerSelectExpression.Tables[0])
Expand Down
39 changes: 26 additions & 13 deletions src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,46 +893,59 @@ private Expression ProcessGroupBy(
LambdaExpression elementSelector,
LambdaExpression resultSelector)
{
source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
var queryable = Reduce(source);
var keySelectorBody = ExpandNavigationsInLambdaExpression(source, keySelector);
Expression result;
if (elementSelector == null)
{
source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
// TODO: Flow include in future
//source = (NavigationExpansionExpression)new IncludeApplyingExpressionVisitor(
// this, _queryCompilationContext.IsTracking).Visit(source);
keySelector = GenerateLambda(keySelectorBody, source.CurrentParameter);
elementSelector = GenerateLambda(source.PendingSelector, source.CurrentParameter);
if (resultSelector == null)
{
result = Expression.Call(
QueryableMethods.GroupByWithKeySelector.MakeGenericMethod(
queryable.Type.TryGetSequenceType(), keySelector.ReturnType),
queryable,
Expression.Quote(keySelector));
QueryableMethods.GroupByWithKeyElementSelector.MakeGenericMethod(
source.CurrentParameter.Type, keySelector.ReturnType, elementSelector.ReturnType),
source.Source,
Expression.Quote(keySelector),
Expression.Quote(elementSelector));
}
else
{
result = Expression.Call(
QueryableMethods.GroupByWithKeyResultSelector.MakeGenericMethod(
queryable.Type.TryGetSequenceType(), keySelector.ReturnType, resultSelector.ReturnType),
queryable,
QueryableMethods.GroupByWithKeyElementResultSelector.MakeGenericMethod(
source.CurrentParameter.Type, keySelector.ReturnType, elementSelector.ReturnType, resultSelector.ReturnType),
source.Source,
Expression.Quote(keySelector),
Expression.Quote(elementSelector),
Expression.Quote(resultSelector));
}
}
else
{
source = (NavigationExpansionExpression)ProcessSelect(source, elementSelector);
source = (NavigationExpansionExpression)_pendingSelectorExpandingExpressionVisitor.Visit(source);
source = (NavigationExpansionExpression)new IncludeApplyingExpressionVisitor(
this, _queryCompilationContext.IsTracking).Visit(source);
keySelector = GenerateLambda(keySelectorBody, source.CurrentParameter);
elementSelector = GenerateLambda(source.PendingSelector, source.CurrentParameter);
if (resultSelector == null)
{
result = Expression.Call(
QueryableMethods.GroupByWithKeyElementSelector.MakeGenericMethod(
queryable.Type.TryGetSequenceType(), keySelector.ReturnType, elementSelector.ReturnType),
queryable,
source.CurrentParameter.Type, keySelector.ReturnType, elementSelector.ReturnType),
source.Source,
Expression.Quote(keySelector),
Expression.Quote(elementSelector));
}
else
{
result = Expression.Call(
QueryableMethods.GroupByWithKeyElementResultSelector.MakeGenericMethod(
queryable.Type.TryGetSequenceType(), keySelector.ReturnType, elementSelector.ReturnType, resultSelector.ReturnType),
queryable,
source.CurrentParameter.Type, keySelector.ReturnType, elementSelector.ReturnType, resultSelector.ReturnType),
source.Source,
Expression.Quote(keySelector),
Expression.Quote(elementSelector),
Expression.Quote(resultSelector));
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Relational.Specification.Tests/Query/QueryNoClientEvalTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public virtual void Throws_when_group_join()
}
}

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual void Throws_when_group_by()
{
using (var context = CreateContext())
Expand Down
8 changes: 4 additions & 4 deletions test/EFCore.Specification.Tests/Query/AsyncGearsOfWarQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ protected AsyncGearsOfWarQueryTestBase(TFixture fixture)

protected GearsOfWarContext CreateContext() => Fixture.CreateContext();

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual async Task Include_with_group_by_on_entity_qsre()
{
using (var ctx = CreateContext())
Expand All @@ -39,7 +39,7 @@ public virtual async Task Include_with_group_by_on_entity_qsre()
}
}

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual async Task Include_with_group_by_on_entity_qsre_with_composite_key()
{
using (var ctx = CreateContext())
Expand All @@ -57,7 +57,7 @@ public virtual async Task Include_with_group_by_on_entity_qsre_with_composite_ke
}
}

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual async Task Include_with_group_by_on_entity_navigation()
{
using (var ctx = CreateContext())
Expand All @@ -75,7 +75,7 @@ public virtual async Task Include_with_group_by_on_entity_navigation()
}
}

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual async Task Include_groupby_constant()
{
using (var ctx = CreateContext())
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ protected AsyncSimpleQueryTestBase(TFixture fixture)

protected NorthwindContext CreateContext() => Fixture.CreateContext();

[ConditionalFact(Skip = "Issue #15249")]
[ConditionalFact(Skip = "Issue#17068")]
public virtual async Task GroupBy_tracking_after_dispose()
{
List<IGrouping<string, Order>> groups;
Expand Down
18 changes: 9 additions & 9 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ public virtual Task Simple_level1_level2_include(bool isAsync)
l1s => l1s.Include(l1 => l1.OneToOne_Required_PK1.OneToOne_Required_PK2), elementSorter: e => e.Id);
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Simple_level1_level2_GroupBy_Count(bool isAsync)
{
Expand All @@ -408,7 +408,7 @@ public virtual Task Simple_level1_level2_GroupBy_Count(bool isAsync)
.Select(g => g.Count()));
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Simple_level1_level2_GroupBy_Having_Count(bool isAsync)
{
Expand Down Expand Up @@ -1888,7 +1888,7 @@ where Maybe(l1.OneToOne_Optional_FK1, () => l1.OneToOne_Optional_FK1.Name) != "L
elementSorter: l1 => l1.Id);
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "issue #17068")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_with_groupjoin_skip_and_take(bool isAsync)
{
Expand Down Expand Up @@ -5098,7 +5098,7 @@ public virtual void Entries_for_detached_entities_are_removed()
}
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#12088")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_reference_with_groupby_in_subquery(bool isAsync)
{
Expand All @@ -5114,7 +5114,7 @@ public virtual Task Include_reference_with_groupby_in_subquery(bool isAsync)
});
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#12088")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_collection_with_groupby_in_subquery(bool isAsync)
{
Expand All @@ -5130,7 +5130,7 @@ public virtual Task Include_collection_with_groupby_in_subquery(bool isAsync)
});
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#12088")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Multi_include_with_groupby_in_subquery(bool isAsync)
{
Expand All @@ -5149,7 +5149,7 @@ public virtual Task Multi_include_with_groupby_in_subquery(bool isAsync)
expectedIncludes);
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#12088")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_collection_with_groupby_in_subquery_and_filter_before_groupby(bool isAsync)
{
Expand All @@ -5166,7 +5166,7 @@ public virtual Task Include_collection_with_groupby_in_subquery_and_filter_befor
});
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#12088")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_collection_with_groupby_in_subquery_and_filter_after_groupby(bool isAsync)
{
Expand Down Expand Up @@ -5388,7 +5388,7 @@ public virtual Task Include_after_SelectMany_and_multiple_reference_navigations(
});
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "Issue#16752")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Include_after_SelectMany_and_reference_navigation_with_another_SelectMany_with_Distinct(bool isAsync)
{
Expand Down
28 changes: 15 additions & 13 deletions test/EFCore.Specification.Tests/Query/GroupByQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ public virtual Task GroupBy_Property_Select_Average(bool isAsync)
os => os.GroupBy(o => o.CustomerID).Select(g => g.Average(o => o.OrderID)));
}

[ConditionalTheory(Skip = "issue #15249")]
[ConditionalTheory(Skip = "issue #17068")]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_Property_Select_Average_with_navigation_expansion(bool isAsync)
{
return AssertQueryScalar<Order>(
isAsync,
os => os.Where(o => o.Customer.City != "London").GroupBy(o => o.CustomerID, (k, es) => new { k, es }).Select(g => g.es.Average(o => o.OrderID)));
os => os.Where(o => o.Customer.City != "London")
.GroupBy(o => o.CustomerID, (k, es) => new { k, es })
.Select(g => g.es.Average(o => o.OrderID)));
}

[ConditionalTheory]
Expand Down Expand Up @@ -1452,7 +1454,7 @@ on o.CustomerID equals c.CustomerID
e => e.Key);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_required_navigation_member_Aggregate(bool isAsync)
{
Expand Down Expand Up @@ -1602,7 +1604,7 @@ from c in grouping.DefaultIfEmpty()
e => e.Value);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_optional_navigation_member_Aggregate(bool isAsync)
{
Expand Down Expand Up @@ -1664,7 +1666,7 @@ on o1.OrderID equals o2.OrderID
e => e.Key);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_multi_navigation_members_Aggregate(bool isAsync)
{
Expand Down Expand Up @@ -1729,7 +1731,7 @@ public virtual Task Select_anonymous_GroupBy_Aggregate(bool isAsync)
}));
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_principal_key_property_optimization(bool isAsync)
{
Expand Down Expand Up @@ -1930,7 +1932,7 @@ public virtual Task GroupBy_filter_count_OrderBy_count_Select_sum(bool isAsync)
}));
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_Aggregate_Join(bool isAsync)
{
Expand All @@ -1955,7 +1957,7 @@ join o in os on a.LastOrderID equals o.OrderID
entryCount: 126);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_GroupBy_Aggregate_multijoins(bool isAsync)
{
Expand All @@ -1981,7 +1983,7 @@ join o in os on a.LastOrderID equals o.OrderID
entryCount: 126);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_GroupBy_Aggregate_single_join(bool isAsync)
{
Expand All @@ -2006,7 +2008,7 @@ on c.CustomerID equals a.CustomerID
entryCount: 63);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_GroupBy_Aggregate_with_another_join(bool isAsync)
{
Expand Down Expand Up @@ -2034,7 +2036,7 @@ from g in grouping
entryCount: 63);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_GroupBy_Aggregate_in_subquery(bool isAsync)
{
Expand Down Expand Up @@ -2064,10 +2066,10 @@ on o.CustomerID equals i.c.CustomerID
i.c,
i.c.CustomerID
},
entryCount: 133);
entryCount: 187);
}

[ConditionalTheory(Skip = "Issue#15249")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Join_GroupBy_Aggregate_on_key(bool isAsync)
{
Expand Down
Loading