Skip to content

Commit

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

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 dotnet#35093.
  • Loading branch information
ranma42 authored Nov 29, 2024
1 parent d1e1dfa commit e6abfdd
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,11 @@ protected virtual Expression VisitSqlBinary(SqlBinaryExpression binary, bool inS

if (binary.OperatorType is ExpressionType.NotEqual or ExpressionType.Equal)
{
var leftType = newLeft.TypeMapping?.Converter?.ProviderClrType ?? newLeft.Type;
var rightType = newRight.TypeMapping?.Converter?.ProviderClrType ?? newRight.Type;
if (!inSearchConditionContext
&& (newLeft.Type == typeof(bool) || newLeft.Type.IsEnum || newLeft.Type.IsInteger())
&& (newRight.Type == typeof(bool) || newRight.Type.IsEnum || newRight.Type.IsInteger()))
&& (leftType == typeof(bool) || leftType.IsInteger())
&& (rightType == typeof(bool) || rightType.IsInteger()))
{
// "lhs != rhs" is the same as "CAST(lhs ^ rhs AS BIT)", except that
// the first is a boolean, the second is a BIT
Expand Down Expand Up @@ -262,7 +264,8 @@ protected virtual Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpression

switch (sqlUnaryExpression.OperatorType)
{
case ExpressionType.Not when sqlUnaryExpression.Type == typeof(bool):
case ExpressionType.Not
when (sqlUnaryExpression.TypeMapping?.Converter?.ProviderClrType ?? sqlUnaryExpression.Type) == typeof(bool):
{
// when possible, avoid converting to/from predicate form
if (!inSearchConditionContext && 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 e6abfdd

Please sign in to comment.