Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: Add optimization to conditional being equated to equality #22251

Merged
1 commit merged into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 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;
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
}

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

private Expression TryOptimizeConditionalEquality(Expression expression)
{
// Simplify (a ? b : null) == null => !a || b == null
// Simplify (a ? null : b) == null => a || b == null
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.IfFalse.IsNullConstantExpression()
&& comparedExpression.IsNullConstantExpression())
{
return Expression.OrElse(
Expression.Not(conditionalExpression.Test),
Expression.Equal(conditionalExpression.IfTrue, comparedExpression));
}

if (conditionalExpression.IfTrue.IsNullConstantExpression()
&& comparedExpression.IsNullConstantExpression())
{
return Expression.OrElse(
conditionalExpression.Test,
Expression.Equal(conditionalExpression.IfFalse, comparedExpression));
}
}

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
55 changes: 43 additions & 12 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1190,19 +1190,50 @@ orderby g.Nickname

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Where_conditional_with_anonymous_type(bool async)
public virtual Task Where_conditional_equality_1(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
? g.HasSoulPatch
: null)
== null
select g.Nickname,
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Where_conditional_equality_2(bool async)
{
return AssertQuery(
async,
ss => from g in ss.Set<Gear>()
orderby g.Nickname
where (g.LeaderNickname == null
? null
: g.HasSoulPatch)
== null
select g.Nickname,
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Where_conditional_equality_3(bool async)
{
return AssertQuery(
async,
ss => from g in ss.Set<Gear>()
orderby g.Nickname
where (g.LeaderNickname != null
? (int?)null
: 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,38 @@ FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}

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

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

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

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

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

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gears] AS [g]
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,27 @@ 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])) OR (
SELECT TOP(1) [o0].[OrderID]
FROM [Orders] AS [o0]
WHERE [c].[CustomerID] = [o0].[CustomerID]) IS NULL THEN CAST(1 AS bit)
ELSE CAST(0 AS bit)
END, (
SELECT TOP(1) [o1].[OrderDate]
FROM [Orders] AS [o1]
WHERE [c].[CustomerID] = [o1].[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,38 @@ FROM [Gears] AS [g]
ORDER BY [g].[Nickname]");
}

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

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

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

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

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

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gears] AS [g]
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