From a8bf609ecc902300219fa92ed5ad9e71cd971f07 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Fri, 20 May 2022 17:37:12 -0700 Subject: [PATCH] Query: Assign correct type/typemapping to result of Round on SqlServer (#28068) Resolves #27124 --- .../Query/SqlExpressions/SelectExpression.cs | 2 +- .../Query/Internal/SqlServerMathTranslator.cs | 22 +++++- .../NorthwindFunctionsQueryCosmosTest.cs | 12 +++ .../Query/NorthwindFunctionsQueryTestBase.cs | 76 +++++++++++++++++++ .../NorthwindFunctionsQuerySqlServerTest.cs | 52 +++++++++++++ .../NorthwindFunctionsQuerySqliteTest.cs | 12 +++ 6 files changed, 171 insertions(+), 5 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index d835a971062..b0fe728d429 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -751,7 +751,7 @@ static void UpdateLimit(SelectExpression selectExpression) var innerExpression = RemoveConvert(innerShaperExpression); if (!(innerExpression is EntityShaperExpression - || innerExpression is IncludeExpression)) + || innerExpression is IncludeExpression)) { var sentinelExpression = innerSelectExpression.Limit!; var sentinelNullableType = sentinelExpression.Type.MakeNullable(); diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerMathTranslator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerMathTranslator.cs index 6a4012e6772..b6bee38f8bd 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerMathTranslator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerMathTranslator.cs @@ -134,13 +134,20 @@ public SqlServerMathTranslator(ISqlExpressionFactory sqlExpressionFactory) if (TruncateMethodInfos.Contains(method)) { var argument = arguments[0]; - // Result of ROUND for float/double is always double in server side + // C# has Round over decimal/double/float only so our argument will be one of those types (compiler puts convert node) + // In database result will be same type except for float which returns double which we need to cast back to float. + var resultType = argument.Type; + if (resultType == typeof(float)) + { + resultType = typeof(double); + } + var result = (SqlExpression)_sqlExpressionFactory.Function( "ROUND", new[] { argument, _sqlExpressionFactory.Constant(0), _sqlExpressionFactory.Constant(1) }, nullable: true, argumentsPropagateNullability: new[] { true, false, false }, - typeof(double)); + resultType); if (argument.Type == typeof(float)) { @@ -154,13 +161,20 @@ public SqlServerMathTranslator(ISqlExpressionFactory sqlExpressionFactory) { var argument = arguments[0]; var digits = arguments.Count == 2 ? arguments[1] : _sqlExpressionFactory.Constant(0); - // Result of ROUND for float/double is always double in server side + // C# has Round over decimal/double/float only so our argument will be one of those types (compiler puts convert node) + // In database result will be same type except for float which returns double which we need to cast back to float. + var resultType = argument.Type; + if (resultType == typeof(float)) + { + resultType = typeof(double); + } + var result = (SqlExpression)_sqlExpressionFactory.Function( "ROUND", new[] { argument, digits }, nullable: true, argumentsPropagateNullability: new[] { true, true }, - typeof(double)); + resultType); if (argument.Type == typeof(float)) { diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs index 8fdf80c9cc7..53a297bd431 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindFunctionsQueryCosmosTest.cs @@ -350,6 +350,18 @@ FROM root c WHERE (((c[""Discriminator""] = ""OrderDetail"") AND (c[""Quantity""] < 5)) AND (ROUND(c[""UnitPrice""]) > 10.0))"); } + public override Task Sum_over_round_works_correctly_in_projection(bool async) + => AssertTranslationFailed(() => base.Sum_over_round_works_correctly_in_projection(async)); + + public override Task Sum_over_round_works_correctly_in_projection_2(bool async) + => AssertTranslationFailed(() => base.Sum_over_round_works_correctly_in_projection_2(async)); + + public override Task Sum_over_truncate_works_correctly_in_projection(bool async) + => AssertTranslationFailed(() => base.Sum_over_truncate_works_correctly_in_projection(async)); + + public override Task Sum_over_truncate_works_correctly_in_projection_2(bool async) + => AssertTranslationFailed(() => base.Sum_over_truncate_works_correctly_in_projection_2(async)); + public override async Task Select_math_round_int(bool async) { await base.Select_math_round_int(async); diff --git a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs index 27e9c25b958..890ef8f6ac2 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindFunctionsQueryTestBase.cs @@ -801,6 +801,82 @@ public virtual Task Where_math_round(bool async) .Where(od => Math.Round(od.UnitPrice) > 10), entryCount: 137); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Sum_over_round_works_correctly_in_projection(bool async) + => AssertQuery( + async, + ss => ss.Set() + .Where(o => o.OrderID < 10300) + .Select(o => new + { + o.OrderID, + Sum = o.OrderDetails.Sum(i => Math.Round(i.UnitPrice, 2)) + }), + elementSorter: e => e.OrderID, + elementAsserter: (e, a) => + { + Assert.Equal(e.OrderID, a.OrderID); + Assert.Equal(e.Sum, a.Sum); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Sum_over_round_works_correctly_in_projection_2(bool async) + => AssertQuery( + async, + ss => ss.Set() + .Where(o => o.OrderID < 10300) + .Select(o => new + { + o.OrderID, + Sum = o.OrderDetails.Select(i => i.UnitPrice * i.UnitPrice).Sum(i => Math.Round(i, 2)) + }), + elementSorter: e => e.OrderID, + elementAsserter: (e, a) => + { + Assert.Equal(e.OrderID, a.OrderID); + Assert.Equal(e.Sum, a.Sum); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Sum_over_truncate_works_correctly_in_projection(bool async) + => AssertQuery( + async, + ss => ss.Set() + .Where(o => o.OrderID < 10300) + .Select(o => new + { + o.OrderID, + Sum = o.OrderDetails.Sum(i => Math.Truncate(i.UnitPrice)) + }), + elementSorter: e => e.OrderID, + elementAsserter: (e, a) => + { + Assert.Equal(e.OrderID, a.OrderID); + Assert.Equal(e.Sum, a.Sum); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Sum_over_truncate_works_correctly_in_projection_2(bool async) + => AssertQuery( + async, + ss => ss.Set() + .Where(o => o.OrderID < 10300) + .Select(o => new + { + o.OrderID, + Sum = o.OrderDetails.Select(i => i.UnitPrice * i.UnitPrice).Sum(i => Math.Truncate(i)) + }), + elementSorter: e => e.OrderID, + elementAsserter: (e, a) => + { + Assert.Equal(e.OrderID, a.OrderID); + Assert.Equal(e.Sum, a.Sum); + }); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Select_math_round_int(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs index 0e5029fdd65..eda3496b493 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindFunctionsQuerySqlServerTest.cs @@ -774,6 +774,58 @@ FROM [Order Details] AS [o] WHERE [o].[Quantity] < CAST(5 AS smallint) AND ROUND([o].[UnitPrice], 0) > 10.0"); } + public override async Task Sum_over_round_works_correctly_in_projection(bool async) + { + await base.Sum_over_round_works_correctly_in_projection(async); + + AssertSql( + @"SELECT [o].[OrderID], ( + SELECT COALESCE(SUM(ROUND([o0].[UnitPrice], 2)), 0.0) + FROM [Order Details] AS [o0] + WHERE [o].[OrderID] = [o0].[OrderID]) AS [Sum] +FROM [Orders] AS [o] +WHERE [o].[OrderID] < 10300"); + } + + public override async Task Sum_over_round_works_correctly_in_projection_2(bool async) + { + await base.Sum_over_round_works_correctly_in_projection_2(async); + + AssertSql( + @"SELECT [o].[OrderID], ( + SELECT COALESCE(SUM(ROUND([o0].[UnitPrice] * [o0].[UnitPrice], 2)), 0.0) + FROM [Order Details] AS [o0] + WHERE [o].[OrderID] = [o0].[OrderID]) AS [Sum] +FROM [Orders] AS [o] +WHERE [o].[OrderID] < 10300"); + } + + public override async Task Sum_over_truncate_works_correctly_in_projection(bool async) + { + await base.Sum_over_truncate_works_correctly_in_projection(async); + + AssertSql( + @"SELECT [o].[OrderID], ( + SELECT COALESCE(SUM(ROUND([o0].[UnitPrice], 0, 1)), 0.0) + FROM [Order Details] AS [o0] + WHERE [o].[OrderID] = [o0].[OrderID]) AS [Sum] +FROM [Orders] AS [o] +WHERE [o].[OrderID] < 10300"); + } + + public override async Task Sum_over_truncate_works_correctly_in_projection_2(bool async) + { + await base.Sum_over_truncate_works_correctly_in_projection_2(async); + + AssertSql( + @"SELECT [o].[OrderID], ( + SELECT COALESCE(SUM(ROUND([o0].[UnitPrice] * [o0].[UnitPrice], 0, 1)), 0.0) + FROM [Order Details] AS [o0] + WHERE [o].[OrderID] = [o0].[OrderID]) AS [Sum] +FROM [Orders] AS [o] +WHERE [o].[OrderID] < 10300"); + } + public override async Task Select_math_round_int(bool async) { await base.Select_math_round_int(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs index 91f725a9846..85dd004d6a3 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs @@ -102,6 +102,18 @@ public override Task Where_math_square(bool async) public override Task Where_math_round(bool async) => AssertTranslationFailed(() => base.Where_math_round(async)); + public override Task Sum_over_round_works_correctly_in_projection(bool async) + => AssertTranslationFailed(() => base.Sum_over_round_works_correctly_in_projection(async)); + + public override Task Sum_over_round_works_correctly_in_projection_2(bool async) + => AssertTranslationFailed(() => base.Sum_over_round_works_correctly_in_projection_2(async)); + + public override Task Sum_over_truncate_works_correctly_in_projection(bool async) + => AssertTranslationFailed(() => base.Sum_over_truncate_works_correctly_in_projection(async)); + + public override Task Sum_over_truncate_works_correctly_in_projection_2(bool async) + => AssertTranslationFailed(() => base.Sum_over_truncate_works_correctly_in_projection_2(async)); + public override Task Where_math_round2(bool async) => AssertTranslationFailed(() => base.Where_math_round2(async));