Skip to content

Commit

Permalink
Avoid using ^ and ~ when invalid because of value converters (#35124
Browse files Browse the repository at this point in the history
) (#35241)

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 <[email protected]>
  • Loading branch information
roji and ranma42 authored Dec 2, 2024
1 parent 1c0ef32 commit fba8789
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit fba8789

Please sign in to comment.