Query: Read Avg/Min/Max as nullable from database to throw exception … (

#18971) * Query: Read Avg/Min/Max as nullable from database to throw exception only when it is empty Resolves #18955 * Update FloatTypeMapping.cs PR Feedback
dotnet · Nov 26, 2019 · 2da12e7 · 2da12e7
1 parent 951aca3
commit 2da12e7
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 38 deletions.
diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
@@ -779,43 +779,33 @@ private ShapedQueryExpression AggregateResultShaper(
 
             selectExpression.ClearOrdering();
 
-            Expression shaper = new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), projection.Type);
+            var nullableResultType = resultType.MakeNullable();
+            Expression shaper = new ProjectionBindingExpression(
+                source.QueryExpression, new ProjectionMember(), throwOnNullResult ? nullableResultType : projection.Type);
 
-            if (throwOnNullResult
-                && resultType.IsNullableType())
+            if (throwOnNullResult)
             {
-                var resultVariable = Expression.Variable(projection.Type, "result");
+                var resultVariable = Expression.Variable(nullableResultType, "result");
+                var returnValueForNull = resultType.IsNullableType()
+                    ? (Expression)Expression.Constant(null, resultType)
+                    : Expression.Throw(
+                        Expression.New(
+                            typeof(InvalidOperationException).GetConstructors()
+                                .Single(ci => ci.GetParameters().Length == 1),
+                            Expression.Constant(CoreStrings.NoElements)),
+                        resultType);
 
                 shaper = Expression.Block(
                     new[] { resultVariable },
                     Expression.Assign(resultVariable, shaper),
                     Expression.Condition(
-                        Expression.Equal(resultVariable, Expression.Default(projection.Type)),
-                        Expression.Constant(null, resultType),
+                        Expression.Equal(resultVariable, Expression.Default(nullableResultType)),
+                        returnValueForNull,
                         resultType != resultVariable.Type
                             ? Expression.Convert(resultVariable, resultType)
                             : (Expression)resultVariable));
             }
-            else if (throwOnNullResult)
-            {
-                var resultVariable = Expression.Variable(projection.Type, "result");
-
-                shaper = Expression.Block(
-                    new[] { resultVariable },
-                    Expression.Assign(resultVariable, shaper),
-                    Expression.Condition(
-                        Expression.Equal(resultVariable, Expression.Default(projection.Type)),
-                        Expression.Throw(
-                            Expression.New(
-                                typeof(InvalidOperationException).GetConstructors()
-                                    .Single(ci => ci.GetParameters().Length == 1),
-                                Expression.Constant(CoreStrings.NoElements)),
-                            resultType),
-                        resultType != resultVariable.Type
-                            ? Expression.Convert(resultVariable, resultType)
-                            : (Expression)resultVariable));
-            }
-            else if (resultType.IsNullableType())
+            else if (resultType != shaper.Type)
             {
                 shaper = Expression.Convert(shaper, resultType);
             }

diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
@@ -1143,11 +1143,13 @@ private ShapedQueryExpression AggregateResultShaper(
 
             selectExpression.ClearOrdering();
 
-            Expression shaper = new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), projection.Type);
+            var nullableResultType = resultType.MakeNullable();
+            Expression shaper = new ProjectionBindingExpression(
+                source.QueryExpression, new ProjectionMember(), throwOnNullResult ? nullableResultType : projection.Type);
 
             if (throwOnNullResult)
             {
-                var resultVariable = Expression.Variable(projection.Type, "result");
+                var resultVariable = Expression.Variable(nullableResultType, "result");
                 var returnValueForNull = resultType.IsNullableType()
                     ? (Expression)Expression.Constant(null, resultType)
                     : Expression.Throw(
@@ -1161,18 +1163,15 @@ private ShapedQueryExpression AggregateResultShaper(
                     new[] { resultVariable },
                     Expression.Assign(resultVariable, shaper),
                     Expression.Condition(
-                        Expression.Equal(resultVariable, Expression.Default(projection.Type)),
+                        Expression.Equal(resultVariable, Expression.Default(nullableResultType)),
                         returnValueForNull,
                         resultType != resultVariable.Type
                             ? Expression.Convert(resultVariable, resultType)
                             : (Expression)resultVariable));
             }
-            else
+            else if (resultType != shaper.Type)
             {
-                if (resultType.IsNullableType())
-                {
-                    shaper = Expression.Convert(shaper, resultType);
-                }
+                shaper = Expression.Convert(shaper, resultType);
             }
 
             source.ShaperExpression = shaper;

diff --git a/src/EFCore.Relational/Storage/FloatTypeMapping.cs b/src/EFCore.Relational/Storage/FloatTypeMapping.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Data;
 using System.Globalization;
 using JetBrains.Annotations;
@@ -55,6 +56,6 @@ protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters p
         ///     The generated string.
         /// </returns>
         protected override string GenerateNonNullSqlLiteral(object value)
-            => ((float)value).ToString("R", CultureInfo.InvariantCulture);
+            => Convert.ToSingle(value).ToString("R", CultureInfo.InvariantCulture);
     }
 }
diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerDoubleTypeMapping.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerDoubleTypeMapping.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Data;
 using System.Data.Common;
 using JetBrains.Annotations;
@@ -58,7 +59,7 @@ protected override string GenerateNonNullSqlLiteral(object value)
         {
             var literal = base.GenerateNonNullSqlLiteral(value);
 
-            var doubleValue = (double)value;
+            var doubleValue = Convert.ToDouble(value);
             return !literal.Contains("E")
                 && !literal.Contains("e")
                 && !double.IsNaN(doubleValue)

diff --git a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs
@@ -1885,9 +1885,46 @@ await AssertCount(
                 ss => ss.Set<Order>().Select(o => new { Id = CodeFormat(o.OrderID) }));
         }
 
-        private static string CodeFormat(int str)
+        private static string CodeFormat(int str) => str.ToString();
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Sum_over_empty_returns_zero(bool isAsync)
+        {
+            return AssertSum(
+                isAsync,
+                ss => ss.Set<Order>().Where(o => o.OrderID == 42),
+                o => o.OrderID);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Average_over_default_returns_default(bool isAsync)
+        {
+            return AssertAverage(
+                isAsync,
+                ss => ss.Set<Order>().Where(o => o.OrderID == 10248),
+                o => o.OrderID - 10248);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Max_over_default_returns_default(bool isAsync)
         {
-            return str.ToString();
+            return AssertMax(
+                isAsync,
+                ss => ss.Set<Order>().Where(o => o.OrderID == 10248),
+                o => o.OrderID - 10248);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Min_over_default_returns_default(bool isAsync)
+        {
+            return AssertMin(
+                isAsync,
+                ss => ss.Set<Order>().Where(o => o.OrderID == 10248),
+                o => o.OrderID - 10248);
         }
     }
 }