Fix to #12657 - Query: logic protecting SUM from returning null doesn…

…'t work for complex scenarios Adding COALESCE around SUM sql function expression Fixes #12657
dotnet · Jun 10, 2020 · f4b1bc1 · f4b1bc1
1 parent c5d04a0
commit f4b1bc1
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 92 deletions.
diff --git a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
@@ -918,8 +918,13 @@ protected virtual SqlExpression VisitSqlFunction(
                 arguments[i] = Visit(sqlFunctionExpression.Arguments[i], out _);
             }
 
-
-            return sqlFunctionExpression.Update(instance, arguments);
+            return sqlFunctionExpression.IsBuiltIn
+                && string.Equals(sqlFunctionExpression.Name, "SUM", StringComparison.OrdinalIgnoreCase)
+                ?  _sqlExpressionFactory.Coalesce(
+                        sqlFunctionExpression.Update(instance, arguments),
+                        _sqlExpressionFactory.Constant(0, sqlFunctionExpression.TypeMapping),
+                        sqlFunctionExpression.TypeMapping)
+                : sqlFunctionExpression.Update(instance, arguments);
         }
 
         /// <summary>

diff --git a/test/EFCore.Specification.Tests/Query/NorthwindNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindNavigationsQueryTestBase.cs
@@ -753,7 +753,7 @@ public virtual Task Collection_select_nav_prop_sum(bool async)
                 elementSorter: e => e.Sum);
         }
 
-        [ConditionalTheory(Skip = "Issue#12657")]
+        [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Collection_select_nav_prop_sum_plus_one(bool async)
         {

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
@@ -975,7 +975,7 @@ public override async Task Result_operator_nav_prop_reference_optional_Sum(bool
             await base.Result_operator_nav_prop_reference_optional_Sum(async);
 
             AssertSql(
-                @"SELECT SUM([l0].[Level1_Required_Id])
+                @"SELECT COALESCE(SUM([l0].[Level1_Required_Id]), 0)
 FROM [LevelOne] AS [l]
 LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]");
         }
@@ -1035,10 +1035,10 @@ public override async Task Result_operator_nav_prop_reference_optional_via_Defau
             await base.Result_operator_nav_prop_reference_optional_via_DefaultIfEmpty(async);
 
             AssertSql(
-                @"SELECT SUM(CASE
+                @"SELECT COALESCE(SUM(CASE
     WHEN [l0].[Id] IS NULL THEN 0
     ELSE [l0].[Level1_Required_Id]
-END)
+END), 0)
 FROM [LevelOne] AS [l]
 LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]");
         }
@@ -4319,7 +4319,7 @@ public override async Task Sum_with_selector_cast_using_as(bool async)
             await base.Sum_with_selector_cast_using_as(async);
 
             AssertSql(
-                @"SELECT SUM([l].[Id])
+                @"SELECT COALESCE(SUM([l].[Id]), 0)
 FROM [LevelOne] AS [l]");
         }
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsWeakQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsWeakQuerySqlServerTest.cs
@@ -119,10 +119,10 @@ public override async Task Result_operator_nav_prop_reference_optional_via_Defau
             await base.Result_operator_nav_prop_reference_optional_via_DefaultIfEmpty(async);
 
             AssertSql(
-                @"SELECT SUM(CASE
+                @"SELECT COALESCE(SUM(CASE
     WHEN [t].[Id0] IS NULL THEN 0
     ELSE [t].[Level1_Required_Id]
-END)
+END), 0)
 FROM [Level1] AS [l]
 LEFT JOIN (
     SELECT [l0].[Id], [l0].[Date], [l0].[Name], [l0].[OneToOne_Required_PK_Date], [l0].[Level1_Optional_Id], [l0].[Level1_Required_Id], [l0].[Level2_Name], [l0].[OneToMany_Optional_Inverse2Id], [l0].[OneToMany_Required_Inverse2Id], [l0].[OneToOne_Optional_PK_Inverse2Id], [l0].[Id] AS [Id0]

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
@@ -2593,7 +2593,7 @@ public override async Task Sum_with_optional_navigation_is_translated_to_sql(boo
             await base.Sum_with_optional_navigation_is_translated_to_sql(async);
 
             AssertSql(
-                @"SELECT SUM([g].[SquadId])
+                @"SELECT COALESCE(SUM([g].[SquadId]), 0)
 FROM [Gears] AS [g]
 LEFT JOIN [Tags] AS [t] ON ([g].[Nickname] = [t].[GearNickName]) AND ([g].[SquadId] = [t].[GearSquadId])
 WHERE ([t].[Note] <> N'Foo') OR [t].[Note] IS NULL");
@@ -5348,7 +5348,7 @@ public override async Task GroupBy_Property_Include_Select_Sum(bool async)
             await base.GroupBy_Property_Include_Select_Sum(async);
 
             AssertSql(
-                @"SELECT SUM([g].[SquadId])
+                @"SELECT COALESCE(SUM([g].[SquadId]), 0)
 FROM [Gears] AS [g]
 GROUP BY [g].[Rank]");
         }
@@ -6145,7 +6145,7 @@ public override async Task Complex_GroupBy_after_set_operator(bool async)
             await base.Complex_GroupBy_after_set_operator(async);
 
             AssertSql(
-                @"SELECT [t].[Name], [t].[Count], SUM([t].[Count]) AS [Sum]
+                @"SELECT [t].[Name], [t].[Count], COALESCE(SUM([t].[Count]), 0) AS [Sum]
 FROM (
     SELECT [c].[Name], (
         SELECT COUNT(*)
@@ -6169,7 +6169,7 @@ public override async Task Complex_GroupBy_after_set_operator_using_result_selec
             await base.Complex_GroupBy_after_set_operator_using_result_selector(async);
 
             AssertSql(
-                @"SELECT [t].[Name], [t].[Count], SUM([t].[Count]) AS [Sum]
+                @"SELECT [t].[Name], [t].[Count], COALESCE(SUM([t].[Count]), 0) AS [Sum]
 FROM (
     SELECT [c].[Name], (
         SELECT COUNT(*)

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs
@@ -38,7 +38,7 @@ public override async Task Sum_with_no_arg(bool async)
             await base.Sum_with_no_arg(async);
 
             AssertSql(
-                @"SELECT SUM([o].[OrderID])
+                @"SELECT COALESCE(SUM([o].[OrderID]), 0)
 FROM [Orders] AS [o]");
         }
 
@@ -47,7 +47,7 @@ public override async Task Sum_with_binary_expression(bool async)
             await base.Sum_with_binary_expression(async);
 
             AssertSql(
-                @"SELECT SUM([o].[OrderID] * 2)
+                @"SELECT COALESCE(SUM([o].[OrderID] * 2), 0)
 FROM [Orders] AS [o]");
         }
 
@@ -56,7 +56,7 @@ public override async Task Sum_with_arg(bool async)
             await base.Sum_with_arg(async);
 
             AssertSql(
-                @"SELECT SUM([o].[OrderID])
+                @"SELECT COALESCE(SUM([o].[OrderID]), 0)
 FROM [Orders] AS [o]");
         }
 
@@ -65,7 +65,7 @@ public override async Task Sum_with_arg_expression(bool async)
             await base.Sum_with_arg_expression(async);
 
             AssertSql(
-                @"SELECT SUM([o].[OrderID] + [o].[OrderID])
+                @"SELECT COALESCE(SUM([o].[OrderID] + [o].[OrderID]), 0)
 FROM [Orders] AS [o]");
         }
 
@@ -74,7 +74,7 @@ public override async Task Sum_with_division_on_decimal(bool async)
             await base.Sum_with_division_on_decimal(async);
 
             AssertSql(
-                @"SELECT SUM(CAST([o].[Quantity] AS decimal(18,2)) / 2.09)
+                @"SELECT COALESCE(SUM(CAST([o].[Quantity] AS decimal(18,2)) / 2.09), 0.0)
 FROM [Order Details] AS [o]");
         }
 
@@ -83,7 +83,7 @@ public override async Task Sum_with_division_on_decimal_no_significant_digits(bo
             await base.Sum_with_division_on_decimal_no_significant_digits(async);
 
             AssertSql(
-                @"SELECT SUM(CAST([o].[Quantity] AS decimal(18,2)) / 2.0)
+                @"SELECT COALESCE(SUM(CAST([o].[Quantity] AS decimal(18,2)) / 2.0), 0.0)
 FROM [Order Details] AS [o]");
         }
 
@@ -92,7 +92,7 @@ public override async Task Sum_with_coalesce(bool async)
             await base.Sum_with_coalesce(async);
 
             AssertSql(
-                @"SELECT SUM(COALESCE([p].[UnitPrice], 0.0))
+                @"SELECT COALESCE(SUM(COALESCE([p].[UnitPrice], 0.0)), 0.0)
 FROM [Products] AS [p]
 WHERE [p].[ProductID] < 40");
         }
@@ -131,7 +131,7 @@ public override async Task Sum_on_float_column(bool async)
             await base.Sum_on_float_column(async);
 
             AssertSql(
-                @"SELECT CAST(SUM([o].[Discount]) AS real)
+                @"SELECT CAST(COALESCE(SUM([o].[Discount]), 0.0E0) AS real)
 FROM [Order Details] AS [o]
 WHERE [o].[ProductID] = 1");
         }
@@ -142,7 +142,7 @@ public override async Task Sum_on_float_column_in_subquery(bool async)
 
             AssertSql(
                 @"SELECT [o0].[OrderID], (
-    SELECT CAST(SUM([o].[Discount]) AS real)
+    SELECT CAST(COALESCE(SUM([o].[Discount]), 0.0E0) AS real)
     FROM [Order Details] AS [o]
     WHERE [o0].[OrderID] = [o].[OrderID]) AS [Sum]
 FROM [Orders] AS [o0]
@@ -1295,7 +1295,7 @@ public override async Task Project_constant_Sum(bool async)
             await base.Project_constant_Sum(async);
 
             AssertSql(
-                @"SELECT SUM(1)
+                @"SELECT COALESCE(SUM(1), 0)
 FROM [Employees] AS [e]");
         }
 
@@ -1453,7 +1453,7 @@ public override async Task Sum_over_explicit_cast_over_column(bool async)
             await base.Sum_over_explicit_cast_over_column(async);
 
             AssertSql(
-                @"SELECT SUM(CAST([o].[OrderID] AS bigint))
+                @"SELECT COALESCE(SUM(CAST([o].[OrderID] AS bigint)), CAST(0 AS bigint))
 FROM [Orders] AS [o]");
         }