[2.0.1] Query: Fix for Access to context bound variable can be optimi…

…zed out

Issue:
We adopted short-circuiting logic in logical expression due to possible exceptions (which compiler would never throw in memory). This works correctly for general cases because the expression tree would have current value of closure variable hence evaluating to correct thing.
When it comes to QueryFilter, the filter has closure member access on context instance which was used during OnModelCreating which may have stale value of context closure variables. We parametrize such closure variables and insert their values through parameters when running query. But if such closure variable is used in complex expression which can short-circuit then we would be using wrong value for short-circuit (by this point we haven't inserted correct value from current context instance) hence generating incorrect query model. The other cause for it is QueryCache has only 1 entry for all the possible values of context bound variables in filter.

Solution:
Since evaluating filter with context bound variables with different value may not always generate a re-usable query model for all values of variables, whenever we get an expression to evaluate during funcletizing query filter, we replace context, with context parameter (which would be injected later) and return this expression without evaluating anything in-memory. Any exception to be thrown from evaluation of context bound variables will be thrown when actual parameter value will be calculated while running query.

Resolves #9825

src/EFCore.Specification.Tests/Query/QueryTestBase.cs

@@ -3126,6 +3126,25 @@ public virtual void Parameter_extraction_can_throw_exception_from_user_code()
             }
         }
 
+        [ConditionalFact]
+        public virtual void Parameter_extraction_can_throw_exception_from_user_code_2()
+        {
+            using (var context = CreateContext())
+            {
+                DateTime? dateFilter = null;
+
+                Assert.Throws<InvalidOperationException>(
+                    () =>
+                        context.Orders
+                            .Where(
+                                o => (o.OrderID < 10400)
+                                     && ((o.OrderDate.HasValue
+                                          && o.OrderDate.Value.Month == dateFilter.Value.Month
+                                          && o.OrderDate.Value.Year == dateFilter.Value.Year)))
+                            .ToList());
+            }
+        }
+
         [ConditionalFact]
         public virtual void Subquery_member_pushdown_does_not_change_original_subquery_model()
         {

src/EFCore/Query/ExpressionVisitors/Internal/ParameterExtractingExpressionVisitor.cs

@@ -32,6 +32,8 @@ public class ParameterExtractingExpressionVisitor : ExpressionVisitor
 
         private bool _inLambda;
 
+        private static bool QuirkEnabled => AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue9825", out var enabled) && enabled;
+
         /// <summary>
         ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
         ///     directly from your code. This API may change or be removed in future releases.
@@ -352,30 +354,48 @@ private Expression TryExtractParameter(Expression expression)
         {
             var parameterValue = Evaluate(expression, out var parameterName);
 
-            if (parameterValue is Expression valueExpression)
+            if (QuirkEnabled)
             {
-                return ExtractParameters(valueExpression);
-            }
+                if (parameterValue is Expression valueExpression)
+                {
+                    return ExtractParameters(valueExpression);
+                }
 
-            if (!_parameterize)
-            {
-                if (_generateContextAccessors
-                    && expression is MemberExpression memberExpression
-                    && memberExpression.Expression != null
-                    && typeof(DbContext).GetTypeInfo()
-                        .IsAssignableFrom(memberExpression.Expression.Type.GetTypeInfo()))
+                if (!_parameterize)
                 {
-                    var contextParameterExpression
-                        = Expression.Parameter(memberExpression.Expression.Type, "context");
+                    if (_generateContextAccessors
+                        && expression is MemberExpression memberExpression
+                        && memberExpression.Expression != null
+                        && typeof(DbContext).GetTypeInfo()
+                            .IsAssignableFrom(memberExpression.Expression.Type.GetTypeInfo()))
+                    {
+                        var contextParameterExpression
+                            = Expression.Parameter(memberExpression.Expression.Type, "context");
 
-                    parameterValue
-                        = Expression.Lambda(
-                            memberExpression.Update(contextParameterExpression),
-                            contextParameterExpression);
+                        parameterValue
+                            = Expression.Lambda(
+                                memberExpression.Update(contextParameterExpression),
+                                contextParameterExpression);
+                    }
+                    else
+                    {
+                        return Expression.Constant(parameterValue);
+                    }
                 }
-                else
+            }
+            else
+            {
+                if (!_generateContextAccessors)
                 {
-                    return Expression.Constant(parameterValue);
+                    if (parameterValue is Expression valueExpression)
+                    {
+                        return ExtractParameters(valueExpression);
+                    }
+
+                    if (!_parameterize)
+                    {
+                        return Expression.Constant(parameterValue);
+                    }
                 }
             }
 
@@ -403,6 +423,22 @@ var compilerPrefixIndex
             return Expression.Parameter(expression.Type, parameterName);
         }
 
+        private class ContextParameterReplacingExpressionVisitor : ExpressionVisitor
+        {
+            public ParameterExpression ContextParameterExpression;
+
+            protected override Expression VisitConstant(ConstantExpression node)
+            {
+                if (typeof(DbContext).GetTypeInfo()
+                    .IsAssignableFrom(node.Type.GetTypeInfo()))
+                {
+                    return ContextParameterExpression ?? (ContextParameterExpression = Expression.Parameter(node.Type, "context"));
+                }
+
+                return base.VisitConstant(node);
+            }
+        }
+
         /// <summary>
         ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
         ///     directly from your code. This API may change or be removed in future releases.
@@ -416,6 +452,26 @@ public virtual object Evaluate([CanBeNull] Expression expression, [CanBeNull] ou
                 return null;
             }
 
+            if (!QuirkEnabled)
+            {
+                if (_generateContextAccessors)
+                {
+                    var contextParameterReplacingExpressionVisitor = new ContextParameterReplacingExpressionVisitor();
+                    var updatedExpression = contextParameterReplacingExpressionVisitor.Visit(expression);
+
+                    if (contextParameterReplacingExpressionVisitor.ContextParameterExpression != null)
+                    {
+                        parameterName = updatedExpression is MemberExpression memberExpression
+                            ? memberExpression.Member.Name
+                            : "_queryFilter";
+
+                        return Expression.Lambda(
+                            updatedExpression,
+                            contextParameterReplacingExpressionVisitor.ContextParameterExpression);
+                    }
+                }
+            }
+
             // ReSharper disable once SwitchStatementMissingSomeCases
             switch (expression.NodeType)
             {

test/EFCore.SqlServer.FunctionalTests/Query/FiltersSqlServerTest.cs

@@ -217,7 +217,7 @@ WHERE [o].[Quantity] > @___quantity_1
         [ConditionalFact]
         public void FromSql_is_composed()
         {
-            using (var context = Fixture.CreateContext())
+            using (var context = Fixture.CreateContext(enableFilters: true))
             {
                 var results = context.Customers.FromSql("select * from Customers").ToList();
 

test/EFCore.SqlServer.FunctionalTests/Query/NullSemanticsQuerySqlServerTest.cs

@@ -12,7 +12,8 @@ public class NullSemanticsQuerySqlServerTest : NullSemanticsQueryTestBase<SqlSer
         public NullSemanticsQuerySqlServerTest(NullSemanticsQuerySqlServerFixture fixture, ITestOutputHelper testOutputHelper)
             : base(fixture)
         {
-            fixture.TestSqlLoggerFactory.Clear();
+            Fixture.TestSqlLoggerFactory.Clear();
+            //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
         }
 
         public override void Compare_bool_with_bool_equal()