Avoid using ^ and ~ when invalid because of value converters (#35124) (#35241)

roji · ranma42 · web-flow · commit fba8789b9fa6 · 2024-12-02T10:45:38.000+02:00
The transformation of equality/in-equality in a (negated) XOR is only possible when the expressions are BIT or integer types on the SQL side (i.e. taking value conversion into account). Similarly, the Boolean negation `NOT` can be implemented as `~` only if the underlying expression is a BIT. Fixes #35093. (cherry picked from commit e6abfdd) Co-authored-by: Andrea Canciani <ranma42@gmail.com>
diff --git a/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs
@@ -16,6 +16,9 @@ public class SearchConditionConvertingExpressionVisitor : SqlExpressionVisitor
     private bool _isSearchCondition;
     private readonly ISqlExpressionFactory _sqlExpressionFactory;
 
+    private static readonly bool UseOldBehavior35093 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35093", out var enabled35093) && enabled35093;
+
     /// <summary>
     ///     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
@@ -344,9 +347,12 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
 
         _isSearchCondition = parentIsSearchCondition;
 
+        var leftType = UseOldBehavior35093 ? newLeft.Type : newLeft.TypeMapping?.Converter?.ProviderClrType ?? newLeft.Type;
+        var rightType = UseOldBehavior35093 ? newRight.Type : newRight.TypeMapping?.Converter?.ProviderClrType ?? newRight.Type;
+
         if (!parentIsSearchCondition
-            && (newLeft.Type == typeof(bool) || newLeft.Type.IsEnum || newLeft.Type.IsInteger())
-            && (newRight.Type == typeof(bool) || newRight.Type.IsEnum || newRight.Type.IsInteger())
+            && (leftType == typeof(bool) || leftType.IsEnum || leftType.IsInteger())
+            && (rightType == typeof(bool) || rightType.IsEnum || rightType.IsInteger())
             && sqlBinaryExpression.OperatorType is ExpressionType.NotEqual or ExpressionType.Equal)
         {
             // "lhs != rhs" is the same as "CAST(lhs ^ rhs AS BIT)", except that
@@ -410,7 +416,10 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
         switch (sqlUnaryExpression.OperatorType)
         {
             case ExpressionType.Not
-                when sqlUnaryExpression.Type == typeof(bool):
+                when (UseOldBehavior35093
+                    ? sqlUnaryExpression.Type
+                    : (sqlUnaryExpression.TypeMapping?.Converter?.ProviderClrType ?? sqlUnaryExpression.Type))
+                == typeof(bool):
             {
                 // when possible, avoid converting to/from predicate form
                 if (!_isSearchCondition && sqlUnaryExpression.Operand is not (ExistsExpression or InExpression or LikeExpression))
diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
@@ -8047,6 +8047,13 @@ public virtual Task Comparison_with_value_converted_subclass(bool async)
             async,
             ss => ss.Set<Faction>().Where(f => f.ServerAddress == IPAddress.Loopback));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Project_equality_with_value_converted_property(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<Mission>().Select(m => m.Difficulty == MissionDifficulty.Unknown));
+
     private static readonly IEnumerable<AmmunitionType?> _weaponTypes = new AmmunitionType?[] { AmmunitionType.Cartridge };
 
     [ConditionalTheory]
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
@@ -9380,6 +9380,20 @@ FROM [Factions] AS [f]
 """);
     }
 
+    public override async Task Project_equality_with_value_converted_property(bool async)
+    {
+        await base.Project_equality_with_value_converted_property(async);
+
+        AssertSql(
+            """
+SELECT CASE
+    WHEN [m].[Difficulty] = N'Unknown' THEN CAST(1 AS bit)
+    ELSE CAST(0 AS bit)
+END
+FROM [Missions] AS [m]
+""");
+    }
+
     public override async Task Contains_on_readonly_enumerable(bool async)
     {
         await base.Contains_on_readonly_enumerable(async);
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs
@@ -12341,6 +12341,20 @@ FROM [LocustHordes] AS [l]
 """);
     }
 
+    public override async Task Project_equality_with_value_converted_property(bool async)
+    {
+        await base.Project_equality_with_value_converted_property(async);
+
+        AssertSql(
+            """
+SELECT CASE
+    WHEN [m].[Difficulty] = N'Unknown' THEN CAST(1 AS bit)
+    ELSE CAST(0 AS bit)
+END
+FROM [Missions] AS [m]
+""");
+    }
+
     public override async Task FirstOrDefault_on_empty_collection_of_DateTime_in_subquery(bool async)
     {
         await base.FirstOrDefault_on_empty_collection_of_DateTime_in_subquery(async);
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs
@@ -10507,6 +10507,20 @@ FROM [Factions] AS [f]
 """);
     }
 
+    public override async Task Project_equality_with_value_converted_property(bool async)
+    {
+        await base.Project_equality_with_value_converted_property(async);
+
+        AssertSql(
+            """
+SELECT CASE
+    WHEN [m].[Difficulty] = N'Unknown' THEN CAST(1 AS bit)
+    ELSE CAST(0 AS bit)
+END
+FROM [Missions] AS [m]
+""");
+    }
+
     public override async Task FirstOrDefault_on_empty_collection_of_DateTime_in_subquery(bool async)
     {
         await base.FirstOrDefault_on_empty_collection_of_DateTime_in_subquery(async);
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs
@@ -6999,6 +6999,20 @@ public override async Task Comparison_with_value_converted_subclass(bool async)
 """);
     }
 
+    public override async Task Project_equality_with_value_converted_property(bool async)
+    {
+        await base.Project_equality_with_value_converted_property(async);
+
+        AssertSql(
+            """
+SELECT CASE
+    WHEN [m].[Difficulty] = N'Unknown' THEN CAST(1 AS bit)
+    ELSE CAST(0 AS bit)
+END
+FROM [Missions] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [m]
+""");
+    }
+
     public override async Task Navigation_access_on_derived_materialized_entity_using_cast(bool async)
     {
         await base.Navigation_access_on_derived_materialized_entity_using_cast(async);
diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs
@@ -3076,6 +3076,17 @@ public override async Task Comparison_with_value_converted_subclass(bool async)
 """);
     }
 
+    public override async Task Project_equality_with_value_converted_property(bool async)
+    {
+        await base.Project_equality_with_value_converted_property(async);
+
+        AssertSql(
+            """
+SELECT "m"."Difficulty" = 'Unknown'
+FROM "Missions" AS "m"
+""");
+    }
+
     public override async Task GetValueOrDefault_in_filter_non_nullable_column(bool async)
     {
         await base.GetValueOrDefault_in_filter_non_nullable_column(async);
