Moves checked->unchecked expression visitor to relational level, fixe…

…s tests
dotnet · Jan 28, 2020 · 5812751 · 5812751
1 parent ccb3833
commit 5812751
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 21 deletions.
diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
@@ -1022,6 +1022,8 @@ private Expression RemapLambdaBody(ShapedQueryExpression shapedQueryExpression,
             var lambdaBody = ReplacingExpressionVisitor.Replace(
                 lambdaExpression.Parameters.Single(), shapedQueryExpression.ShaperExpression, lambdaExpression.Body);
 
+            lambdaBody = IgnoreCheckedExpressionVisitor.Ignore(lambdaBody);
+
             return ExpandWeakEntities((SelectExpression)shapedQueryExpression.QueryExpression, lambdaBody);
         }
 

diff --git a/src/EFCore/Query/IgnoreCheckedExpressionVisitor.cs b/src/EFCore/Query/IgnoreCheckedExpressionVisitor.cs
@@ -2,38 +2,43 @@
 // 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.Utilities;
 
 namespace Microsoft.EntityFrameworkCore.Query
 {
     public class IgnoreCheckedExpressionVisitor: ExpressionVisitor
     {
+        public static Expression Ignore([NotNull] Expression expression)
+        {
+            Check.NotNull(expression, nameof(expression));
+
+            return new IgnoreCheckedExpressionVisitor().Visit(expression);
+        }
+
         protected override Expression VisitBinary(BinaryExpression node)
         {
             var visitedLeft = Visit(node.Left);
             var visitedRight = Visit(node.Right);
 
-            var visitBinary = node.NodeType switch
-                              {
-                                  ExpressionType.AddChecked      => Expression.Add(visitedLeft, visitedRight),
-                                  ExpressionType.SubtractChecked => Expression.Subtract(visitedLeft, visitedRight),
-                                  ExpressionType.MultiplyChecked => Expression.Multiply(visitedLeft, visitedRight),
-                                  _                              => base.VisitBinary(node)
-                              };
-
-            return visitBinary;
+            return node.NodeType switch
+                   {
+                       ExpressionType.AddChecked => Expression.Add(visitedLeft, visitedRight),
+                       ExpressionType.SubtractChecked => Expression.Subtract(visitedLeft, visitedRight),
+                       ExpressionType.MultiplyChecked => Expression.Multiply(visitedLeft, visitedRight),
+                       _ => node.Update(visitedLeft, node.Conversion, visitedRight)
+                   };
         }
 
         protected override Expression VisitUnary(UnaryExpression node)
         {
             var operand = Visit(node.Operand);
 
-            var visitUnary = node.NodeType switch
-                             {
-                                 ExpressionType.ConvertChecked => Expression.Convert(operand, node.Type),
-                                 _                             => base.VisitUnary(node)
-                             };
-
-            return visitUnary;
+            return node.NodeType switch
+                   {
+                       ExpressionType.ConvertChecked => Expression.Convert(operand, node.Type),
+                       _ => node.Update(operand)
+                   };
         }
     }
 }
diff --git a/src/EFCore/Query/QueryTranslationPreprocessor.cs b/src/EFCore/Query/QueryTranslationPreprocessor.cs
@@ -29,7 +29,6 @@ public virtual Expression Process([NotNull] Expression query)
         {
             Check.NotNull(query, nameof(query));
 
-            query = new IgnoreCheckedExpressionVisitor().Visit(query);
             query = new EnumerableToQueryableMethodConvertingExpressionVisitor().Visit(query);
             query = new QueryMetadataExtractingExpressionVisitor(_queryCompilationContext).Visit(query);
             query = new InvocationExpressionRemovingExpressionVisitor().Visit(query);

diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
@@ -7522,8 +7522,6 @@ public virtual Task Indexer_property_is_pushdown_into_subquery(bool isAsync)
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Checked_context_with_cast_does_not_fail(bool isAsync)
         {
-            using var ctx = CreateContext();
-
             checked
             {
                 return AssertQuery(
@@ -7536,8 +7534,6 @@ public virtual Task Checked_context_with_cast_does_not_fail(bool isAsync)
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Checked_context_with_addition_does_not_fail(bool isAsync)
         {
-            using var ctx = CreateContext();
-
             checked
             {
                 return AssertQuery(
@@ -7546,6 +7542,22 @@ public virtual Task Checked_context_with_addition_does_not_fail(bool isAsync)
             }
         }
 
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Checked_context_throws_on_client_evaluation(bool isAsync)
+        {
+            checked
+            {
+                return Assert.ThrowsAsync<InvalidOperationException>(
+                    () => AssertQueryScalar(
+                        isAsync,
+                        ss => ss.Set<LocustLeader>().Select(w => w.ThreatLevel >= (byte)GetThreatLevel()))
+                    );
+            }
+        }
+
+        private int GetThreatLevel() => 256;
+
         protected GearsOfWarContext CreateContext() => Fixture.CreateContext();
 
         protected virtual void ClearLog()

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
@@ -7515,6 +7515,11 @@ FROM [LocustLeaders] AS [l]
 WHERE [l].[Discriminator] IN (N'LocustLeader', N'LocustCommander') AND (CAST([l].[ThreatLevel] AS bigint) >= (CAST(5 AS bigint) + CAST([l].[ThreatLevel] AS bigint)))");
         }
 
+        public override async Task Checked_context_throws_on_client_evaluation(bool isAsync)
+        {
+            await base.Checked_context_throws_on_client_evaluation(isAsync);
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
     }