Merge branch 'master' of https://github.com/aspnet/EntityFrameworkCore

maumar · maumar · commit 7f086a8c3f9a · 2020-01-03T15:06:57.000-08:00
diff --git a/src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs b/src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs
@@ -181,15 +181,6 @@ SqlBinaryExpression And(
         SqlBinaryExpression Or(
             [NotNull] SqlExpression left, [NotNull] SqlExpression right, [CanBeNull] CoreTypeMapping typeMapping = null);
 
-        /// <summary>
-        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-        ///     any release. You should only use it directly in your code with extreme caution and knowing that
-        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-        /// </summary>
-        SqlBinaryExpression Coalesce(
-            [NotNull] SqlExpression left, [NotNull] SqlExpression right, [CanBeNull] CoreTypeMapping typeMapping = null);
-
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
diff --git a/src/EFCore.Cosmos/Query/Internal/QuerySqlGenerator.cs b/src/EFCore.Cosmos/Query/Internal/QuerySqlGenerator.cs
@@ -60,10 +60,7 @@ public class QuerySqlGenerator : SqlExpressionVisitor
             // Unary
             { ExpressionType.UnaryPlus, "+" },
             { ExpressionType.Negate, "-" },
-            { ExpressionType.Not, "~" },
-
-            // Others
-            { ExpressionType.Coalesce, " ?? " }
+            { ExpressionType.Not, "~" }
         };
 
         /// <summary>
diff --git a/src/EFCore.Cosmos/Query/Internal/SqlBinaryExpression.cs b/src/EFCore.Cosmos/Query/Internal/SqlBinaryExpression.cs
@@ -37,7 +37,6 @@ public class SqlBinaryExpression : SqlExpression
             ExpressionType.Equal,
             ExpressionType.NotEqual,
             ExpressionType.ExclusiveOr,
-            ExpressionType.Coalesce,
             ExpressionType.RightShift,
             ExpressionType.LeftShift
         };
@@ -160,12 +159,8 @@ public override void Print(ExpressionPrinter expressionPrinter)
             {
                 expressionPrinter.Append(")");
             }
-        }
 
-        private bool RequiresBrackets(SqlExpression expression)
-        {
-            return expression is SqlBinaryExpression sqlBinary
-                   && sqlBinary.OperatorType != ExpressionType.Coalesce;
+            static bool RequiresBrackets(SqlExpression expression) => expression is SqlBinaryExpression;
         }
 
         /// <summary>
diff --git a/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs b/src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs
@@ -179,7 +179,6 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
                 case ExpressionType.Modulo:
                 case ExpressionType.LeftShift:
                 case ExpressionType.RightShift:
-                case ExpressionType.Coalesce:
                 case ExpressionType.And:
                 case ExpressionType.Or:
                 {
@@ -373,15 +372,6 @@ public virtual SqlBinaryExpression And(SqlExpression left, SqlExpression right,
         public virtual SqlBinaryExpression Or(SqlExpression left, SqlExpression right, CoreTypeMapping typeMapping = null)
             => MakeBinary(ExpressionType.Or, left, right, typeMapping);
 
-        /// <summary>
-        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-        ///     any release. You should only use it directly in your code with extreme caution and knowing that
-        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-        /// </summary>
-        public virtual SqlBinaryExpression Coalesce(SqlExpression left, SqlExpression right, CoreTypeMapping typeMapping = null)
-            => MakeBinary(ExpressionType.Coalesce, left, right, typeMapping);
-
         private SqlUnaryExpression MakeUnary(
             ExpressionType operatorType, SqlExpression operand, Type type, CoreTypeMapping typeMapping = null)
         {
diff --git a/src/EFCore.Relational/Query/ISqlExpressionFactory.cs b/src/EFCore.Relational/Query/ISqlExpressionFactory.cs
@@ -79,7 +79,7 @@ SqlBinaryExpression Or(
             [NotNull] SqlExpression left, [NotNull] SqlExpression right, [CanBeNull] RelationalTypeMapping typeMapping = null);
 
         // Other
-        SqlBinaryExpression Coalesce(
+        SqlFunctionExpression Coalesce(
             [NotNull] SqlExpression left, [NotNull] SqlExpression right, [CanBeNull] RelationalTypeMapping typeMapping = null);
 
         SqlUnaryExpression IsNull([NotNull] SqlExpression operand);
diff --git a/src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs
@@ -624,6 +624,23 @@ protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunction
             var canOptimize = _canOptimize;
             _canOptimize = false;
 
+            if (sqlFunctionExpression.IsBuiltIn
+                && string.Equals(sqlFunctionExpression.Name, "COALESCE", StringComparison.OrdinalIgnoreCase))
+            {
+                _isNullable = false;
+                var newLeft = (SqlExpression)Visit(sqlFunctionExpression.Arguments[0]);
+                var leftNullable = _isNullable;
+
+                _isNullable = false;
+                var newRight = (SqlExpression)Visit(sqlFunctionExpression.Arguments[1]);
+                var rightNullable = _isNullable;
+
+                _isNullable = leftNullable && rightNullable;
+                _canOptimize = canOptimize;
+
+                return sqlFunctionExpression.Update(sqlFunctionExpression.Instance, new[] { newLeft, newRight });
+            }
+
             var newInstance = (SqlExpression)Visit(sqlFunctionExpression.Instance);
             var newArguments = new SqlExpression[sqlFunctionExpression.Arguments.Count];
             for (var i = 0; i < newArguments.Length; i++)
diff --git a/src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs
@@ -250,9 +250,6 @@ private SqlExpression SimplifyNullNotNullExpression(
                             // in general:
                             // binaryOp(a, b) == null -> a == null || b == null
                             // binaryOp(a, b) != null -> a != null && b != null
-                            // for coalesce:
-                            // (a ?? b) == null -> a == null && b == null
-                            // (a ?? b) != null -> a != null || b != null
                             // for AndAlso, OrElse we can't do this optimization
                             // we could do something like this, but it seems too complicated:
                             // (a && b) == null -> a == null && b != 0 || a != 0 && b == null
@@ -262,15 +259,7 @@ private SqlExpression SimplifyNullNotNullExpression(
                                 var newLeft = SimplifyNullNotNullExpression(operatorType, sqlBinaryOperand.Left, typeof(bool), typeMapping);
                                 var newRight = SimplifyNullNotNullExpression(operatorType, sqlBinaryOperand.Right, typeof(bool), typeMapping);
 
-                                return sqlBinaryOperand.OperatorType == ExpressionType.Coalesce
-                                    ? SimplifyLogicalSqlBinaryExpression(
-                                        operatorType == ExpressionType.Equal
-                                            ? ExpressionType.AndAlso
-                                            : ExpressionType.OrElse,
-                                        newLeft,
-                                        newRight,
-                                        typeMapping)
-                                    : SimplifyLogicalSqlBinaryExpression(
+                                return SimplifyLogicalSqlBinaryExpression(
                                         operatorType == ExpressionType.Equal
                                             ? ExpressionType.OrElse
                                             : ExpressionType.AndAlso,
@@ -279,6 +268,25 @@ private SqlExpression SimplifyNullNotNullExpression(
                                         typeMapping);
                             }
                             break;
+
+                        case SqlFunctionExpression sqlFunctionExpression
+                            when sqlFunctionExpression.IsBuiltIn
+                            && string.Equals("COALESCE", sqlFunctionExpression.Name, StringComparison.OrdinalIgnoreCase):
+                            // for coalesce:
+                            // (a ?? b) == null -> a == null && b == null
+                            // (a ?? b) != null -> a != null || b != null
+                            var leftArgument = SimplifyNullNotNullExpression(
+                                operatorType, sqlFunctionExpression.Arguments[0], typeof(bool), typeMapping);
+                            var rightArgument = SimplifyNullNotNullExpression(
+                                operatorType, sqlFunctionExpression.Arguments[1], typeof(bool), typeMapping);
+
+                            return SimplifyLogicalSqlBinaryExpression(
+                                operatorType == ExpressionType.Equal
+                                    ? ExpressionType.AndAlso
+                                    : ExpressionType.OrElse,
+                                        leftArgument,
+                                        rightArgument,
+                                        typeMapping);
                     }
                     break;
             }
diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs
@@ -375,56 +375,41 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
         {
             Check.NotNull(sqlBinaryExpression, nameof(sqlBinaryExpression));
 
-            if (sqlBinaryExpression.OperatorType == ExpressionType.Coalesce)
+            var requiresBrackets = RequiresBrackets(sqlBinaryExpression.Left);
+
+            if (requiresBrackets)
             {
-                _relationalCommandBuilder.Append("COALESCE(");
-                Visit(sqlBinaryExpression.Left);
-                _relationalCommandBuilder.Append(", ");
-                Visit(sqlBinaryExpression.Right);
-                _relationalCommandBuilder.Append(")");
+                _relationalCommandBuilder.Append("(");
             }
-            else
-            {
-                var requiresBrackets = RequiresBrackets(sqlBinaryExpression.Left);
 
-                if (requiresBrackets)
-                {
-                    _relationalCommandBuilder.Append("(");
-                }
+            Visit(sqlBinaryExpression.Left);
 
-                Visit(sqlBinaryExpression.Left);
-
-                if (requiresBrackets)
-                {
-                    _relationalCommandBuilder.Append(")");
-                }
+            if (requiresBrackets)
+            {
+                _relationalCommandBuilder.Append(")");
+            }
 
-                _relationalCommandBuilder.Append(GenerateOperator(sqlBinaryExpression));
+            _relationalCommandBuilder.Append(GenerateOperator(sqlBinaryExpression));
 
-                requiresBrackets = RequiresBrackets(sqlBinaryExpression.Right);
+            requiresBrackets = RequiresBrackets(sqlBinaryExpression.Right);
 
-                if (requiresBrackets)
-                {
-                    _relationalCommandBuilder.Append("(");
-                }
+            if (requiresBrackets)
+            {
+                _relationalCommandBuilder.Append("(");
+            }
 
-                Visit(sqlBinaryExpression.Right);
+            Visit(sqlBinaryExpression.Right);
 
-                if (requiresBrackets)
-                {
-                    _relationalCommandBuilder.Append(")");
-                }
+            if (requiresBrackets)
+            {
+                _relationalCommandBuilder.Append(")");
             }
 
             return sqlBinaryExpression;
         }
 
-        private bool RequiresBrackets(SqlExpression expression)
-        {
-            return expression is SqlBinaryExpression sqlBinary
-                && sqlBinary.OperatorType != ExpressionType.Coalesce
-                || expression is LikeExpression;
-        }
+        private static bool RequiresBrackets(SqlExpression expression)
+            => expression is SqlBinaryExpression || expression is LikeExpression;
 
         protected override Expression VisitSqlConstant(SqlConstantExpression sqlConstantExpression)
         {
diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
@@ -512,11 +512,13 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
             return TranslationFailed(binaryExpression.Left, Visit(left), out var sqlLeft)
                 || TranslationFailed(binaryExpression.Right, Visit(right), out var sqlRight)
                 ? null
-                : SqlExpressionFactory.MakeBinary(
-                    binaryExpression.NodeType,
-                    sqlLeft,
-                    sqlRight,
-                    null);
+                : binaryExpression.NodeType == ExpressionType.Coalesce
+                    ? SqlExpressionFactory.Coalesce(sqlLeft, sqlRight)
+                    : (Expression)SqlExpressionFactory.MakeBinary(
+                        binaryExpression.NodeType,
+                        sqlLeft,
+                        sqlRight,
+                        null);
         }
 
         private SqlConstantExpression GetConstantOrNull(Expression expression)
diff --git a/src/EFCore.Relational/Query/SqlExpressionFactory.cs b/src/EFCore.Relational/Query/SqlExpressionFactory.cs
@@ -180,7 +180,6 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
                 case ExpressionType.Multiply:
                 case ExpressionType.Divide:
                 case ExpressionType.Modulo:
-                case ExpressionType.Coalesce:
                 case ExpressionType.And:
                 case ExpressionType.Or:
                 {
@@ -358,12 +357,27 @@ public virtual SqlBinaryExpression Or(SqlExpression left, SqlExpression right, R
             return MakeBinary(ExpressionType.Or, left, right, typeMapping);
         }
 
-        public virtual SqlBinaryExpression Coalesce(SqlExpression left, SqlExpression right, RelationalTypeMapping typeMapping = null)
+        public virtual SqlFunctionExpression Coalesce(SqlExpression left, SqlExpression right, RelationalTypeMapping typeMapping = null)
         {
             Check.NotNull(left, nameof(left));
             Check.NotNull(right, nameof(right));
 
-            return MakeBinary(ExpressionType.Coalesce, left, right, typeMapping);
+            var resultType = right.Type;
+            var inferredTypeMapping = typeMapping
+                ?? ExpressionExtensions.InferTypeMapping(left, right)
+                ?? _typeMappingSource.FindMapping(resultType);
+
+            var typeMappedArguments = new List<SqlExpression>()
+            {
+                ApplyTypeMapping(left, inferredTypeMapping),
+                ApplyTypeMapping(right, inferredTypeMapping)
+            };
+
+            return SqlFunctionExpression.Create(
+                "COALESCE",
+                typeMappedArguments,
+                resultType,
+                inferredTypeMapping);
         }
 
         public virtual SqlUnaryExpression MakeUnary(
@@ -677,7 +691,7 @@ private void AddConditions(
 
                 if (sharingTypes.Count > 0)
                 {
-                    bool discriminatorAdded = AddDiscriminatorCondition(selectExpression, entityType);
+                    var discriminatorAdded = AddDiscriminatorCondition(selectExpression, entityType);
 
                     var linkingFks = entityType.GetRootType().FindForeignKeys(entityType.FindPrimaryKey().Properties)
                         .Where(
diff --git a/src/EFCore.Relational/Query/SqlExpressions/SqlBinaryExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SqlBinaryExpression.cs
@@ -31,7 +31,6 @@ public class SqlBinaryExpression : SqlExpression
             ExpressionType.Equal,
             ExpressionType.NotEqual,
             //ExpressionType.ExclusiveOr,
-            ExpressionType.Coalesce
             //ExpressionType.ArrayIndex,
             //ExpressionType.RightShift,
             //ExpressionType.LeftShift,
@@ -116,13 +115,8 @@ public override void Print(ExpressionPrinter expressionPrinter)
             {
                 expressionPrinter.Append(")");
             }
-        }
 
-        private bool RequiresBrackets(SqlExpression expression)
-        {
-            return expression is SqlBinaryExpression sqlBinary
-                && sqlBinary.OperatorType != ExpressionType.Coalesce
-                || expression is LikeExpression;
+            static bool RequiresBrackets(SqlExpression expression) => expression is SqlBinaryExpression || expression is LikeExpression;
         }
 
         public override bool Equals(object obj)

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,6 @@ public class SqlBinaryExpression : SqlExpression`
`37`	`37`	`ExpressionType.Equal,`
`38`	`38`	`ExpressionType.NotEqual,`
`39`	`39`	`ExpressionType.ExclusiveOr,`
`40`		`- ExpressionType.Coalesce,`
`41`	`40`	`ExpressionType.RightShift,`
`42`	`41`	`ExpressionType.LeftShift`
`43`	`42`	`};`
`@@ -160,12 +159,8 @@ public override void Print(ExpressionPrinter expressionPrinter)`
`160`	`159`	`{`
`161`	`160`	`expressionPrinter.Append(")");`
`162`	`161`	`}`
`163`		`- }`
`164`	`162`
`165`		`- private bool RequiresBrackets(SqlExpression expression)`
`166`		`- {`
`167`		`- return expression is SqlBinaryExpression sqlBinary`
`168`		`- && sqlBinary.OperatorType != ExpressionType.Coalesce;`
	`163`	`+ static bool RequiresBrackets(SqlExpression expression) => expression is SqlBinaryExpression;`
`169`	`164`	`}`
`170`	`165`
`171`	`166`	`/// <summary>`