Skip to content

Commit

Permalink
Query: Add optimization to conditional being equated to equality
Browse files Browse the repository at this point in the history
Resolves #12148
  • Loading branch information
smitpatel committed Aug 26, 2020
1 parent 029ad85 commit daa28f1
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 17 deletions.
49 changes: 49 additions & 0 deletions src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ public class NullCheckRemovingExpressionVisitor : ExpressionVisitor
private readonly NullSafeAccessVerifyingExpressionVisitor _nullSafeAccessVerifyingExpressionVisitor
= new NullSafeAccessVerifyingExpressionVisitor();

/// <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
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected override Expression VisitBinary(BinaryExpression binaryExpression)
{
var visitedExpression = base.VisitBinary(binaryExpression);

return TryOptimizeConditionalEquality(visitedExpression) ?? visitedExpression;
}

/// <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 @@ -60,6 +73,42 @@ protected override Expression VisitConditional(ConditionalExpression conditional
return base.VisitConditional(conditionalExpression);
}

private Expression TryOptimizeConditionalEquality(Expression expression)
{
// Simplify (a ? b : null) == null => a
// Simplify (a ? null : c) == null => !a
if (expression is BinaryExpression binaryExpression
&& binaryExpression.NodeType == ExpressionType.Equal
&& (binaryExpression.Left is ConditionalExpression
|| binaryExpression.Right is ConditionalExpression))
{
Expression comparedExpression;
if (binaryExpression.Left is ConditionalExpression conditionalExpression)
{
comparedExpression = binaryExpression.Right;
}
else
{
conditionalExpression = binaryExpression.Right as ConditionalExpression;
comparedExpression = binaryExpression.Left;
}

if (conditionalExpression.IfTrue.IsNullConstantExpression()
&& comparedExpression.IsNullConstantExpression())
{
return conditionalExpression.Test;
}

if (conditionalExpression.IfFalse.IsNullConstantExpression()
&& comparedExpression.IsNullConstantExpression())
{
return Expression.Not(conditionalExpression.Test);
}
}

return null;
}

private sealed class NullSafeAccessVerifyingExpressionVisitor : ExpressionVisitor
{
private readonly ISet<Expression> _nullSafeAccesses = new HashSet<Expression>(ExpressionEqualityComparer.Instance);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4178,6 +4178,12 @@ public override Task DefaultIfEmpty_Sum_over_collection_navigation(bool async)
return base.DefaultIfEmpty_Sum_over_collection_navigation(async);
}

[ConditionalTheory(Skip = "Non embedded collection subquery Issue#17246")]
public override Task Entity_equality_on_subquery_with_null_check(bool async)
{
return base.Entity_equality_on_subquery_with_null_check(async);
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ public override Task Client_side_equality_with_parameter_works_with_optional_nav
public override Task Where_coalesce_with_anonymous_types(bool async)
=> base.Where_coalesce_with_anonymous_types(async);

[ConditionalTheory(Skip = "issue #17386")]
public override Task Where_conditional_with_anonymous_type(bool async)
=> base.Where_conditional_with_anonymous_type(async);

[ConditionalTheory(Skip = "issue #17386")]
public override Task GetValueOrDefault_on_DateTimeOffset(bool async)
=> base.GetValueOrDefault_on_DateTimeOffset(async);
Expand Down
21 changes: 10 additions & 11 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,17 +1192,16 @@ orderby g.Nickname
[MemberData(nameof(IsAsyncData))]
public virtual Task Where_conditional_with_anonymous_type(bool async)
{
return AssertTranslationFailed(
() => AssertQuery(
async,
ss => from g in ss.Set<Gear>()
orderby g.Nickname
where (g.LeaderNickname != null
? new { g.HasSoulPatch }
: null)
== null
select g.Nickname,
assertOrder: true));
return AssertQuery(
async,
ss => from g in ss.Set<Gear>()
orderby g.Nickname
where (g.LeaderNickname != null
? new { g.HasSoulPatch }
: null)
== null
select g.Nickname,
assertOrder: true);
}

[ConditionalTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,8 @@ public virtual Task Ternary_should_not_evaluate_both_sides_with_parameter(bool a
o => new
{
// ReSharper disable SimplifyConditionalTernaryExpression
Data1 = param != null ? o.OrderDate == param.Value : true, Data2 = param == null ? true : o.OrderDate == param.Value
Data1 = param != null ? o.OrderDate == param.Value : true,
Data2 = param == null ? true : o.OrderDate == param.Value
// ReSharper restore SimplifyConditionalTernaryExpression
}));
}
Expand Down Expand Up @@ -3441,7 +3442,7 @@ public virtual void Select_Where_Subquery_Equality()
using var context = CreateContext();
var orders
= (from o in context.Orders.OrderBy(o => o.OrderID).Take(1)
// ReSharper disable once UseMethodAny.0
// ReSharper disable once UseMethodAny.0
where (from od in context.OrderDetails.OrderBy(od => od.OrderID).Take(2)
where (from c in context.Set<Customer>()
where c.CustomerID == o.CustomerID
Expand Down Expand Up @@ -6241,5 +6242,23 @@ public virtual Task DefaultIfEmpty_Sum_over_collection_navigation(bool async)
.Select(c => new { c.CustomerID, Sum = c.Orders.Select(o => o.OrderID).DefaultIfEmpty().Sum() }),
elementSorter: c => c.CustomerID);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Entity_equality_on_subquery_with_null_check(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Customer>()
.Select(c => new
{
c.CustomerID,
Order = (c.Orders.Any() ? c.Orders.FirstOrDefault() : null) == null
? null
: new { c.Orders.FirstOrDefault().OrderDate }
}),
elementSorter: c => c.CustomerID,
elementAsserter: (e, a) => AssertEqual(e.Order, a.Order));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,17 @@ FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}

public override async Task Where_conditional_with_anonymous_type(bool async)
{
await base.Where_conditional_with_anonymous_type(async);

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gears] AS [g]
WHERE [g].[LeaderNickname] IS NULL
ORDER BY [g].[Nickname]");
}

public override async Task Select_coalesce_with_anonymous_types(bool async)
{
await base.Select_coalesce_with_anonymous_types(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5202,6 +5202,24 @@ FROM [Orders] AS [o]
FROM [Customers] AS [c]");
}

public override async Task Entity_equality_on_subquery_with_null_check(bool async)
{
await base.Entity_equality_on_subquery_with_null_check(async);

AssertSql(
@"SELECT [c].[CustomerID], CASE
WHEN NOT (EXISTS (
SELECT 1
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID])) THEN CAST(1 AS bit)
ELSE CAST(0 AS bit)
END, (
SELECT TOP(1) [o0].[OrderDate]
FROM [Orders] AS [o0]
WHERE [c].[CustomerID] = [o0].[CustomerID])
FROM [Customers] AS [c]");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,17 @@ FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}

public override async Task Where_conditional_with_anonymous_type(bool async)
{
await base.Where_conditional_with_anonymous_type(async);

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gears] AS [g]
WHERE [g].[LeaderNickname] IS NULL
ORDER BY [g].[Nickname]");
}

public override async Task Select_coalesce_with_anonymous_types(bool async)
{
await base.Select_coalesce_with_anonymous_types(async);
Expand Down

0 comments on commit daa28f1

Please sign in to comment.