Fix to dotnet#7061 - the binary operator Equal is not defined for the…

… types 'System.Nullable`1[System.Boolean]' and 'System.Boolean' Problem was that when we were fixing search conditions on a query, we did not take into account differences in nullability that could be created by those transformations. e.g.: myEntity.MyNullableBool (result type is bool?) would get converted to: myEntity.MyNullableBool == (bool?)true (result type bool) If that in turn was a part of a bigger expression, e.g. AndAlso with another bool? expression, that did not have to be converted, then we would produce a type discrepancy which was resulting in an error. Fix is to adjust types of the Left and Right expressions that we process before reconstructing a binary expression node.
davidMuir · Nov 22, 2016 · a7e0117 · a7e0117
1 parent dd44082
commit a7e0117
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 9 deletions.
diff --git a/...ore.Relational/Query/ExpressionVisitors/Internal/PredicateReductionExpressionOptimizer.cs b/...ore.Relational/Query/ExpressionVisitors/Internal/PredicateReductionExpressionOptimizer.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Linq.Expressions;
 using Microsoft.EntityFrameworkCore.Internal;
-using Microsoft.EntityFrameworkCore.Query.Expressions;
 using Remotion.Linq.Parsing;
 
 namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal

diff --git a/src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs b/src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs
@@ -1691,6 +1691,9 @@ public static bool IsSearchCondition(Expression expression)
 
             protected override Expression VisitBinary(BinaryExpression expression)
             {
+                Expression newLeft;
+                Expression newRight;
+
                 if (_isSearchCondition)
                 {
                     if (expression.IsComparisonOperation()
@@ -1701,19 +1704,16 @@ protected override Expression VisitBinary(BinaryExpression expression)
 
                         _isSearchCondition = false;
 
-                        var left = Visit(expression.Left);
-                        var right = Visit(expression.Right);
+                        newLeft = AdjustExpressionType(Visit(expression.Left), expression.Left.Type);
+                        newRight = AdjustExpressionType(Visit(expression.Right), expression.Right.Type);
 
                         _isSearchCondition = parentIsSearchCondition;
 
-                        return Expression.MakeBinary(expression.NodeType, left, right);
+                        return Expression.MakeBinary(expression.NodeType, newLeft, newRight);
                     }
                 }
                 else
                 {
-                    Expression newLeft;
-                    Expression newRight;
-
                     if (expression.IsLogicalOperation()
                         || expression.NodeType == ExpressionType.Or
                         || expression.NodeType == ExpressionType.And)
@@ -1732,6 +1732,9 @@ protected override Expression VisitBinary(BinaryExpression expression)
                         newRight = Visit(expression.Right);
                     }
 
+                    newLeft = AdjustExpressionType(newLeft, expression.Left.Type);
+                    newRight = AdjustExpressionType(newRight, expression.Right.Type);
+
                     var newExpression
                         = expression.Update(newLeft, expression.Conversion, newRight);
 
@@ -1746,9 +1749,17 @@ var newExpression
                     }
                 }
 
-                return base.VisitBinary(expression);
+                newLeft = AdjustExpressionType(Visit(expression.Left), expression.Left.Type);
+                newRight = AdjustExpressionType(Visit(expression.Right), expression.Right.Type);
+
+                return expression.Update(newLeft, expression.Conversion, newRight);
             }
 
+            private static Expression AdjustExpressionType(Expression expression, Type expectedType)
+                => expression.Type != expectedType
+                    ? Expression.Convert(expression, expectedType)
+                    : expression;
+
             protected override Expression VisitConditional(ConditionalExpression expression)
             {
                 var parentIsSearchCondition = _isSearchCondition;
@@ -1806,7 +1817,7 @@ protected override Expression VisitUnary(UnaryExpression expression)
                     {
                         return Expression.Equal(
                             expression.Operand,
-                            Expression.Constant(false, typeof(bool)));
+                            Expression.Constant(false, expression.Operand.Type));
                     }
 
                     if (expression.NodeType == ExpressionType.Convert

diff --git a/src/Microsoft.EntityFrameworkCore.Specification.Tests/GearsOfWarQueryTestBase.cs b/src/Microsoft.EntityFrameworkCore.Specification.Tests/GearsOfWarQueryTestBase.cs
@@ -2038,6 +2038,21 @@ from w in group1.DefaultIfEmpty()
             }
         }
 
+        [ConditionalFact]
+        public virtual void Complex_predicate_with_AndAlso_and_nullable_bool_property()
+        {
+            using (var context = CreateContext())
+            {
+                var query = from w in context.Weapons
+                            where w.Id != 50 && !w.Owner.HasSoulPatch
+                            select w;
+
+                var result = query.ToList();
+
+                Assert.Equal(5, result.Count);
+            }
+        }
+
         protected GearsOfWarContext CreateContext() => Fixture.CreateContext(TestStore);
 
         protected GearsOfWarQueryTestBase(TFixture fixture)

diff --git a/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/GearsOfWarQuerySqlServerTest.cs b/test/Microsoft.EntityFrameworkCore.SqlServer.FunctionalTests/GearsOfWarQuerySqlServerTest.cs
@@ -2036,6 +2036,19 @@ THEN CAST(0 AS BIT) ELSE CAST(1 AS BIT)
                 Sql);
         }
 
+        public override void Complex_predicate_with_AndAlso_and_nullable_bool_property()
+        {
+            base.Complex_predicate_with_AndAlso_and_nullable_bool_property();
+
+            Assert.Equal(
+                @"SELECT [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId], [w.Owner].[Nickname], [w.Owner].[SquadId], [w.Owner].[AssignedCityName], [w.Owner].[CityOrBirthName], [w.Owner].[Discriminator], [w.Owner].[FullName], [w.Owner].[HasSoulPatch], [w.Owner].[LeaderNickname], [w.Owner].[LeaderSquadId], [w.Owner].[Rank]
+FROM [Weapon] AS [w]
+LEFT JOIN [Gear] AS [w.Owner] ON [w].[OwnerFullName] = [w.Owner].[FullName]
+WHERE ([w].[Id] <> 50) AND ([w.Owner].[HasSoulPatch] = 0)
+ORDER BY [w].[OwnerFullName]",
+                Sql);
+        }
+
         protected override void ClearLog() => TestSqlLoggerFactory.Reset();
 
         private const string FileLineEnding = @"