Skip to content

Commit 26fe02c

Browse files
author
Ben O'Donovan
committed
Keep conversions around a grouping key on the server
- A projection lifted out of a grouping ran a conversion around g.Key in its client selector, replacing the database's CAST semantics with .NET's: (decimal)g.Key over a double key no longer rounded to the mapped scale, and (int)g.Key over a long key wrapped around instead of failing on overflow. - Only conversions which can't change the value - to or from the nullable form, boxing, and reference conversions - are now applied on the client. Any other conversion makes the projection keep its non-lifted translation, which casts on the server. - Add tests for a key conversion which changes the value and one which doesn't. Fixes #39367
1 parent 4dbb574 commit 26fe02c

3 files changed

Lines changed: 68 additions & 1 deletion

File tree

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1658,7 +1658,14 @@ public Expression Rewrite(Expression projection, ParameterExpression groupingPar
16581658
case NewArrayExpression:
16591659
case ConstantExpression:
16601660
case ParameterExpression:
1661-
case UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked }:
1661+
return base.Visit(expression);
1662+
1663+
// A conversion is computed too, unless it leaves the value as it is: (decimal)g.Key over a double key is a CAST on the
1664+
// server, which rounds to the mapped scale, and (int)g.Key over a long key is a CAST which fails on overflow rather than
1665+
// wrapping around. Only wrapping a value in its nullable form (or unwrapping it), boxing it and casting a reference
1666+
// up its hierarchy are the same in either place.
1667+
case UnaryExpression { NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked } convert
1668+
when IsValuePreservingConversion(convert):
16621669
return base.Visit(expression);
16631670

16641671
default:
@@ -1667,6 +1674,11 @@ public Expression Rewrite(Expression projection, ParameterExpression groupingPar
16671674
}
16681675
}
16691676

1677+
private static bool IsValuePreservingConversion(UnaryExpression convert)
1678+
=> convert.Method is null
1679+
&& (convert.Type.UnwrapNullableType() == convert.Operand.Type.UnwrapNullableType()
1680+
|| (!convert.Type.IsValueType && convert.Type.IsAssignableFrom(convert.Operand.Type)));
1681+
16701682
private bool TryMatchElements(
16711683
Expression expression,
16721684
out LambdaExpression? elementSelector,

‎test/EFCore.Specification.Tests/Query/NorthwindGroupByQueryTestBase.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3172,6 +3172,32 @@ public virtual Task GroupBy_selecting_grouping_element_list_with_translated_key(
31723172
AssertCollection(e.Orders, a.Orders);
31733173
});
31743174

3175+
[Theory, MemberData(nameof(IsAsyncData))]
3176+
public virtual Task GroupBy_selecting_grouping_element_list_with_converted_key(bool async)
3177+
=> AssertQuery(
3178+
async,
3179+
ss => ss.Set<OrderDetail>().GroupBy(od => od.Quantity)
3180+
.Select(g => new { Key = (int)g.Key, Orders = g.Select(od => od.OrderID).ToList() }),
3181+
elementSorter: e => e.Key,
3182+
elementAsserter: (e, a) =>
3183+
{
3184+
Assert.Equal(e.Key, a.Key);
3185+
AssertCollection(e.Orders, a.Orders);
3186+
});
3187+
3188+
[Theory, MemberData(nameof(IsAsyncData))]
3189+
public virtual Task GroupBy_selecting_grouping_element_list_with_nullable_converted_key(bool async)
3190+
=> AssertQuery(
3191+
async,
3192+
ss => ss.Set<Employee>().GroupBy(e => e.EmployeeID)
3193+
.Select(g => new { Key = (uint?)g.Key, Employees = g.Select(e => e.EmployeeID).ToList() }),
3194+
elementSorter: e => e.Key,
3195+
elementAsserter: (e, a) =>
3196+
{
3197+
Assert.Equal(e.Key, a.Key);
3198+
AssertCollection(e.Employees, a.Employees);
3199+
});
3200+
31753201
[Theory, MemberData(nameof(IsAsyncData))]
31763202
public virtual Task Select_GroupBy_SelectMany(bool async)
31773203
// Entity equality. Issue #15938.

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4286,6 +4286,35 @@ ORDER BY [c1].[Region]
42864286
""");
42874287
}
42884288

4289+
public override async Task GroupBy_selecting_grouping_element_list_with_converted_key(bool async)
4290+
{
4291+
await base.GroupBy_selecting_grouping_element_list_with_converted_key(async);
4292+
4293+
AssertSql(
4294+
"""
4295+
SELECT [o1].[c], [o1].[Quantity], [o0].[OrderID], [o0].[ProductID]
4296+
FROM (
4297+
SELECT CAST([o].[Quantity] AS int) AS [c], [o].[Quantity]
4298+
FROM [Order Details] AS [o]
4299+
GROUP BY [o].[Quantity]
4300+
) AS [o1]
4301+
LEFT JOIN [Order Details] AS [o0] ON [o1].[Quantity] = [o0].[Quantity]
4302+
ORDER BY [o1].[Quantity], [o0].[OrderID]
4303+
""");
4304+
}
4305+
4306+
public override async Task GroupBy_selecting_grouping_element_list_with_nullable_converted_key(bool async)
4307+
{
4308+
await base.GroupBy_selecting_grouping_element_list_with_nullable_converted_key(async);
4309+
4310+
AssertSql(
4311+
"""
4312+
SELECT [e].[EmployeeID], DENSE_RANK() OVER(ORDER BY [e].[EmployeeID])
4313+
FROM [Employees] AS [e]
4314+
ORDER BY [e].[EmployeeID]
4315+
""");
4316+
}
4317+
42894318
public override async Task GroupBy_selecting_grouping_element_list(bool async)
42904319
{
42914320
await base.GroupBy_selecting_grouping_element_list(async);

0 commit comments

Comments
 (0)