Implementing C# null semantics in the new pipeline

Currently we always apply "full" null semantics translation, meaning null values are completely removed from the tree. We could produce slightly simpler translation where it doesn't matter if we return NULL or FALSE (e.g. in predicates)
Also, currently when testing if a given subtree is null we do it in naive way, simply adding IsNull call around entire subtree. Instead we can test it's constituents, but to do it properly we need to persist nullability information.

src/EFCore.Relational/Query/PipeLine/SqlExpressionOptimizingVisitor.cs

@@ -0,0 +1,196 @@
+// 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.Collections.Generic;
+using System.Linq.Expressions;
+using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline;
+using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
+
+namespace Microsoft.EntityFrameworkCore.Query.Pipeline
+{
+    public class SqlExpressionOptimizingVisitor : ExpressionVisitor
+    {
+        private readonly ISqlExpressionFactory _sqlExpressionFactory;
+
+        private readonly Dictionary<ExpressionType, ExpressionType> _expressionTypesNegationMap
+            = new Dictionary<ExpressionType, ExpressionType>
+            {
+                { ExpressionType.AndAlso, ExpressionType.OrElse },
+                { ExpressionType.OrElse, ExpressionType.AndAlso },
+                { ExpressionType.Equal, ExpressionType.NotEqual },
+                { ExpressionType.NotEqual, ExpressionType.Equal },
+                { ExpressionType.GreaterThan, ExpressionType.LessThanOrEqual },
+                { ExpressionType.GreaterThanOrEqual, ExpressionType.LessThan },
+                { ExpressionType.LessThan, ExpressionType.GreaterThanOrEqual },
+                { ExpressionType.LessThanOrEqual, ExpressionType.GreaterThan },
+            };
+
+        public SqlExpressionOptimizingVisitor(ISqlExpressionFactory sqlExpressionFactory)
+        {
+            _sqlExpressionFactory = sqlExpressionFactory;
+        }
+
+        protected override Expression VisitExtension(Expression extensionExpression)
+        {
+            if (extensionExpression is SqlUnaryExpression sqlUnaryExpression)
+            {
+                return VisitSqlUnaryExpression(sqlUnaryExpression);
+            }
+
+            if (extensionExpression is SqlBinaryExpression sqlBinaryExpression)
+            {
+                return VisitSqlBinaryExpression(sqlBinaryExpression);
+            }
+
+            return base.VisitExtension(extensionExpression);
+        }
+
+        private Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
+        {
+            // !(true) -> false
+            // !(false) -> true
+            if (sqlUnaryExpression.OperatorType == ExpressionType.Not
+                && sqlUnaryExpression.Operand is SqlConstantExpression innerConstantBool
+                && innerConstantBool.Value is bool value)
+            {
+                return value
+                    ? _sqlExpressionFactory.Constant(false, sqlUnaryExpression.TypeMapping)
+                    : _sqlExpressionFactory.Constant(true, sqlUnaryExpression.TypeMapping);
+            }
+
+            // NULL IS NULL -> true
+            // non_nullablee_constant IS NULL -> false
+            if (sqlUnaryExpression.OperatorType == ExpressionType.Equal
+                && sqlUnaryExpression.Operand is SqlConstantExpression innerConstantNull1)
+            {
+                return _sqlExpressionFactory.Constant(innerConstantNull1.Value == null, sqlUnaryExpression.TypeMapping);
+            }
+
+            // NULL IS NOT NULL -> false
+            // non_nullablee_constant IS NOT NULL -> true
+            if (sqlUnaryExpression.OperatorType == ExpressionType.NotEqual
+                && sqlUnaryExpression.Operand is SqlConstantExpression innerConstantNull2)
+            {
+                return _sqlExpressionFactory.Constant(innerConstantNull2.Value != null, sqlUnaryExpression.TypeMapping);
+            }
+
+            if (sqlUnaryExpression.Operand is SqlUnaryExpression innerUnary)
+            {
+                if (sqlUnaryExpression.OperatorType == ExpressionType.Not)
+                {
+                    // !(!a) -> a
+                    if (innerUnary.OperatorType == ExpressionType.Not)
+                    {
+                        return Visit(innerUnary.Operand);
+                    }
+
+                    if (innerUnary.OperatorType == ExpressionType.Equal)
+                    {
+                        //!(a IS NULL) -> a IS NOT NULL
+                        return Visit(_sqlExpressionFactory.IsNotNull(innerUnary.Operand));
+                    }
+
+                    //!(a IS NOT NULL) -> a IS NULL
+                    if (innerUnary.OperatorType == ExpressionType.NotEqual)
+                    {
+                        return Visit(_sqlExpressionFactory.IsNull(innerUnary.Operand));
+                    }
+                }
+
+                // (!a) IS NULL <==> a IS NULL
+                if (sqlUnaryExpression.OperatorType == ExpressionType.Equal
+                    && innerUnary.OperatorType == ExpressionType.Not)
+                {
+                    return Visit(_sqlExpressionFactory.IsNull(innerUnary.Operand));
+                }
+
+                // (!a) IS NOT NULL <==> a IS NOT NULL
+                if (sqlUnaryExpression.OperatorType == ExpressionType.NotEqual
+                    && innerUnary.OperatorType == ExpressionType.Not)
+                {
+                    return Visit(_sqlExpressionFactory.IsNotNull(innerUnary.Operand));
+                }
+            }
+
+            if (sqlUnaryExpression.Operand is SqlBinaryExpression innerBinary)
+            {
+                // De Morgan's
+                if (innerBinary.OperatorType == ExpressionType.AndAlso
+                    || innerBinary.OperatorType == ExpressionType.OrElse)
+                {
+                    var newLeft = (SqlExpression)Visit(_sqlExpressionFactory.Not(innerBinary.Left));
+                    var newRight = (SqlExpression)Visit(_sqlExpressionFactory.Not(innerBinary.Right));
+
+                    return innerBinary.OperatorType == ExpressionType.AndAlso
+                        ? _sqlExpressionFactory.OrElse(newLeft, newRight)
+                        : _sqlExpressionFactory.AndAlso(newLeft, newRight);
+                }
+
+                // note that those optimizations are only valid in 2-value logic
+                // they are safe to do here because null semantics removes possibility of nulls in the tree
+                // however if we decide to do "partial" null semantics (that doesn't distinguish between NULL and FALSE, e.g. for predicates)
+                // we need to be extra careful here
+                if (_expressionTypesNegationMap.ContainsKey(innerBinary.OperatorType))
+                {
+                    return Visit(
+                        _sqlExpressionFactory.MakeBinary(
+                            _expressionTypesNegationMap[innerBinary.OperatorType],
+                            innerBinary.Left,
+                            innerBinary.Right,
+                            innerBinary.TypeMapping));
+                }
+            }
+
+            var newOperand = (SqlExpression)Visit(sqlUnaryExpression.Operand);
+
+            return sqlUnaryExpression.Update(newOperand);
+        }
+
+        private Expression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression)
+        {
+            var newLeft = (SqlExpression)Visit(sqlBinaryExpression.Left);
+            var newRight = (SqlExpression)Visit(sqlBinaryExpression.Right);
+
+            if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
+                || sqlBinaryExpression.OperatorType == ExpressionType.OrElse)
+            {
+
+                var newLeftConstant = newLeft as SqlConstantExpression;
+                var newRightConstant = newRight as SqlConstantExpression;
+
+                // true && a -> a
+                // true || a -> true
+                // false && a -> false
+                // false || a -> a 
+                if (newLeftConstant != null)
+                {
+                    return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
+                        ? (bool)newLeftConstant.Value
+                            ? newRight
+                            : newLeftConstant
+                        : (bool)newLeftConstant.Value
+                            ? newLeftConstant
+                            : newRight;
+                }
+                else if (newRightConstant != null)
+                {
+                    // a && true -> a
+                    // a || true -> true 
+                    // a && false -> false
+                    // a || false -> a
+                    return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
+                        ? (bool)newRightConstant.Value
+                            ? newLeft
+                            : newRightConstant
+                        : (bool)newRightConstant.Value
+                            ? newRightConstant
+                            : newLeft;
+                }
+
+                return sqlBinaryExpression.Update(newLeft, newRight);
+            }
+
+            return sqlBinaryExpression.Update(newLeft, newRight);
+        }
+    }
+}

src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryOptimizingExpressionVisitors.cs

@@ -2,25 +2,38 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Linq.Expressions;
+using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Query.Pipeline;
 
 namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline
 {
     public class RelationalShapedQueryOptimizer : ShapedQueryOptimizer
     {
-        private QueryCompilationContext2 _queryCompilationContext;
+        private readonly QueryCompilationContext2 _queryCompilationContext;
 
-        public RelationalShapedQueryOptimizer(QueryCompilationContext2 queryCompilationContext)
+        public RelationalShapedQueryOptimizer(
+            QueryCompilationContext2 queryCompilationContext,
+            ISqlExpressionFactory sqlExpressionFactory)
         {
             _queryCompilationContext = queryCompilationContext;
+            SqlExpressionFactory = sqlExpressionFactory;
         }
 
+        protected ISqlExpressionFactory SqlExpressionFactory { get; private set; }
+
         public override Expression Visit(Expression query)
         {
             query = base.Visit(query);
             query = new ShaperExpressionDedupingExpressionVisitor().Process(query);
             query = new SelectExpressionProjectionApplyingExpressionVisitor().Visit(query);
             query = new SelectExpressionTableAliasUniquifyingExpressionVisitor().Visit(query);
+
+            if (!RelationalOptionsExtension.Extract(_queryCompilationContext.ContextOptions).UseRelationalNulls)
+            {
+                query = new NullSemanticsRewritingVisitor(SqlExpressionFactory).Visit(query);
+            }
+
+            query = new SqlExpressionOptimizingVisitor(SqlExpressionFactory).Visit(query);
             query = new NullComparisonTransformingExpressionVisitor().Visit(query);
 
             return query;

src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryOptimizingExpressionVisitorsFactory.cs

@@ -7,9 +7,16 @@ namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline
 {
     public class RelationalShapedQueryOptimizerFactory : ShapedQueryOptimizerFactory
     {
+        protected ISqlExpressionFactory SqlExpressionFactory { get; private set; }
+
+        public RelationalShapedQueryOptimizerFactory(ISqlExpressionFactory sqlExpressionFactory)
+        {
+            SqlExpressionFactory = sqlExpressionFactory;
+        }
+
         public override ShapedQueryOptimizer Create(QueryCompilationContext2 queryCompilationContext)
         {
-            return new RelationalShapedQueryOptimizer(queryCompilationContext);
+            return new RelationalShapedQueryOptimizer(queryCompilationContext, SqlExpressionFactory);
         }
     }
 }

src/EFCore.Relational/Query/Pipeline/SqlExpressions/CaseExpression.cs

@@ -82,9 +82,13 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
         }
 
         public virtual CaseExpression Update(
-            SqlExpression operand, IReadOnlyList<CaseWhenClause> whenClauses, SqlExpression elseResult)
+            SqlExpression operand,
+            IReadOnlyList<CaseWhenClause> whenClauses,
+            SqlExpression elseResult)
         {
-            return new CaseExpression(operand, whenClauses, elseResult);
+            return operand != Operand || !whenClauses.SequenceEqual(WhenClauses) || elseResult != ElseResult
+                ? new CaseExpression(operand, whenClauses, elseResult)
+                : this;
         }
 
         #endregion

src/EFCore.Relational/Query/Pipeline/SqlExpressions/SqlBinaryExpression.cs

@@ -136,7 +136,7 @@ public override void Print(ExpressionPrinter expressionPrinter)
                 expressionPrinter.StringBuilder.Append(")");
             }
 
-            expressionPrinter.StringBuilder.Append(" " + expressionPrinter.GenerateBinaryOperator(OperatorType) + " ");
+            expressionPrinter.StringBuilder.Append(expressionPrinter.GenerateBinaryOperator(OperatorType));
 
             requiresBrackets = RequiresBrackets(Right);
 

src/EFCore.Relational/Query/Pipeline/SqlExpressions/SqlFunctionExpression.cs

@@ -136,7 +136,7 @@ public SqlFunctionExpression ApplyTypeMapping(RelationalTypeMapping typeMapping)
 
         public SqlFunctionExpression Update(SqlExpression instance, IReadOnlyList<SqlExpression> arguments)
         {
-            return instance != Instance || arguments != Arguments
+            return instance != Instance || !arguments.SequenceEqual(Arguments)
                 ? new SqlFunctionExpression(instance, Schema, FunctionName, IsNiladic, arguments, Type, TypeMapping)
                 : this;
         }

src/EFCore.SqlServer/Query/Pipeline/SearchConditionConvertingExpressionVisitor.cs

@@ -24,9 +24,9 @@ private Expression ApplyConversion(SqlExpression sqlExpression, bool condition)
                     : ConvertToValue(sqlExpression, condition);
 
         private Expression ConvertToSearchCondition(SqlExpression sqlExpression, bool condition)
-                => condition
-                    ? sqlExpression
-                    : BuildCompareToExpression(sqlExpression);
+            => condition
+                ? sqlExpression
+                : BuildCompareToExpression(sqlExpression);
 
         private Expression ConvertToValue(SqlExpression sqlExpression, bool condition)
         {
@@ -246,67 +246,15 @@ protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunction
         {
             var parentSearchCondition = _isSearchCondition;
             _isSearchCondition = false;
-            var changed = false;
             var instance = (SqlExpression)Visit(sqlFunctionExpression.Instance);
-            changed |= instance != sqlFunctionExpression.Instance;
             var arguments = new SqlExpression[sqlFunctionExpression.Arguments.Count];
             for (var i = 0; i < arguments.Length; i++)
             {
                 arguments[i] = (SqlExpression)Visit(sqlFunctionExpression.Arguments[i]);
-                changed |= arguments[i] != sqlFunctionExpression.Arguments[i];
             }
 
             _isSearchCondition = parentSearchCondition;
-            SqlExpression newFunction;
-            if (changed)
-            {
-                if (sqlFunctionExpression.Instance != null)
-                {
-                    if (sqlFunctionExpression.IsNiladic)
-                    {
-                        newFunction = _sqlExpressionFactory.Function(
-                            instance,
-                            sqlFunctionExpression.FunctionName,
-                            sqlFunctionExpression.IsNiladic,
-                            sqlFunctionExpression.Type,
-                            sqlFunctionExpression.TypeMapping);
-                    }
-                    else
-                    {
-                        newFunction = _sqlExpressionFactory.Function(
-                            instance,
-                            sqlFunctionExpression.FunctionName,
-                            arguments,
-                            sqlFunctionExpression.Type,
-                            sqlFunctionExpression.TypeMapping);
-                    }
-                }
-                else
-                {
-                    if (sqlFunctionExpression.IsNiladic)
-                    {
-                        newFunction = _sqlExpressionFactory.Function(
-                            sqlFunctionExpression.Schema,
-                            sqlFunctionExpression.FunctionName,
-                            sqlFunctionExpression.IsNiladic,
-                            sqlFunctionExpression.Type,
-                            sqlFunctionExpression.TypeMapping);
-                    }
-                    else
-                    {
-                        newFunction = _sqlExpressionFactory.Function(
-                            sqlFunctionExpression.Schema,
-                            sqlFunctionExpression.FunctionName,
-                            arguments,
-                            sqlFunctionExpression.Type,
-                            sqlFunctionExpression.TypeMapping);
-                    }
-                }
-            }
-            else
-            {
-                newFunction = sqlFunctionExpression;
-            }
+            var newFunction = sqlFunctionExpression.Update(instance, arguments);
 
             var condition = string.Equals(sqlFunctionExpression.FunctionName, "FREETEXT")
                 || string.Equals(sqlFunctionExpression.FunctionName, "CONTAINS");