Skip to content

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

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 21, 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
12 changes: 6 additions & 6 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,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 @@ -1226,12 +1226,12 @@ private bool ContainsTableReference(TableExpressionBase table)

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

public SelectExpressionCorrelationFindingExpressionVisitor(IReadOnlyList<TableExpressionBase> tables)
public SelectExpressionCorrelationFindingExpressionVisitor(SelectExpression outerSelectExpression)
{
_tables = tables;
_outerSelectExpression = outerSelectExpression;
}

public bool ContainsOuterReference(SelectExpression selectExpression)
Expand All @@ -1251,7 +1251,7 @@ public override Expression Visit(Expression expression)
}

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

Expand Down Expand Up @@ -1304,7 +1304,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 @@ -71,5 +71,17 @@ public override Task Select_subquery_single_nested_subquery2(bool async)
{
return base.Select_subquery_single_nested_subquery2(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);
}

[ConditionalTheory(Skip = "issue #19967")]
public override Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
{
return base.Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(async);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5164,6 +5164,7 @@ public virtual Task Select_subquery_single_nested_subquery(bool async)
});

}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Select_subquery_single_nested_subquery2(bool async)
Expand Down Expand Up @@ -5197,5 +5198,31 @@ public virtual Task Select_subquery_single_nested_subquery2(bool async)
});
});
}

[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);
}

[ConditionalTheory(Skip = "issue #19095")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Level1>().SelectMany(
l1 => l1.OneToMany_Optional1.DefaultIfEmpty().SelectMany(
l2 => l2.OneToOne_Required_PK2.OneToMany_Optional3.DefaultIfEmpty()
.Select(l4 => new { l1Name = l1.Name, l2Name = l2.OneToOne_Required_PK2.Name, l3Name = l4.OneToOne_Optional_PK_Inverse4.Name }))));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4398,6 +4398,31 @@ FROM [LevelThree] AS [l1]
ORDER BY [l].[Id], [t1].[Id], [t1].[Id0]");
}

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]");
}

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

AssertSql(
@"");
}

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,9 @@ public override Task Include_inside_subquery(bool async)
{
return base.Include_inside_subquery(async);
}

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

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