Skip to content

Commit

Permalink
Remove last ORDER BY for collection joins
Browse files Browse the repository at this point in the history 
Closes #19828
  • Loading branch information
@roji
roji committed Aug 10, 2021
1 parent 159c7e7 commit 1c59244
Show file tree
Hide file tree
Showing 42 changed files with 791 additions and 762 deletions.
31 changes: 29 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public sealed partial class SelectExpression : TableExpressionBase
private readonly List<TableReferenceExpression> _tableReferences = new();
private readonly List<SqlExpression> _groupBy = new();
private readonly List<OrderingExpression> _orderings = new();
private OrderingExpression? _pendingOrdering;
private HashSet<string> _usedAliases = new();

private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _identifier = new();
Expand Down Expand Up @@ -732,7 +733,7 @@ static Expression RemoveConvert(Expression expression)
{
var updatedColumn = identifier.Column.MakeNullable();
_childIdentifiers.Add((updatedColumn, identifier.Comparer));
AppendOrdering(new OrderingExpression(updatedColumn, ascending: true));
AppendOrdering(new OrderingExpression(updatedColumn, ascending: true), isPending: true);
}

var result = new SingleCollectionInfo(
Expand Down Expand Up @@ -1206,13 +1207,39 @@ public void ApplyOrdering(OrderingExpression orderingExpression)
/// </summary>
/// <param name="orderingExpression"> An ordering expression to use for ordering. </param>
public void AppendOrdering(OrderingExpression orderingExpression)
=> AppendOrdering(orderingExpression, isPending: false);

private void AppendOrdering(OrderingExpression orderingExpression, bool isPending)
{
Check.NotNull(orderingExpression, nameof(orderingExpression));

if (orderingExpression.Equals(_pendingOrdering))
{
AppendOrderingCore(_pendingOrdering);
_pendingOrdering = null;
return;
}

if (_orderings.FirstOrDefault(o => o.Expression.Equals(orderingExpression.Expression)) == null)
{
_orderings.Add(orderingExpression.Update(AssignUniqueAliases(orderingExpression.Expression)));
if (_pendingOrdering is not null)
{
AppendOrderingCore(_pendingOrdering);
_pendingOrdering = null;
}

if (isPending)
{
_pendingOrdering = orderingExpression;
}
else
{
AppendOrderingCore(orderingExpression);
}
}

void AppendOrderingCore(OrderingExpression orderingExpression)
=> _orderings.Add(orderingExpression.Update(AssignUniqueAliases(orderingExpression.Expression)));
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6568,7 +6568,7 @@ public virtual Task Nullable_bool_comparison_is_translated_to_server(bool async)

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Acessing_reference_navigation_collection_composition_generates_single_query(bool async)
public virtual Task Accessing_reference_navigation_collection_composition_generates_single_query(bool async)
{
return AssertQuery(
async,
Expand Down
10 changes: 6 additions & 4 deletions test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1095,11 +1095,13 @@ public IReadOnlyDictionary<Type, object> GetEntityAsserters()
Assert.Equal(ee.Id, aa.Id);
Assert.Equal(ee.Name, aa.Name);
Assert.Equal(ee.Composition.Count, aa.Composition.Count);
for (var i = 0; i < ee.Composition.Count; i++)
foreach (var (eec, aac) in Enumerable.Zip(
ee.Composition.OrderBy(eec => eec.Id),
aa.Composition.OrderBy(aac => aac.Id)))
{
Assert.Equal(ee.Composition[i].Id, aa.Composition[i].Id);
Assert.Equal(ee.Composition[i].Name, aa.Composition[i].Name);
Assert.Equal(ee.Composition[i].StarId, aa.Composition[i].StarId);
Assert.Equal(eec.Id, aac.Id);
Assert.Equal(eec.Name, aac.Name);
Assert.Equal(eec.StarId, aac.StarId);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions test/EFCore.SqlServer.FunctionalTests/ManyToManyFieldsLoadSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ FROM [JoinOneToTwo] AS [j0]
WHERE [e1].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id], [t0].[OneId], [t0].[TwoId], [t0].[Id]");
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id], [t0].[OneId], [t0].[TwoId]");
}

public override async Task Load_collection_using_Query_with_Include_for_inverse(bool async)
Expand All @@ -64,7 +64,7 @@ FROM [EntityOneEntityTwo] AS [e2]
WHERE [e3].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId]");
}

public override async Task Load_collection_using_Query_with_Include_for_same_collection(bool async)
Expand Down Expand Up @@ -93,7 +93,7 @@ FROM [EntityOneEntityTwo] AS [e4]
WHERE [e3].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t0].[EntityOneId0], [t0].[EntityTwoId0], [t0].[Id0]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t0].[EntityOneId0], [t0].[EntityTwoId0]");
}

public override async Task Load_collection_using_Query_with_Include(bool async)
Expand Down Expand Up @@ -122,7 +122,7 @@ FROM [JoinTwoToThree] AS [j]
INNER JOIN [EntityThrees] AS [e4] ON [j].[ThreeId] = [e4].[Id]
) AS [t1] ON [t].[Id] = [t1].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId], [t1].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId]");
}

public override async Task Load_collection_using_Query_with_filtered_Include(bool async)
Expand Down Expand Up @@ -152,7 +152,7 @@ FROM [JoinTwoToThree] AS [j]
WHERE [e4].[Id] IN (13, 11)
) AS [t1] ON [t].[Id] = [t1].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId], [t1].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId]");
}

public override async Task Load_collection_using_Query_with_filtered_Include_and_projection(bool async)
Expand Down Expand Up @@ -211,7 +211,7 @@ FROM [EntityOneEntityTwo] AS [e5]
WHERE [e6].[Id] = @__p_0
) AS [t2] ON [t].[Id] = [t2].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId], [t2].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId]");
}

protected override void ClearLog()
Expand Down
12 changes: 6 additions & 6 deletions test/EFCore.SqlServer.FunctionalTests/ManyToManyLoadSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ FROM [JoinOneToTwo] AS [j0]
WHERE [e1].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id], [t0].[OneId], [t0].[TwoId], [t0].[Id]");
ORDER BY [e].[Id], [t].[OneId], [t].[TwoId], [t].[Id], [t0].[OneId], [t0].[TwoId]");
}

public override async Task Load_collection_using_Query_with_Include_for_inverse(bool async)
Expand All @@ -63,7 +63,7 @@ FROM [EntityOneEntityTwo] AS [e2]
WHERE [e3].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId]");
}

public override async Task Load_collection_using_Query_with_Include_for_same_collection(bool async)
Expand Down Expand Up @@ -92,7 +92,7 @@ FROM [EntityOneEntityTwo] AS [e4]
WHERE [e3].[Id] = @__p_0
) AS [t0] ON [t].[Id] = [t0].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t0].[EntityOneId0], [t0].[EntityTwoId0], [t0].[Id0]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t0].[EntityOneId0], [t0].[EntityTwoId0]");
}

public override async Task Load_collection_using_Query_with_Include(bool async)
Expand Down Expand Up @@ -121,7 +121,7 @@ FROM [JoinTwoToThree] AS [j]
INNER JOIN [EntityThrees] AS [e4] ON [j].[ThreeId] = [e4].[Id]
) AS [t1] ON [t].[Id] = [t1].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId], [t1].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId]");
}

public override async Task Load_collection_using_Query_with_filtered_Include(bool async)
Expand Down Expand Up @@ -151,7 +151,7 @@ FROM [JoinTwoToThree] AS [j]
WHERE [e4].[Id] IN (13, 11)
) AS [t1] ON [t].[Id] = [t1].[TwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId], [t1].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id], [t1].[ThreeId], [t1].[TwoId]");
}

public override async Task Load_collection_using_Query_with_filtered_Include_and_projection(bool async)
Expand Down Expand Up @@ -210,7 +210,7 @@ FROM [EntityOneEntityTwo] AS [e5]
WHERE [e6].[Id] = @__p_0
) AS [t2] ON [t].[Id] = [t2].[EntityTwoId]
WHERE [e].[Id] = @__p_0
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId], [t2].[Id]");
ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t0].[Id], [t0].[EntityOneId], [t0].[EntityTwoId], [t0].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId]");
}

protected override void ClearLog()
Expand Down
Loading

0 comments on commit 1c59244

Please sign in to comment.