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

[3.1] Fix to #19825 - Query: incorrectly generating JOIN rather than APPLY for subqueries with outside references to a joined table #20064

Merged
merged 1 commit into from
Apr 4, 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
34 changes: 25 additions & 9 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ public Expression ApplyCollectionJoin(
}

var joinPredicate = TryExtractJoinKey(innerSelectExpression);
var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(Tables)
var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this)
.ContainsOuterReference(innerSelectExpression);
if (containsOuterReference && joinPredicate != null)
{
Expand Down Expand Up @@ -1152,12 +1152,15 @@ private bool ContainsTableReference(TableExpressionBase table)

private class SelectExpressionCorrelationFindingExpressionVisitor : ExpressionVisitor
{
private readonly IReadOnlyList<TableExpressionBase> _tables;
private readonly SelectExpression _outerSelectExpression;
private bool _containsOuterReference;

public SelectExpressionCorrelationFindingExpressionVisitor(IReadOnlyList<TableExpressionBase> tables)
private readonly bool _quirkMode19825;

public SelectExpressionCorrelationFindingExpressionVisitor(SelectExpression outerSelectExpression)
{
_tables = tables;
_outerSelectExpression = outerSelectExpression;
_quirkMode19825 = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue19825", out var enabled) && enabled;
}

public bool ContainsOuterReference(SelectExpression selectExpression)
Expand All @@ -1176,12 +1179,25 @@ public override Expression Visit(Expression expression)
return expression;
}

if (expression is ColumnExpression columnExpression
&& _tables.Contains(columnExpression.Table))
if (_quirkMode19825)
{
_containsOuterReference = true;
if (expression is ColumnExpression columnExpression
&& _outerSelectExpression.Tables.Contains(columnExpression.Table))
{
_containsOuterReference = true;

return expression;
return expression;
}
}
else
{
if (expression is ColumnExpression columnExpression
&& _outerSelectExpression.ContainsTableReference(columnExpression.Table))
{
_containsOuterReference = true;

return expression;
}
}

return base.Visit(expression);
Expand Down Expand Up @@ -1230,7 +1246,7 @@ private void AddJoin(
joinPredicate = TryExtractJoinKey(innerSelectExpression);
if (joinPredicate != null)
{
var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(Tables)
var containsOuterReference = new SelectExpressionCorrelationFindingExpressionVisitor(this)
.ContainsOuterReference(innerSelectExpression);
if (containsOuterReference)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,11 @@ public override Task OrderBy_collection_count_ThenBy_reference_navigation(bool a
{
return base.OrderBy_collection_count_ThenBy_reference_navigation(async);
}

[ConditionalTheory(Skip = "issue #19967")]
public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
{
return base.SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(async);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5958,5 +5958,19 @@ public virtual Task OrderBy_collection_count_ThenBy_reference_navigation(bool as
() => l1.OneToOne_Required_FK1.OneToOne_Required_FK2.Name)),
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
{
return AssertQuery(
async,
ss => from l1 in ss.Set<Level1>()
join l2 in ss.Set<Level2>() on l1.Id equals l2.Level1_Required_Id
join l3 in ss.Set<Level3>() on l2.Id equals l3.Level2_Required_Id
join l4 in ss.Set<Level4>() on l3.Id equals l4.Level3_Required_Id
from other in ss.Set<Level1>().Where(x => x.Id <= l2.Id && x.Name == l4.Name).DefaultIfEmpty()
select l1);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4460,6 +4460,23 @@ FROM [LevelThree] AS [l2]
WHERE [l0].[Id] IS NOT NULL AND ([l0].[Id] = [l2].[OneToMany_Required_Inverse3Id])), [l1].[Name]");
}

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

AssertSql(
@"SELECT [l].[Id], [l].[Date], [l].[Name], [l].[OneToMany_Optional_Self_Inverse1Id], [l].[OneToMany_Required_Self_Inverse1Id], [l].[OneToOne_Optional_Self1Id]
FROM [LevelOne] AS [l]
INNER JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Required_Id]
INNER JOIN [LevelThree] AS [l1] ON [l0].[Id] = [l1].[Level2_Required_Id]
INNER JOIN [LevelFour] AS [l2] ON [l1].[Id] = [l2].[Level3_Required_Id]
OUTER APPLY (
SELECT [l3].[Id], [l3].[Date], [l3].[Name], [l3].[OneToMany_Optional_Self_Inverse1Id], [l3].[OneToMany_Required_Self_Inverse1Id], [l3].[OneToOne_Optional_Self1Id]
FROM [LevelOne] AS [l3]
WHERE ([l3].[Id] <= [l0].[Id]) AND (([l2].[Name] = [l3].[Name]) OR ([l2].[Name] IS NULL AND [l3].[Name] IS NULL))
) AS [t]");
}

private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@ public override Task Include_inside_subquery(bool isAsync)
{
return base.Include_inside_subquery(isAsync);
}

// Sqlite does not support cross/outer apply
public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async) => null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,8 @@ public override Task Project_collection_navigation_nested_with_take(bool isAsync
{
return base.Project_collection_navigation_nested_with_take(isAsync);
}

// Sqlite does not support cross/outer apply
public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async) => null;
}
}