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.

Sign up for GitHub

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

Jump to bottom

Query: Fix ordering for filtered include in single query scenario #21337

Merged
merged 1 commit into from
Jun 23, 2020
Merged

Query: Fix ordering for filtered include in single query scenario #21337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 4 additions & 14 deletions src/EFCore.Relational/Query/Internal/FromSqlQueryingEnumerable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ private sealed class Enumerator : IEnumerator<T>

private RelationalDataReader _dataReader;
private int[] _indexMap;
private IExecutionStrategy _executionStrategy;

public Enumerator(FromSqlQueryingEnumerable<T> queryingEnumerable)
{
Expand All @@ -169,12 +168,8 @@ public bool MoveNext()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

_executionStrategy.Execute(true, InitializeReader, null);
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(true, InitializeReader, null);
}

var hasNext = _dataReader.Read();
Expand Down Expand Up @@ -236,7 +231,6 @@ private sealed class AsyncEnumerator : IAsyncEnumerator<T>

private RelationalDataReader _dataReader;
private int[] _indexMap;
private IExecutionStrategy _executionStrategy;

public AsyncEnumerator(
FromSqlQueryingEnumerable<T> queryingEnumerable,
Expand All @@ -262,12 +256,8 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

await _executionStrategy.ExecuteAsync(true, InitializeReaderAsync, null, _cancellationToken).ConfigureAwait(false);
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(true, InitializeReaderAsync, null, _cancellationToken).ConfigureAwait(false);
}

var hasNext = await _dataReader.ReadAsync(_cancellationToken).ConfigureAwait(false);
Expand Down
18 changes: 4 additions & 14 deletions src/EFCore.Relational/Query/Internal/SingleQueryingEnumerable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ private sealed class Enumerator : IEnumerator<T>

private RelationalDataReader _dataReader;
private SingleQueryResultCoordinator _resultCoordinator;
private IExecutionStrategy _executionStrategy;

public Enumerator(SingleQueryingEnumerable<T> queryingEnumerable)
{
Expand All @@ -137,12 +136,8 @@ public bool MoveNext()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

_executionStrategy.Execute(true, InitializeReader, null);
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(true, InitializeReader, null);
}

var hasNext = _resultCoordinator.HasNext ?? _dataReader.Read();
Expand Down Expand Up @@ -228,7 +223,6 @@ private sealed class AsyncEnumerator : IAsyncEnumerator<T>

private RelationalDataReader _dataReader;
private SingleQueryResultCoordinator _resultCoordinator;
private IExecutionStrategy _executionStrategy;

public AsyncEnumerator(
SingleQueryingEnumerable<T> queryingEnumerable,
Expand All @@ -253,12 +247,8 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

await _executionStrategy.ExecuteAsync(true, InitializeReaderAsync, null, _cancellationToken)
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(true, InitializeReaderAsync, null, _cancellationToken)
.ConfigureAwait(false);
}

Expand Down
44 changes: 43 additions & 1 deletion src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,50 @@ public Expression ApplyCollectionJoin(
}

AddJoin(JoinType.OuterApply, ref innerSelectExpression);
var innerOrderingExpressions = new List<OrderingExpression>();
var joinedTable = innerSelectExpression.Tables.Single();
if (joinedTable is SelectExpression collectionSelectExpression
&& collectionSelectExpression.Predicate != null
&& collectionSelectExpression.Tables.Count == 1
&& collectionSelectExpression.Tables[0] is SelectExpression rowNumberSubquery
&& rowNumberSubquery.Projection.Select(pe => pe.Expression)
.OfType<RowNumberExpression>().SingleOrDefault() is RowNumberExpression rowNumberExpression)
{
foreach (var partition in rowNumberExpression.Partitions)
{
innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(partition)),
ascending: true));
}

foreach (var ordering in rowNumberExpression.Orderings)
{
innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression.GenerateOuterColumn(rowNumberSubquery.GenerateOuterColumn(ordering.Expression)),
ordering.IsAscending));
}
}
else if (joinedTable is SelectExpression collectionSelectExpression2
&& collectionSelectExpression2.Orderings.Count > 0)
{
foreach (var ordering in collectionSelectExpression2.Orderings)
{
if (innerSelectExpression._identifier.Any(e => e.Column.Equals(ordering.Expression)))
{
continue;
}

innerOrderingExpressions.Add(new OrderingExpression(
collectionSelectExpression2.GenerateOuterColumn(ordering.Expression),
ordering.IsAscending));
}
}
else
{
innerOrderingExpressions.AddRange(innerSelectExpression.Orderings);
}

foreach (var ordering in innerSelectExpression.Orderings)
foreach (var ordering in innerOrderingExpressions)
{
AppendOrdering(ordering.Update(MakeNullable(ordering.Expression)));
}
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5140,7 +5140,7 @@ public virtual Task Filtered_include_after_reference_navigation(bool async)
assertOrder: true)));
}

[ConditionalTheory(Skip = "issue #21338")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_after_different_filtered_include_same_level(bool async)
{
Expand Down Expand Up @@ -5205,7 +5205,7 @@ public virtual async Task Filtered_include_different_filter_set_on_same_navigati
.Include(l1 => l1.OneToMany_Optional1.Where(x => x.Name != "Bar")).ThenInclude(l2 => l2.OneToOne_Required_FK2)))).Message;
}

[ConditionalTheory(Skip = "issue #21338")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice(bool async)
{
Expand Down
30 changes: 15 additions & 15 deletions test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4567,7 +4567,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Name], [t0].[Id]");
}

public override async Task Filtered_include_basic_OrderBy_Skip(bool async)
Expand All @@ -4585,7 +4585,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE 1 < [t].[row]
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Name], [t0].[Id]");
}

public override async Task Filtered_include_basic_OrderBy_Skip_Take(bool async)
Expand All @@ -4603,7 +4603,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE (1 < [t].[row]) AND ([t].[row] <= 4)
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Name], [t0].[Id]");
}

public override void Filtered_include_Skip_without_OrderBy()
Expand All @@ -4621,7 +4621,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE 1 < [t].[row]
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override void Filtered_include_Take_without_OrderBy()
Expand All @@ -4639,7 +4639,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 1
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override async Task Filtered_include_on_ThenInclude(bool async)
Expand All @@ -4659,7 +4659,7 @@ FROM [LevelThree] AS [l1]
) AS [t]
WHERE (1 < [t].[row]) AND ([t].[row] <= 4)
) AS [t0] ON [l0].[Id] = [t0].[OneToMany_Optional_Inverse3Id]
ORDER BY [l].[Id], [l0].[Id], [t0].[Id]");
ORDER BY [l].[Id], [l0].[Id], [t0].[OneToMany_Optional_Inverse3Id], [t0].[Name], [t0].[Id]");
}

public override async Task Filtered_include_after_reference_navigation(bool async)
Expand All @@ -4679,7 +4679,7 @@ FROM [LevelThree] AS [l1]
) AS [t]
WHERE (1 < [t].[row]) AND ([t].[row] <= 4)
) AS [t0] ON [l0].[Id] = [t0].[OneToMany_Optional_Inverse3Id]
ORDER BY [l].[Id], [l0].[Id], [t0].[Id]");
ORDER BY [l].[Id], [l0].[Id], [t0].[OneToMany_Optional_Inverse3Id], [t0].[Name], [t0].[Id]");
}

public override async Task Filtered_include_after_different_filtered_include_same_level(bool async)
Expand Down Expand Up @@ -4707,7 +4707,7 @@ FROM [LevelTwo] AS [l1]
) AS [t1]
WHERE 1 < [t1].[row]
) AS [t2] ON [l].[Id] = [t2].[OneToMany_Required_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id], [t2].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Name], [t0].[Id], [t2].[OneToMany_Required_Inverse2Id], [t2].[Name] DESC, [t2].[Id]");
}

public override async Task Filtered_include_after_different_filtered_include_different_level(bool async)
Expand Down Expand Up @@ -4735,7 +4735,7 @@ FROM [LevelThree] AS [l1]
WHERE 1 < [t0].[row]
) AS [t1] ON [t].[Id] = [t1].[OneToMany_Required_Inverse3Id]
) AS [t2]
ORDER BY [l].[Id], [t2].[Name], [t2].[Id], [t2].[Id0]");
ORDER BY [l].[Id], [t2].[Name], [t2].[Id], [t2].[OneToMany_Required_Inverse3Id], [t2].[Name0] DESC, [t2].[Id0]");
}

public override async Task Filtered_include_same_filter_set_on_same_navigation_twice(bool async)
Expand All @@ -4754,7 +4754,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 2
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id] DESC");
}

public override async Task Filtered_include_same_filter_set_on_same_navigation_twice_followed_by_ThenIncludes(bool async)
Expand Down Expand Up @@ -4815,7 +4815,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override async Task Filtered_include_and_non_filtered_include_on_same_navigation2(bool async)
Expand All @@ -4834,7 +4834,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override async Task Filtered_include_and_non_filtered_include_followed_by_then_include_on_same_navigation(bool async)
Expand Down Expand Up @@ -4930,7 +4930,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override void Filtered_include_context_accessed_inside_filter()
Expand All @@ -4954,7 +4954,7 @@ FROM [LevelTwo] AS [l0]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override void Filtered_include_context_accessed_inside_filter_correlated()
Expand All @@ -4976,7 +4976,7 @@ FROM [LevelOne] AS [l1]
) AS [t]
WHERE [t].[row] <= 3
) AS [t0] ON [l].[Id] = [t0].[OneToMany_Optional_Inverse2Id]
ORDER BY [l].[Id], [t0].[Id]");
ORDER BY [l].[Id], [t0].[OneToMany_Optional_Inverse2Id], [t0].[Id]");
}

public override void Filtered_include_outer_parameter_used_inside_filter()
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5896,7 +5896,7 @@ FROM [Gears] AS [g0]
WHERE [t0].[row] <= 50
) AS [t1] ON (([g].[Nickname] = [t1].[LeaderNickname]) OR ([g].[Nickname] IS NULL AND [t1].[LeaderNickname] IS NULL)) AND ([g].[SquadId] = [t1].[LeaderSquadId])
WHERE [g].[Discriminator] = N'Officer'
ORDER BY [t].[Id], [g].[Nickname], [g].[SquadId], [t1].[Nickname], [t1].[SquadId]");
ORDER BY [t].[Id], [g].[Nickname], [g].[SquadId], [t1].[LeaderNickname], [t1].[LeaderSquadId], [t1].[Nickname], [t1].[SquadId]");
}

public override async Task Project_collection_navigation_nested_composite_key(bool async)
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4742,7 +4742,7 @@ WHERE DATEPART(year, [o].[OrderDate]) = 1998
) AS [t]
WHERE [t].[row] <= 1
) AS [t0] ON [c].[CustomerID] = [t0].[CustomerID]
ORDER BY [c].[CustomerID], [t0].[OrderID]");
ORDER BY [c].[CustomerID], [t0].[CustomerID], [t0].[OrderID]");
}

public override async Task Subquery_DefaultIfEmpty_Any(bool async)
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSelectQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ WHERE [o].[OrderID] < 10500
WHERE [t].[row] <= 3
) AS [t0] ON [c].[CustomerID] = [t0].[CustomerID]
WHERE [c].[CustomerID] LIKE N'A%'
ORDER BY [c].[CustomerID], [t0].[OrderID]");
ORDER BY [c].[CustomerID], [t0].[CustomerID], [t0].[OrderID]");
}

public override void Select_nested_collection_multi_level2()
Expand Down