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

…'t work for complex scenarios Adding COALESCE around SUM sql function expression inside subquery Fixes #12657
dotnet · Jun 10, 2020 · 11a8896 · 11a8896
1 parent 6d1a286
commit 11a8896
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 10 deletions.
diff --git a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
@@ -29,6 +29,7 @@ public class SqlNullabilityProcessor
         private readonly List<ColumnExpression> _nonNullableColumns;
         private readonly ISqlExpressionFactory _sqlExpressionFactory;
         private bool _canCache;
+        private bool _insideSubquery;
 
         /// <summary>
         ///     Creates a new instance of the <see cref="SqlNullabilityProcessor" /> class.
@@ -64,7 +65,7 @@ public SqlNullabilityProcessor(
         /// <param name="canCache"> A bool value indicating whether the select expression can be cached. </param>
         /// <returns> An optimized select expression. </returns>
         public virtual SelectExpression Process(
-            [NotNull] SelectExpression selectExpression, [NotNull] IReadOnlyDictionary<string, object> parameterValues,  out bool canCache)
+            [NotNull] SelectExpression selectExpression, [NotNull] IReadOnlyDictionary<string, object> parameterValues, out bool canCache)
         {
             Check.NotNull(selectExpression, nameof(selectExpression));
             Check.NotNull(parameterValues, nameof(parameterValues));
@@ -718,9 +719,13 @@ protected virtual SqlExpression VisitScalarSubquery(
         {
             Check.NotNull(scalarSubqueryExpression, nameof(scalarSubqueryExpression));
 
+            var insideSubquery = _insideSubquery;
+            _insideSubquery = true;
             nullable = true;
+            var subquery = Visit(scalarSubqueryExpression.Subquery);
+            _insideSubquery = insideSubquery;
 
-            return scalarSubqueryExpression.Update(Visit(scalarSubqueryExpression.Subquery));
+            return scalarSubqueryExpression.Update(subquery);
         }
 
         /// <summary>
@@ -918,8 +923,14 @@ protected virtual SqlExpression VisitSqlFunction(
                 arguments[i] = Visit(sqlFunctionExpression.Arguments[i], out _);
             }
 
-
-            return sqlFunctionExpression.Update(instance, arguments);
+            return _insideSubquery
+                && 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/NorthwindAggregateOperatorsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs
@@ -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]

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindNavigationsQuerySqlServerTest.cs
@@ -292,7 +292,7 @@ public override async Task Select_count_plus_sum(bool async)
 
             AssertSql(
                 @"SELECT (
-    SELECT SUM(CAST([o].[Quantity] AS int))
+    SELECT COALESCE(SUM(CAST([o].[Quantity] AS int)), 0)
     FROM [Order Details] AS [o]
     WHERE [o1].[OrderID] = [o].[OrderID]) + (
     SELECT COUNT(*)
@@ -625,7 +625,7 @@ public override async Task Collection_select_nav_prop_sum(bool async)
 
             AssertSql(
                 @"SELECT (
-    SELECT SUM([o].[OrderID])
+    SELECT COALESCE(SUM([o].[OrderID]), 0)
     FROM [Orders] AS [o]
     WHERE [c].[CustomerID] = [o].[CustomerID]) AS [Sum]
 FROM [Customers] AS [c]");
@@ -636,7 +636,11 @@ public override async Task Collection_select_nav_prop_sum_plus_one(bool async)
             await base.Collection_select_nav_prop_sum_plus_one(async);
 
             AssertSql(
-                "");
+                @"SELECT (
+    SELECT COALESCE(SUM([o].[OrderID]), 0)
+    FROM [Orders] AS [o]
+    WHERE [c].[CustomerID] = [o].[CustomerID]) + 1 AS [Sum]
+FROM [Customers] AS [c]");
         }
 
         public override async Task Collection_where_nav_prop_sum(bool async)
@@ -647,7 +651,7 @@ public override async Task Collection_where_nav_prop_sum(bool async)
                 @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
 FROM [Customers] AS [c]
 WHERE (
-    SELECT SUM([o].[OrderID])
+    SELECT COALESCE(SUM([o].[OrderID]), 0)
     FROM [Orders] AS [o]
     WHERE [c].[CustomerID] = [o].[CustomerID]) > 1000");
         }