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

Jump to bottom

Query: Push down into subquery when group by has unallowed terms #24676

Merged
1 commit merged into from
Apr 20, 2021
Merged

Query: Push down into subquery when group by has unallowed terms #24676

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
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent
source = TranslateSelect(source, elementSelector);
}

selectExpression.ApplyGrouping(translatedKey);
translatedKey = selectExpression.ApplyGrouping(translatedKey);
var groupByShaper = new GroupByShaperExpression(translatedKey, source.ShaperExpression);

if (resultSelector == null)
Expand Down
50 changes: 34 additions & 16 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 @@ -1062,13 +1062,34 @@ public void ApplyPredicate(SqlExpression expression)
/// Applies grouping from given key selector.
/// </summary>
/// <param name="keySelector"> An key selector expression for the GROUP BY. </param>
public void ApplyGrouping(Expression keySelector)
public Expression ApplyGrouping(Expression keySelector)
{
Check.NotNull(keySelector, nameof(keySelector));

ClearOrdering();

AppendGroupBy(keySelector);
var groupByTerms = new List<SqlExpression>();
var groupByAliases = new List<string?>();
AppendGroupBy(keySelector, groupByTerms, groupByAliases, "Key");

if (groupByTerms.Any(e => e is SqlConstantExpression || e is SqlParameterExpression || e is ScalarSubqueryExpression))
{
var sqlRemappingVisitor = PushdownIntoSubqueryInternal();
var newGroupByTerms = new List<SqlExpression>(groupByTerms.Count);
var subquery = (SelectExpression)_tables[0];
var subqueryTableReference = _tableReferences[0];
for (var i = 0; i < groupByTerms.Count; i++)
{
var item = groupByTerms[i];
var newItem = subquery._projection.Any(e => e.Expression.Equals(item))
? sqlRemappingVisitor.Remap(item)
: subquery.GenerateOuterColumn(subqueryTableReference, item, groupByAliases[i] ?? "Key");
newGroupByTerms.Add(newItem);
}
keySelector = new ReplacingExpressionVisitor(groupByTerms, newGroupByTerms).Visit(keySelector);
groupByTerms = newGroupByTerms;
}
_groupBy.AddRange(groupByTerms);

if (!_identifier.All(e => _groupBy.Contains(e.Column)))
{
Expand All @@ -1078,44 +1099,41 @@ public void ApplyGrouping(Expression keySelector)
_identifier.AddRange(_groupBy.Select(e => ((ColumnExpression)e, e.TypeMapping!.KeyComparer)));
}
}

return keySelector;
}

private void AppendGroupBy(Expression keySelector)
private void AppendGroupBy(Expression keySelector, List<SqlExpression> groupByTerms, List<string?> groupByAliases, string? name)
{
Check.NotNull(keySelector, nameof(keySelector));

switch (keySelector)
{
case SqlExpression sqlExpression:
if (!(sqlExpression is SqlConstantExpression
|| sqlExpression is SqlParameterExpression))
{
_groupBy.Add(sqlExpression);
}

groupByTerms.Add(sqlExpression);
groupByAliases.Add(name);
break;

case NewExpression newExpression:
foreach (var argument in newExpression.Arguments)
for (var i = 0; i < newExpression.Arguments.Count; i++)
{
AppendGroupBy(argument);
AppendGroupBy(newExpression.Arguments[i], groupByTerms, groupByAliases, newExpression.Members?[i].Name);
}

break;

case MemberInitExpression memberInitExpression:
AppendGroupBy(memberInitExpression.NewExpression);
AppendGroupBy(memberInitExpression.NewExpression, groupByTerms, groupByAliases, null);
foreach (var argument in memberInitExpression.Bindings)
{
AppendGroupBy(((MemberAssignment)argument).Expression);
var memberAssignment = (MemberAssignment)argument;
AppendGroupBy(memberAssignment.Expression, groupByTerms, groupByAliases, memberAssignment.Member.Name);
}

break;

case UnaryExpression unaryExpression
when unaryExpression.NodeType == ExpressionType.Convert
|| unaryExpression.NodeType == ExpressionType.ConvertChecked:
AppendGroupBy(unaryExpression.Operand);
AppendGroupBy(unaryExpression.Operand, groupByTerms, groupByAliases, name);
break;

default:
Expand Down
24 changes: 23 additions & 1 deletion test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,8 @@ public virtual Task GroupBy_constant_with_where_on_grouping_with_aggregate_opera
ss => ss.Set<Order>().GroupBy(o => 1)
.OrderBy(g => g.Key)
.Select(
g => new {
g => new
{
Min = g.Where(i => 1 == g.Key).Min(o => o.OrderDate),
Max = g.Where(i => 1 == g.Key).Max(o => o.OrderDate),
Sum = g.Where(i => 1 == g.Key).Sum(o => o.OrderID),
Expand Down Expand Up @@ -2917,6 +2918,27 @@ public virtual Task All_with_predicate_after_GroupBy_without_aggregate(bool asyn
g => g.Count() > 1);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_aggregate_followed_by_another_GroupBy_aggregate(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Order>().GroupBy(o => o.CustomerID)
.Select(g => new
{
g.Key,
Count = g.Count(),
LastOrder = g.Max(e => e.OrderID)
})
.GroupBy(e => 1)
.Select(g => new
{
g.Key,
Count = g.Sum(e => e.Count)
}));
}

#endregion

# region GroupByInSubquery
Expand Down
151 changes: 121 additions & 30 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,12 @@ public override async Task GroupBy_Property_Select_Key_with_constant(bool async)
await base.GroupBy_Property_Select_Key_with_constant(async);

AssertSql(
@"SELECT N'CustomerID' AS [Name], [o].[CustomerID] AS [Value], COUNT(*) AS [Count]
FROM [Orders] AS [o]
GROUP BY [o].[CustomerID]");
@"SELECT [t].[Name], [t].[CustomerID] AS [Value], COUNT(*) AS [Count]
FROM (
SELECT [o].[CustomerID], N'CustomerID' AS [Name]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Name], [t].[CustomerID]");
}

public override async Task GroupBy_aggregate_projecting_conditional_expression(bool async)
Expand Down Expand Up @@ -522,63 +525,100 @@ public override async Task GroupBy_Constant_Select_Sum_Min_Key_Max_Avg(bool asyn
await base.GroupBy_Constant_Select_Sum_Min_Key_Max_Avg(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum], MIN([o].[OrderID]) AS [Min], 2 AS [Key], MAX([o].[OrderID]) AS [Max], AVG(CAST([o].[OrderID] AS float)) AS [Avg]
FROM [Orders] AS [o]");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum], MIN([t].[OrderID]) AS [Min], [t].[Key], MAX([t].[OrderID]) AS [Max], AVG(CAST([t].[OrderID] AS float)) AS [Avg]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_Constant_with_element_selector_Select_Sum(bool async)
{
await base.GroupBy_Constant_with_element_selector_Select_Sum(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_Constant_with_element_selector_Select_Sum2(bool async)
{
await base.GroupBy_Constant_with_element_selector_Select_Sum2(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_Constant_with_element_selector_Select_Sum3(bool async)
{
await base.GroupBy_Constant_with_element_selector_Select_Sum3(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_after_predicate_Constant_Select_Sum_Min_Key_Max_Avg(bool async)
{
await base.GroupBy_after_predicate_Constant_Select_Sum_Min_Key_Max_Avg(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum], MIN([o].[OrderID]) AS [Min], 2 AS [Random], MAX([o].[OrderID]) AS [Max], AVG(CAST([o].[OrderID] AS float)) AS [Avg]
FROM [Orders] AS [o]
WHERE [o].[OrderID] > 10500");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum], MIN([t].[OrderID]) AS [Min], [t].[Key] AS [Random], MAX([t].[OrderID]) AS [Max], AVG(CAST([t].[OrderID] AS float)) AS [Avg]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
WHERE [o].[OrderID] > 10500
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_Constant_with_element_selector_Select_Sum_Min_Key_Max_Avg(bool async)
{
await base.GroupBy_Constant_with_element_selector_Select_Sum_Min_Key_Max_Avg(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum], 2 AS [Key]
FROM [Orders] AS [o]");
@"SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum], [t].[Key]
FROM (
SELECT [o].[OrderID], 2 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_constant_with_where_on_grouping_with_aggregate_operators(bool async)
{
await base.GroupBy_constant_with_where_on_grouping_with_aggregate_operators(async);

AssertSql(
@"SELECT MIN([o].[OrderDate]) AS [Min], MAX([o].[OrderDate]) AS [Max], COALESCE(SUM([o].[OrderID]), 0) AS [Sum], AVG(CAST([o].[OrderID] AS float)) AS [Average]
FROM [Orders] AS [o]");
@"SELECT MIN(CASE
WHEN 1 = [t].[Key] THEN [t].[OrderDate]
END) AS [Min], MAX(CASE
WHEN 1 = [t].[Key] THEN [t].[OrderDate]
END) AS [Max], COALESCE(SUM(CASE
WHEN 1 = [t].[Key] THEN [t].[OrderID]
END), 0) AS [Sum], AVG(CAST(CASE
WHEN 1 = [t].[Key] THEN [t].[OrderID]
END AS float)) AS [Average]
FROM (
SELECT [o].[OrderID], [o].[OrderDate], 1 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]
ORDER BY [t].[Key]");
}

public override async Task GroupBy_param_Select_Sum_Min_Key_Max_Avg(bool async)
Expand All @@ -588,35 +628,57 @@ public override async Task GroupBy_param_Select_Sum_Min_Key_Max_Avg(bool async)
AssertSql(
@"@__a_0='2'

SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum], MIN([o].[OrderID]) AS [Min], @__a_0 AS [Key], MAX([o].[OrderID]) AS [Max], AVG(CAST([o].[OrderID] AS float)) AS [Avg]
FROM [Orders] AS [o]");
SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum], MIN([t].[OrderID]) AS [Min], [t].[Key], MAX([t].[OrderID]) AS [Max], AVG(CAST([t].[OrderID] AS float)) AS [Avg]
FROM (
SELECT [o].[OrderID], @__a_0 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_param_with_element_selector_Select_Sum(bool async)
{
await base.GroupBy_param_with_element_selector_Select_Sum(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"@__a_0='2'

SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], @__a_0 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_param_with_element_selector_Select_Sum2(bool async)
{
await base.GroupBy_param_with_element_selector_Select_Sum2(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"@__a_0='2'

SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], @__a_0 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_param_with_element_selector_Select_Sum3(bool async)
{
await base.GroupBy_param_with_element_selector_Select_Sum3(async);

AssertSql(
@"SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum]
FROM [Orders] AS [o]");
@"@__a_0='2'

SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum]
FROM (
SELECT [o].[OrderID], @__a_0 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_param_with_element_selector_Select_Sum_Min_Key_Max_Avg(bool async)
Expand All @@ -626,8 +688,12 @@ public override async Task GroupBy_param_with_element_selector_Select_Sum_Min_Ke
AssertSql(
@"@__a_0='2'

SELECT COALESCE(SUM([o].[OrderID]), 0) AS [Sum], @__a_0 AS [Key]
FROM [Orders] AS [o]");
SELECT COALESCE(SUM([t].[OrderID]), 0) AS [Sum], [t].[Key]
FROM (
SELECT [o].[OrderID], @__a_0 AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_anonymous_key_type_mismatch_with_aggregate(bool async)
Expand Down Expand Up @@ -2085,6 +2151,23 @@ ELSE CAST(0 AS bit)
END");
}

public override async Task GroupBy_aggregate_followed_by_another_GroupBy_aggregate(bool async)
{
await base.GroupBy_aggregate_followed_by_another_GroupBy_aggregate(async);

AssertSql(
@"SELECT [t0].[Key0] AS [Key], COALESCE(SUM([t0].[Count]), 0) AS [Count]
FROM (
SELECT [t].[Count], 1 AS [Key0]
FROM (
SELECT COUNT(*) AS [Count]
FROM [Orders] AS [o]
GROUP BY [o].[CustomerID]
) AS [t]
) AS [t0]
GROUP BY [t0].[Key0]");
}

public override async Task GroupBy_based_on_renamed_property_simple(bool async)
{
await base.GroupBy_based_on_renamed_property_simple(async);
Expand Down Expand Up @@ -2495,12 +2578,20 @@ FROM [Orders] AS [o]
GROUP BY [o].[CustomerID]");
}

[ConditionalTheory(Skip = "Issue#19027")]
public override async Task GroupBy_scalar_subquery(bool async)
{
await base.GroupBy_scalar_subquery(async);

AssertSql(" ");
AssertSql(
@"SELECT [t].[Key], COUNT(*) AS [Count]
FROM (
SELECT (
SELECT TOP(1) [c].[ContactName]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] = [o].[CustomerID]) AS [Key]
FROM [Orders] AS [o]
) AS [t]
GROUP BY [t].[Key]");
}

public override async Task GroupBy_scalar_aggregate_in_set_operation(bool async)
Expand Down
Loading