Fix to #18492 - Query: inject SearchConditionConvertingExpressionVisi…

…tor into ParameterValueBasedSelectExpressionOptimizer

Using DI to create parameter value based postprocessor, which allows for provider-specific optimizations, including SearchConditionConvertingExpressionVisitor

Resolves #18492
Resolved #18940

src/EFCore.Relational/Infrastructure/EntityFrameworkRelationalServicesBuilder.cs

@@ -89,7 +89,7 @@ public static readonly IDictionary<Type, ServiceCharacteristics> RelationalServi
                 { typeof(IRelationalTypeMappingSourcePlugin), new ServiceCharacteristics(ServiceLifetime.Singleton, multipleRegistrations: true) },
                 { typeof(IMethodCallTranslatorPlugin), new ServiceCharacteristics(ServiceLifetime.Singleton, multipleRegistrations: true) },
                 { typeof(IMemberTranslatorPlugin), new ServiceCharacteristics(ServiceLifetime.Singleton, multipleRegistrations: true) },
-
+                { typeof(IRelationalParameterBasedQueryTranslationPostprocessorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) }
             };
 
         /// <summary>
@@ -166,6 +166,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
             TryAdd<IRelationalSqlTranslatingExpressionVisitorFactory, RelationalSqlTranslatingExpressionVisitorFactory>();
             TryAdd<ISqlExpressionFactory, SqlExpressionFactory>();
             TryAdd<IQueryTranslationPreprocessorFactory, RelationalQueryTranslationPreprocessorFactory>();
+            TryAdd<IRelationalParameterBasedQueryTranslationPostprocessorFactory, RelationalParameterBasedQueryTranslationPostprocessorFactory>();
 
             ServiceCollectionMap.GetInfrastructure()
                 .AddDependencySingleton<RelationalSqlGenerationHelperDependencies>()
@@ -187,6 +188,7 @@ public override EntityFrameworkServicesBuilder TryAddCoreServices()
                 .AddDependencySingleton<RelationalQueryTranslationPostprocessorDependencies>()
                 .AddDependencySingleton<RelationalEvaluatableExpressionFilterDependencies>()
                 .AddDependencySingleton<RelationalQueryTranslationPreprocessorDependencies>()
+                .AddDependencySingleton<RelationalParameterBasedQueryTranslationPostprocessorDependencies>()
                 .AddDependencyScoped<MigrationsSqlGeneratorDependencies>()
                 .AddDependencyScoped<RelationalConventionSetBuilderDependencies>()
                 .AddDependencyScoped<ModificationCommandBatchFactoryDependencies>()

src/EFCore.Relational/Query/IRelationalParameterBasedQueryTranslationPostprocessorFactory.cs

@@ -0,0 +1,23 @@
+// 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 Microsoft.EntityFrameworkCore.Query.Internal;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Microsoft.EntityFrameworkCore.Query
+{
+    /// <summary>
+    ///     <para>
+    ///         A factory for creating <see cref="RelationalParameterBasedQueryTranslationPostprocessor" /> instances.
+    ///     </para>
+    ///     <para>
+    ///         The service lifetime is <see cref="ServiceLifetime.Singleton" />. This means a single instance
+    ///         is used by many <see cref="DbContext" /> instances. The implementation must be thread-safe.
+    ///         This service cannot depend on services registered as <see cref="ServiceLifetime.Scoped" />.
+    ///     </para>
+    /// </summary>
+    public interface IRelationalParameterBasedQueryTranslationPostprocessorFactory
+    {
+        RelationalParameterBasedQueryTranslationPostprocessor Create(bool useRelationalNulls);
+    }
+}

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

@@ -24,24 +24,20 @@ private static readonly ConcurrentDictionary<object, object> _syncObjects
         private readonly IMemoryCache _memoryCache;
         private readonly IQuerySqlGeneratorFactory _querySqlGeneratorFactory;
         private readonly SelectExpression _selectExpression;
-        private readonly ParameterValueBasedSelectExpressionOptimizer _parameterValueBasedSelectExpressionOptimizer;
+        private readonly RelationalParameterBasedQueryTranslationPostprocessor _relationalParameterBasedQueryTranslationPostprocessor;
 
         public RelationalCommandCache(
             IMemoryCache memoryCache,
             ISqlExpressionFactory sqlExpressionFactory,
-            IParameterNameGeneratorFactory parameterNameGeneratorFactory,
             IQuerySqlGeneratorFactory querySqlGeneratorFactory,
+            IRelationalParameterBasedQueryTranslationPostprocessorFactory relationalParameterBasedQueryTranslationPostprocessorFactory,
             bool useRelationalNulls,
             SelectExpression selectExpression)
         {
             _memoryCache = memoryCache;
             _querySqlGeneratorFactory = querySqlGeneratorFactory;
             _selectExpression = selectExpression;
-
-            _parameterValueBasedSelectExpressionOptimizer = new ParameterValueBasedSelectExpressionOptimizer(
-                sqlExpressionFactory,
-                parameterNameGeneratorFactory,
-                useRelationalNulls);
+            _relationalParameterBasedQueryTranslationPostprocessor = relationalParameterBasedQueryTranslationPostprocessorFactory.Create(useRelationalNulls);
         }
 
         public virtual IRelationalCommand GetRelationalCommand(IReadOnlyDictionary<string, object> parameters)
@@ -59,7 +55,7 @@ public virtual IRelationalCommand GetRelationalCommand(IReadOnlyDictionary<strin
                 try
                 {
                     var (selectExpression, canCache) =
-                        _parameterValueBasedSelectExpressionOptimizer.Optimize(_selectExpression, parameters);
+                        _relationalParameterBasedQueryTranslationPostprocessor.Optimize(_selectExpression, parameters);
                     relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression);
 
                     if (canCache)

src/EFCore.Relational/Query/Internal/ParameterValueBasedSelectExpressionOptimizer.cs → src/EFCore.Relational/Query/Internal/RelationalParameterBasedQueryTranslationPostprocessor.cs

@@ -1,11 +1,10 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.Collections;
 using System.Collections.Generic;
 using System.Data.Common;
-using System.Linq;
 using System.Linq.Expressions;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
@@ -20,41 +19,39 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal
     ///     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 class ParameterValueBasedSelectExpressionOptimizer
+    public class RelationalParameterBasedQueryTranslationPostprocessor
     {
-        private readonly ISqlExpressionFactory _sqlExpressionFactory;
-        private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory;
-        private readonly bool _useRelationalNulls;
-
-        public ParameterValueBasedSelectExpressionOptimizer(
-            ISqlExpressionFactory sqlExpressionFactory,
-            IParameterNameGeneratorFactory parameterNameGeneratorFactory,
+        public RelationalParameterBasedQueryTranslationPostprocessor(
+            RelationalParameterBasedQueryTranslationPostprocessorDependencies dependencies,
             bool useRelationalNulls)
         {
-            _sqlExpressionFactory = sqlExpressionFactory;
-            _parameterNameGeneratorFactory = parameterNameGeneratorFactory;
-            _useRelationalNulls = useRelationalNulls;
+            Dependencies = dependencies;
+            UseRelationalNulls = useRelationalNulls;
         }
 
+        protected virtual RelationalParameterBasedQueryTranslationPostprocessorDependencies Dependencies { get; }
+
+        protected virtual bool UseRelationalNulls { get; }
+
         public virtual (SelectExpression selectExpression, bool canCache) Optimize(
             SelectExpression selectExpression, IReadOnlyDictionary<string, object> parametersValues)
         {
             var canCache = true;
 
             var inExpressionOptimized = new InExpressionValuesExpandingExpressionVisitor(
-                _sqlExpressionFactory, parametersValues).Visit(selectExpression);
+                Dependencies.SqlExpressionFactory, parametersValues).Visit(selectExpression);
 
             if (!ReferenceEquals(selectExpression, inExpressionOptimized))
             {
                 canCache = false;
             }
 
             var nullParametersOptimized = new ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
-                _sqlExpressionFactory, _useRelationalNulls, parametersValues).Visit(inExpressionOptimized);
+                Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(inExpressionOptimized);
 
             var fromSqlParameterOptimized = new FromSqlParameterApplyingExpressionVisitor(
-                _sqlExpressionFactory,
-                _parameterNameGeneratorFactory.Create(),
+                Dependencies.SqlExpressionFactory,
+                Dependencies.ParameterNameGeneratorFactory.Create(),
                 parametersValues).Visit(nullParametersOptimized);
 
             if (!ReferenceEquals(nullParametersOptimized, fromSqlParameterOptimized))
@@ -78,53 +75,6 @@ public ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
                 _parametersValues = parametersValues;
             }
 
-            protected override Expression VisitExtension(Expression extensionExpression)
-            {
-                // workaround for issue #18492
-                var newExpression = base.VisitExtension(extensionExpression);
-                if (newExpression is SelectExpression newSelectExpression)
-                {
-                    var changed = false;
-                    var newPredicate = newSelectExpression.Predicate;
-                    var newHaving = newSelectExpression.Having;
-                    if (newSelectExpression.Predicate is SqlConstantExpression predicateConstantExpression
-                        && predicateConstantExpression.Value is bool predicateBoolValue
-                        && !predicateBoolValue)
-                    {
-                        changed = true;
-                        newPredicate = SqlExpressionFactory.Equal(
-                            predicateConstantExpression,
-                            SqlExpressionFactory.Constant(true, predicateConstantExpression.TypeMapping));
-                    }
-
-                    if (newSelectExpression.Having is SqlConstantExpression havingConstantExpression
-                        && havingConstantExpression.Value is bool havingBoolValue
-                        && !havingBoolValue)
-                    {
-                        changed = true;
-                        newHaving = SqlExpressionFactory.Equal(
-                            havingConstantExpression,
-                            SqlExpressionFactory.Constant(true, havingConstantExpression.TypeMapping));
-                    }
-
-                    return changed
-                        ? newSelectExpression.Update(
-                            newSelectExpression.Projection.ToList(),
-                            newSelectExpression.Tables.ToList(),
-                            newPredicate,
-                            newSelectExpression.GroupBy.ToList(),
-                            newHaving,
-                            newSelectExpression.Orderings.ToList(),
-                            newSelectExpression.Limit,
-                            newSelectExpression.Offset,
-                            newSelectExpression.IsDistinct,
-                            newSelectExpression.Alias)
-                        : newSelectExpression;
-                }
-
-                return newExpression;
-            }
-
             protected override Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
             {
                 var result = base.VisitSqlUnaryExpression(sqlUnaryExpression);

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

@@ -0,0 +1,93 @@
+// 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 JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
+using Microsoft.EntityFrameworkCore.Utilities;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Microsoft.EntityFrameworkCore.Query.Internal
+{
+    /// <summary>
+    ///     <para>
+    ///         Service dependencies parameter class for <see cref="RelationalParameterBasedQueryTranslationPostprocessorFactory" />
+    ///     </para>
+    ///     <para>
+    ///         This type is typically used by database providers (and other extensions). It is generally
+    ///         not used in application code.
+    ///     </para>
+    ///     <para>
+    ///         Do not construct instances of this class directly from either provider or application code as the
+    ///         constructor signature may change as new dependencies are added. Instead, use this type in
+    ///         your constructor so that an instance will be created and injected automatically by the
+    ///         dependency injection container. To create an instance with some dependent services replaced,
+    ///         first resolve the object from the dependency injection container, then replace selected
+    ///         services using the 'With...' methods. Do not call the constructor at any point in this process.
+    ///     </para>
+    ///     <para>
+    ///         The service lifetime is <see cref="ServiceLifetime.Singleton" />. This means a single instance
+    ///         is used by many <see cref="DbContext" /> instances. The implementation must be thread-safe.
+    ///         This service cannot depend on services registered as <see cref="ServiceLifetime.Scoped" />.
+    ///     </para>
+    /// </summary>
+    public sealed class RelationalParameterBasedQueryTranslationPostprocessorDependencies
+    {
+        /// <summary>
+        ///     <para>
+        ///         Creates the service dependencies parameter object for a <see cref="RelationalParameterBasedQueryTranslationPostprocessorFactory" />.
+        ///     </para>
+        ///     <para>
+        ///         Do not call this constructor directly from either provider or application code as it may change
+        ///         as new dependencies are added. Instead, use this type in your constructor so that an instance
+        ///         will be created and injected automatically by the dependency injection container. To create
+        ///         an instance with some dependent services replaced, first resolve the object from the dependency
+        ///         injection container, then replace selected services using the 'With...' methods. Do not call
+        ///         the constructor at any point in this process.
+        ///     </para>
+        ///     <para>
+        ///         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.
+        ///     </para>
+        /// </summary>
+        [EntityFrameworkInternal]
+        public RelationalParameterBasedQueryTranslationPostprocessorDependencies(
+            [NotNull] ISqlExpressionFactory sqlExpressionFactory,
+            [NotNull] IParameterNameGeneratorFactory parameterNameGeneratorFactory)
+        {
+            Check.NotNull(sqlExpressionFactory, nameof(sqlExpressionFactory));
+            Check.NotNull(parameterNameGeneratorFactory, nameof(parameterNameGeneratorFactory));
+
+            SqlExpressionFactory = sqlExpressionFactory;
+            ParameterNameGeneratorFactory = parameterNameGeneratorFactory;
+        }
+
+        /// <summary>
+        ///     Sql expression factory.
+        /// </summary>
+        public ISqlExpressionFactory SqlExpressionFactory { get; }
+
+        /// <summary>
+        ///     Parameter name generator factory.
+        /// </summary>
+        public IParameterNameGeneratorFactory ParameterNameGeneratorFactory { get; }
+
+        /// <summary>
+        ///     Clones this dependency parameter object with one service replaced.
+        /// </summary>
+        /// <param name="sqlExpressionFactory"> A replacement for the current dependency of this type. </param>
+        /// <returns> A new parameter object with the given service replaced. </returns>
+        public RelationalParameterBasedQueryTranslationPostprocessorDependencies With(ISqlExpressionFactory sqlExpressionFactory)
+            => new RelationalParameterBasedQueryTranslationPostprocessorDependencies(sqlExpressionFactory, ParameterNameGeneratorFactory);
+
+        /// <summary>
+        ///     Clones this dependency parameter object with one service replaced.
+        /// </summary>
+        /// <param name="parameterNameGeneratorFactory"> A replacement for the current dependency of this type. </param>
+        /// <returns> A new parameter object with the given service replaced. </returns>
+        public RelationalParameterBasedQueryTranslationPostprocessorDependencies With(IParameterNameGeneratorFactory parameterNameGeneratorFactory)
+            => new RelationalParameterBasedQueryTranslationPostprocessorDependencies(SqlExpressionFactory, parameterNameGeneratorFactory);
+    }
+}

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

@@ -0,0 +1,18 @@
+// 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.
+
+namespace Microsoft.EntityFrameworkCore.Query.Internal
+{
+    public class RelationalParameterBasedQueryTranslationPostprocessorFactory : IRelationalParameterBasedQueryTranslationPostprocessorFactory
+    {
+        private readonly RelationalParameterBasedQueryTranslationPostprocessorDependencies _dependencies;
+
+        public RelationalParameterBasedQueryTranslationPostprocessorFactory(RelationalParameterBasedQueryTranslationPostprocessorDependencies dependencies)
+        {
+            _dependencies = dependencies;
+        }
+
+        public virtual RelationalParameterBasedQueryTranslationPostprocessor Create(bool useRelationalNulls)
+            => new RelationalParameterBasedQueryTranslationPostprocessor(_dependencies, useRelationalNulls);
+    }
+}

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

@@ -413,13 +413,9 @@ private SqlExpression SimplifyBoolConstantComparisonExpression(
         {
             if (leftBoolConstant != null && rightBoolConstant != null)
             {
-                // potential optimization:
-                // we can't do it on SqlServer because it reverts search conditions back to values
-                // and we run this visitor after search condition visitor
-                //return operatorType == ExpressionType.Equal
-                //    ? SqlExpressionFactory.Constant((bool)leftBoolConstant.Value == (bool)rightBoolConstant.Value, typeMapping)
-                //    : SqlExpressionFactory.Constant((bool)leftBoolConstant.Value != (bool)rightBoolConstant.Value, typeMapping);
-                return SqlExpressionFactory.MakeBinary(operatorType, left, right, typeMapping);
+                return operatorType == ExpressionType.Equal
+                    ? SqlExpressionFactory.Constant((bool)leftBoolConstant.Value == (bool)rightBoolConstant.Value, typeMapping)
+                    : SqlExpressionFactory.Constant((bool)leftBoolConstant.Value != (bool)rightBoolConstant.Value, typeMapping);
             }
 
             if (rightBoolConstant != null

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs

@@ -75,8 +75,8 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s
             var relationalCommandCache = new RelationalCommandCache(
                 Dependencies.MemoryCache,
                 RelationalDependencies.SqlExpressionFactory,
-                RelationalDependencies.ParameterNameGeneratorFactory,
                 RelationalDependencies.QuerySqlGeneratorFactory,
+                RelationalDependencies.RelationalParameterBasedQueryTranslationPostprocessorFactory,
                 _useRelationalNulls,
                 selectExpression);