Skip to content

Commit b8a037c

Browse files
Translate conversions around lifted aggregates on the server
- Record a conversion around an aggregate as part of the aggregate, so it is translated with it rather than applied on the client: a cast such as (decimal)g.Average(...) rounds on SQL Server, and moving it to the client changed the result with the opt-in on (1.3333... instead of 1.33) - Such a conversion translates to a CAST around the function, which can't carry a window, so the projection keeps the GROUP BY translation; a conversion the server doesn't need, such as to a nullable type, translates to nothing and the aggregate is still windowed - Add tests for a decimal-converted average, with the opt-in off and on, and a nullable-converted count Part of #12747
1 parent 6bb6573 commit b8a037c

4 files changed

Lines changed: 146 additions & 1 deletion

File tree

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1776,7 +1776,11 @@ public Expression Rewrite(Expression projection, ParameterExpression groupingPar
17761776
return _groupingParameter is null ? expression : RewriteAggregate(aggregateIndex);
17771777
}
17781778

1779-
if (_groupingParameter is null && IsAggregateOverGrouping(expression))
1779+
// A conversion around the aggregate is part of it, and gets translated with it: on the server it can be a CAST with
1780+
// semantics of its own (a decimal cast rounds), which moving it to the client would lose. If it translates to a CAST
1781+
// around the function, the aggregate can't carry a window and the projection keeps the GROUP BY translation; a conversion
1782+
// the server doesn't need, such as to a nullable type, translates to nothing and the aggregate is windowed as usual.
1783+
if (_groupingParameter is null && IsAggregateOverGrouping(RemoveConversions(expression)))
17801784
{
17811785
_aggregates.Add(expression);
17821786

@@ -1872,6 +1876,16 @@ private bool TryMatchElements(
18721876
return true;
18731877
}
18741878

1879+
private static Expression RemoveConversions(Expression expression)
1880+
{
1881+
while (expression is UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked } conversion)
1882+
{
1883+
expression = conversion.Operand;
1884+
}
1885+
1886+
return expression;
1887+
}
1888+
18751889
/// <summary>
18761890
/// Checks whether the given expression is an aggregate applied directly to the grouping, e.g. <c>g.Sum(e => e.Value)</c>.
18771891
/// </summary>

‎test/EFCore.Relational.Specification.Tests/Query/WindowFunctionQueryRelationalTestBase.cs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,69 @@ public virtual async Task Elements_and_predicate_counts()
228228
});
229229
}
230230

231+
/// <summary>
232+
/// A conversion around an aggregate is translated with it. A decimal cast rounds on SQL Server (to decimal(18,2)), so it has to
233+
/// stay on the server, inside the aggregate's translation; there it's a CAST around the function, which can't carry a window, and
234+
/// the projection keeps the GROUP BY translation whether the option is on or off.
235+
/// </summary>
236+
[Theory, InlineData(false), InlineData(true)]
237+
public virtual async Task Elements_and_decimal_converted_aggregate(bool useWindowFunctionAggregates)
238+
{
239+
var contextFactory = await InitializeAsync(useWindowFunctionAggregates);
240+
await using var context = contextFactory.CreateDbContext();
241+
context.Employees.AddRange(
242+
new Employee { Name = "Gus", DepartmentName = "Support", Salary = 1 },
243+
new Employee { Name = "Hana", DepartmentName = "Support", Salary = 1 },
244+
new Employee { Name = "Ivo", DepartmentName = "Support", Salary = 2 });
245+
await context.SaveChangesAsync();
246+
247+
ClearLog();
248+
249+
var results = await context.Employees
250+
.Where(e => e.DepartmentName == "Support")
251+
.GroupBy(e => e.DepartmentName)
252+
.Select(g => new { Names = g.Select(e => e.Name).ToList(), AverageSalary = (decimal)g.Average(e => e.Salary) })
253+
.ToListAsync();
254+
255+
var support = Assert.Single(results);
256+
Assert.Equal(["Gus", "Hana", "Ivo"], support.Names.Order());
257+
Assert.Equal(DecimalAverageOfOneOneTwo, support.AverageSalary);
258+
}
259+
260+
/// <summary>
261+
/// The average of 1, 1 and 2 after the provider's conversion to decimal: rounded where the database converts to a fixed scale.
262+
/// </summary>
263+
protected virtual decimal DecimalAverageOfOneOneTwo
264+
=> 1.33m;
265+
266+
/// <summary>
267+
/// A conversion the server doesn't need, such as to a nullable type, translates to nothing, so the aggregate is windowed as usual.
268+
/// </summary>
269+
[Fact]
270+
public virtual async Task Elements_and_nullable_converted_count()
271+
{
272+
var contextFactory = await InitializeAsync(useWindowFunctionAggregates: true);
273+
await using var context = contextFactory.CreateDbContext();
274+
275+
var results = await context.Employees
276+
.GroupBy(e => e.DepartmentName)
277+
.Select(g => new { g.Key, Names = g.Select(e => e.Name).ToList(), Count = (int?)g.Count() })
278+
.ToListAsync();
279+
280+
Assert.Collection(
281+
results.OrderBy(r => r.Key),
282+
engineering =>
283+
{
284+
Assert.Equal(["Alice", "Bob", "Cheng"], engineering.Names.Order());
285+
Assert.Equal(3, engineering.Count);
286+
},
287+
sales =>
288+
{
289+
Assert.Equal(["Dara", "Emre"], sales.Names.Order());
290+
Assert.Equal(2, sales.Count);
291+
});
292+
}
293+
231294
[Fact]
232295
public virtual async Task Projected_elements_and_aggregate()
233296
{

‎test/EFCore.SqlServer.FunctionalTests/Query/WindowFunctionQuerySqlServerTest.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ ORDER BY [e].[DepartmentName]
9898
""");
9999
}
100100

101+
public override async Task Elements_and_decimal_converted_aggregate(bool useWindowFunctionAggregates)
102+
{
103+
await base.Elements_and_decimal_converted_aggregate(useWindowFunctionAggregates);
104+
105+
AssertSql(
106+
"""
107+
SELECT [e1].[DepartmentName], [e2].[Name], [e2].[Id], [e1].[c]
108+
FROM (
109+
SELECT CAST(AVG(CAST([e].[Salary] AS float)) AS decimal(18,2)) AS [c], [e].[DepartmentName]
110+
FROM [Employees] AS [e]
111+
WHERE [e].[DepartmentName] = N'Support'
112+
GROUP BY [e].[DepartmentName]
113+
) AS [e1]
114+
LEFT JOIN (
115+
SELECT [e0].[Name], [e0].[Id], [e0].[DepartmentName]
116+
FROM [Employees] AS [e0]
117+
WHERE [e0].[DepartmentName] = N'Support'
118+
) AS [e2] ON [e1].[DepartmentName] = [e2].[DepartmentName]
119+
ORDER BY [e1].[DepartmentName]
120+
""");
121+
}
122+
123+
public override async Task Elements_and_nullable_converted_count()
124+
{
125+
await base.Elements_and_nullable_converted_count();
126+
127+
AssertSql(
128+
"""
129+
SELECT [e].[DepartmentName], DENSE_RANK() OVER(ORDER BY [e].[DepartmentName]), [e].[Name], COUNT(*) OVER(PARTITION BY [e].[DepartmentName])
130+
FROM [Employees] AS [e]
131+
ORDER BY [e].[DepartmentName]
132+
""");
133+
}
134+
101135
public override async Task Projected_elements_and_aggregate()
102136
{
103137
await base.Projected_elements_and_aggregate();

‎test/EFCore.Sqlite.FunctionalTests/Query/WindowFunctionQuerySqliteTest.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ ORDER BY "e"."DepartmentName"
9898
""");
9999
}
100100

101+
// SQLite converts to decimal without a fixed scale, so nothing is rounded.
102+
protected override decimal DecimalAverageOfOneOneTwo
103+
=> 1.3333333333333333m;
104+
105+
public override async Task Elements_and_decimal_converted_aggregate(bool useWindowFunctionAggregates)
106+
{
107+
await base.Elements_and_decimal_converted_aggregate(useWindowFunctionAggregates);
108+
109+
AssertSql(
110+
"""
111+
SELECT "e1"."DepartmentName", "e0"."Name", "e0"."Id", "e1"."c"
112+
FROM (
113+
SELECT CAST(AVG(CAST("e"."Salary" AS REAL)) AS TEXT) AS "c", "e"."DepartmentName"
114+
FROM "Employees" AS "e"
115+
WHERE "e"."DepartmentName" = 'Support'
116+
GROUP BY "e"."DepartmentName"
117+
) AS "e1"
118+
LEFT JOIN "Employees" AS "e0" ON "e1"."DepartmentName" = "e0"."DepartmentName" AND "e0"."DepartmentName" = 'Support'
119+
ORDER BY "e1"."DepartmentName"
120+
""");
121+
}
122+
123+
public override async Task Elements_and_nullable_converted_count()
124+
{
125+
await base.Elements_and_nullable_converted_count();
126+
127+
AssertSql(
128+
"""
129+
SELECT "e"."DepartmentName", DENSE_RANK() OVER(ORDER BY "e"."DepartmentName"), "e"."Name", COUNT(*) OVER(PARTITION BY "e"."DepartmentName")
130+
FROM "Employees" AS "e"
131+
ORDER BY "e"."DepartmentName"
132+
""");
133+
}
134+
101135
public override async Task Projected_elements_and_aggregate()
102136
{
103137
await base.Projected_elements_and_aggregate();

0 commit comments

Comments
 (0)