Additional refactoring of Null Semantics:

- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689

Author: maumar

src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Data.Common;
 using System.Linq.Expressions;
@@ -46,17 +45,19 @@ public virtual (SelectExpression selectExpression, bool canCache) Optimize(
             Check.NotNull(parametersValues, nameof(parametersValues));
 
             var canCache = true;
+            var nullabilityHandlingExpressionVisitor = new NullabilityHandlingExpressionVisitor(
+                UseRelationalNulls,
+                Dependencies.SqlExpressionFactory,
+                parametersValues);
 
-            var inExpressionOptimized = new InExpressionValuesExpandingExpressionVisitor(
-                Dependencies.SqlExpressionFactory, parametersValues).Visit(selectExpression);
-
-            if (!ReferenceEquals(selectExpression, inExpressionOptimized))
+            var nullabilityHandled = nullabilityHandlingExpressionVisitor.Visit(selectExpression);
+            if (!nullabilityHandlingExpressionVisitor.CanCache)
             {
                 canCache = false;
             }
 
-            var nullParametersOptimized = new ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
-                Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(inExpressionOptimized);
+            var nullParametersOptimized = new SqlExpressionOptimizingExpressionVisitor(
+                Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(nullabilityHandled);
 
             var fromSqlParameterOptimized = new FromSqlParameterApplyingExpressionVisitor(
                 Dependencies.SqlExpressionFactory,
@@ -71,163 +72,6 @@ public virtual (SelectExpression selectExpression, bool canCache) Optimize(
             return (selectExpression: (SelectExpression)fromSqlParameterOptimized, canCache);
         }
 
-        private sealed class ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor : SqlExpressionOptimizingExpressionVisitor
-        {
-            private readonly IReadOnlyDictionary<string, object> _parametersValues;
-
-            public ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
-                ISqlExpressionFactory sqlExpressionFactory,
-                bool useRelationalNulls,
-                IReadOnlyDictionary<string, object> parametersValues)
-                : base(sqlExpressionFactory, useRelationalNulls)
-            {
-                _parametersValues = parametersValues;
-            }
-
-            protected override Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
-            {
-                var result = base.VisitSqlUnaryExpression(sqlUnaryExpression);
-                if (result is SqlUnaryExpression newUnaryExpression
-                    && newUnaryExpression.Operand is SqlParameterExpression parameterOperand)
-                {
-                    var parameterValue = _parametersValues[parameterOperand.Name];
-                    if (sqlUnaryExpression.OperatorType == ExpressionType.Equal)
-                    {
-                        return SqlExpressionFactory.Constant(parameterValue == null, sqlUnaryExpression.TypeMapping);
-                    }
-
-                    if (sqlUnaryExpression.OperatorType == ExpressionType.NotEqual)
-                    {
-                        return SqlExpressionFactory.Constant(parameterValue != null, sqlUnaryExpression.TypeMapping);
-                    }
-                }
-
-                return result;
-            }
-
-            protected override Expression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression)
-            {
-                var result = base.VisitSqlBinaryExpression(sqlBinaryExpression);
-                if (result is SqlBinaryExpression sqlBinaryResult)
-                {
-                    var leftNullParameter = sqlBinaryResult.Left is SqlParameterExpression leftParameter
-                        && _parametersValues[leftParameter.Name] == null;
-
-                    var rightNullParameter = sqlBinaryResult.Right is SqlParameterExpression rightParameter
-                        && _parametersValues[rightParameter.Name] == null;
-
-                    if ((sqlBinaryResult.OperatorType == ExpressionType.Equal || sqlBinaryResult.OperatorType == ExpressionType.NotEqual)
-                        && (leftNullParameter || rightNullParameter))
-                    {
-                        return SimplifyNullComparisonExpression(
-                            sqlBinaryResult.OperatorType,
-                            sqlBinaryResult.Left,
-                            sqlBinaryResult.Right,
-                            leftNullParameter,
-                            rightNullParameter,
-                            sqlBinaryResult.TypeMapping);
-                    }
-                }
-
-                return result;
-            }
-        }
-
-        private sealed class InExpressionValuesExpandingExpressionVisitor : ExpressionVisitor
-        {
-            private readonly ISqlExpressionFactory _sqlExpressionFactory;
-            private readonly IReadOnlyDictionary<string, object> _parametersValues;
-
-            public InExpressionValuesExpandingExpressionVisitor(
-                ISqlExpressionFactory sqlExpressionFactory, IReadOnlyDictionary<string, object> parametersValues)
-            {
-                _sqlExpressionFactory = sqlExpressionFactory;
-                _parametersValues = parametersValues;
-            }
-
-            public override Expression Visit(Expression expression)
-            {
-                if (expression is InExpression inExpression
-                    && inExpression.Values != null)
-                {
-                    var inValues = new List<object>();
-                    var hasNullValue = false;
-                    RelationalTypeMapping typeMapping = null;
-
-                    switch (inExpression.Values)
-                    {
-                        case SqlConstantExpression sqlConstant:
-                        {
-                            typeMapping = sqlConstant.TypeMapping;
-                            var values = (IEnumerable)sqlConstant.Value;
-                            foreach (var value in values)
-                            {
-                                if (value == null)
-                                {
-                                    hasNullValue = true;
-                                    continue;
-                                }
-
-                                inValues.Add(value);
-                            }
-
-                            break;
-                        }
-
-                        case SqlParameterExpression sqlParameter:
-                        {
-                            typeMapping = sqlParameter.TypeMapping;
-                            var values = (IEnumerable)_parametersValues[sqlParameter.Name];
-                            foreach (var value in values)
-                            {
-                                if (value == null)
-                                {
-                                    hasNullValue = true;
-                                    continue;
-                                }
-
-                                inValues.Add(value);
-                            }
-
-                            break;
-                        }
-                    }
-
-                    var updatedInExpression = inValues.Count > 0
-                        ? _sqlExpressionFactory.In(
-                            (SqlExpression)Visit(inExpression.Item),
-                            _sqlExpressionFactory.Constant(inValues, typeMapping),
-                            inExpression.IsNegated)
-                        : null;
-
-                    var nullCheckExpression = hasNullValue
-                        ? inExpression.IsNegated
-                            ? _sqlExpressionFactory.IsNotNull(inExpression.Item)
-                            : _sqlExpressionFactory.IsNull(inExpression.Item)
-                        : null;
-
-                    if (updatedInExpression != null
-                        && nullCheckExpression != null)
-                    {
-                        return inExpression.IsNegated
-                            ? _sqlExpressionFactory.AndAlso(updatedInExpression, nullCheckExpression)
-                            : _sqlExpressionFactory.OrElse(updatedInExpression, nullCheckExpression);
-                    }
-
-                    if (updatedInExpression == null
-                        && nullCheckExpression == null)
-                    {
-                        return _sqlExpressionFactory.Equal(
-                            _sqlExpressionFactory.Constant(true), _sqlExpressionFactory.Constant(inExpression.IsNegated));
-                    }
-
-                    return (SqlExpression)updatedInExpression ?? nullCheckExpression;
-                }
-
-                return base.Visit(expression);
-            }
-        }
-
         private sealed class FromSqlParameterApplyingExpressionVisitor : ExpressionVisitor
         {
             private readonly IDictionary<FromSqlExpression, Expression> _visitedFromSqlExpressions

src/EFCore.Relational/Query/Internal/NullabilityHandlingExpressionVisitor.cs

@@ -11,8 +11,6 @@ namespace Microsoft.EntityFrameworkCore.Query
 {
     public class RelationalQueryTranslationPostprocessor : QueryTranslationPostprocessor
     {
-        private readonly SqlExpressionOptimizingExpressionVisitor _sqlExpressionOptimizingExpressionVisitor;
-
         public RelationalQueryTranslationPostprocessor(
             [NotNull] QueryTranslationPostprocessorDependencies dependencies,
             [NotNull] RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
@@ -25,8 +23,6 @@ public RelationalQueryTranslationPostprocessor(
             RelationalDependencies = relationalDependencies;
             UseRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls;
             SqlExpressionFactory = relationalDependencies.SqlExpressionFactory;
-            _sqlExpressionOptimizingExpressionVisitor
-                = new SqlExpressionOptimizingExpressionVisitor(SqlExpressionFactory, UseRelationalNulls);
         }
 
         protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; }
@@ -42,22 +38,12 @@ public override Expression Process(Expression query)
             query = new CollectionJoinApplyingExpressionVisitor().Visit(query);
             query = new TableAliasUniquifyingExpressionVisitor().Visit(query);
             query = new CaseWhenFlatteningExpressionVisitor(SqlExpressionFactory).Visit(query);
-
-            if (!UseRelationalNulls)
-            {
-                query = new NullSemanticsRewritingExpressionVisitor(SqlExpressionFactory).Visit(query);
-            }
-
             query = OptimizeSqlExpression(query);
 
             return query;
         }
 
         protected virtual Expression OptimizeSqlExpression([NotNull] Expression query)
-        {
-            Check.NotNull(query, nameof(query));
-
-            return _sqlExpressionOptimizingExpressionVisitor.Visit(query);
-        }
+            => query;
     }
 }

src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs

@@ -54,11 +54,11 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
         {
             Check.NotNull(visitor, nameof(visitor));
 
-            var newItem = (SqlExpression)visitor.Visit(Item);
+            var item = (SqlExpression)visitor.Visit(Item);
             var subquery = (SelectExpression)visitor.Visit(Subquery);
             var values = (SqlExpression)visitor.Visit(Values);
 
-            return Update(newItem, values, subquery);
+            return Update(item, values, subquery);
         }
 
         public virtual InExpression Negate() => new InExpression(Item, !IsNegated, Values, Subquery, TypeMapping);

src/EFCore.Relational/Query/RelationalParameterBasedQueryTranslationPostprocessor.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Query;
+using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Utilities;
 
@@ -30,7 +31,10 @@ public override (SelectExpression selectExpression, bool canCache) Optimize(
             var searchConditionOptimized = (SelectExpression)new SearchConditionConvertingExpressionVisitor(Dependencies.SqlExpressionFactory)
                 .Visit(optimizedSelectExpression);
 
-            return (searchConditionOptimized, canCache);
+            var optimized = (SelectExpression)new SqlExpressionOptimizingExpressionVisitor(
+                Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(searchConditionOptimized);
+
+            return (optimized, canCache);
         }
     }
 }

src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs

@@ -1,7 +1,6 @@
 // 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.Linq.Expressions;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Query;
 
@@ -16,13 +15,5 @@ public SqlServerQueryTranslationPostprocessor(
             : base(dependencies, relationalDependencies, queryCompilationContext)
         {
         }
-
-        public override Expression Process(Expression query)
-        {
-            query = base.Process(query);
-            query = new SearchConditionConvertingExpressionVisitor(SqlExpressionFactory).Visit(query);
-
-            return query;
-        }
     }
 }