Skip to content

Commit

Permalink
Fix to #23617 - IQueryable.All evaluate to false with predicate _ => …
Browse files Browse the repository at this point in the history
…true

We have optimization for EXISTS, that returns false when the subquery has a predicate which filters out all the rows. In such case we return constant false. However, ExistExpression also stores information about it being negated or not (i.e. EXISTS vs NOT EXISTS).
If the Exists expression is negated and the predicate filters out all the rows we should return true instead.

Fixes #23617
  • Loading branch information
maumar committed Jan 5, 2021
1 parent 0e91fa1 commit ff01e5c
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -551,10 +551,18 @@ protected virtual SqlExpression VisitExists(
var subquery = Visit(existsExpression.Subquery);
nullable = false;

// if subquery has predicate which evaluates to false, we can simply return false
return TryGetBoolConstantValue(subquery.Predicate) == false
? subquery.Predicate
: existsExpression.Update(subquery);
//if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore23617", out var enabled) && enabled)
{
return TryGetBoolConstantValue(subquery.Predicate) == false
? subquery.Predicate
: existsExpression.Update(subquery);
}

//// if subquery has predicate which evaluates to false, we can simply return false
//// if the exisits is negated we need to return true instead
//return TryGetBoolConstantValue(subquery.Predicate) == false
// ? _sqlExpressionFactory.Constant(existsExpression.IsNegated, existsExpression.TypeMapping)
// : (SqlExpression)existsExpression.Update(subquery);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,18 @@ public override Task Multiple_collection_navigation_with_FirstOrDefault_chained(
return base.Multiple_collection_navigation_with_FirstOrDefault_chained(async);
}

[ConditionalTheory(Skip = "Issue#20441")]
public override Task All_true(bool async)
{
return base.All_true(async);
}

[ConditionalTheory(Skip = "Issue#20441")]
public override Task Not_Any_false(bool async)
{
return base.Not_Any_false(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 @@ -1986,5 +1986,24 @@ public virtual Task Min_after_default_if_empty_does_not_throw(bool isAsync)
isAsync,
ss => ss.Set<Order>().Where(o => o.OrderID == 10243).Select(o => o.OrderID).DefaultIfEmpty());
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task All_true(bool async)
{
return AssertAll(
async,
ss => ss.Set<Customer>(),
predicate: x => true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Not_Any_false(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => !c.Orders.Any(o => false)).Select(c => c.CustomerID));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,23 @@ public override async Task Count_on_projection_with_client_eval(bool async)
FROM [Orders] AS [o]");
}

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

AssertSql(
@"SELECT CAST(1 AS bit)");
}

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

AssertSql(
@"SELECT [c].[CustomerID]
FROM [Customers] AS [c]");
}

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

Expand Down

0 comments on commit ff01e5c

Please sign in to comment.