Remove last ORDER BY for collection joins

Closes #19828
dotnet · Mar 11, 2021 · db61e87 · db61e87
1 parent d40410d
commit db61e87
Show file tree

Hide file tree

Showing 35 changed files with 685 additions and 666 deletions.
diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -55,6 +55,7 @@ private readonly IDictionary<EntityProjectionExpression, IDictionary<IProperty,
         private readonly List<TableExpressionBase> _tables = new();
         private readonly List<SqlExpression> _groupBy = new();
         private readonly List<OrderingExpression> _orderings = new();
+        private OrderingExpression? _pendingOrdering;
 
         private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _identifier
             = new();
@@ -627,12 +628,28 @@ public void ApplyOrdering([NotNull] OrderingExpression orderingExpression)
         /// </summary>
         /// <param name="orderingExpression"> An ordering expression to use for ordering. </param>
         public void AppendOrdering([NotNull] OrderingExpression orderingExpression)
+            => AppendOrdering(orderingExpression, isPending: false);
+
+        private void AppendOrdering([NotNull] OrderingExpression orderingExpression, bool isPending)
         {
             Check.NotNull(orderingExpression, nameof(orderingExpression));
 
             if (_orderings.FirstOrDefault(o => o.Expression.Equals(orderingExpression.Expression)) == null)
             {
-                _orderings.Add(orderingExpression);
+                if (_pendingOrdering is not null)
+                {
+                    _orderings.Add(_pendingOrdering);
+                }
+
+                if (isPending)
+                {
+                    _pendingOrdering = orderingExpression;
+                }
+                else
+                {
+                    _pendingOrdering = null;
+                    _orderings.Add(orderingExpression);
+                }
             }
         }
 
@@ -1624,7 +1641,7 @@ public CollectionShaperExpression AddCollectionProjection(
                 {
                     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 RelationalCollectionShaperExpression(

diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
@@ -6548,7 +6548,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,

diff --git a/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
@@ -1064,11 +1064,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++)
+                                var eComposition = ee.Composition.OrderBy(e => e.Id).ToList();
+                                var aComposition = aa.Composition.OrderBy(e => e.Id).ToList();
+                                for (var i = 0; i < eComposition.Count; i++)
                                 {
-                                    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(eComposition[i].Id, aComposition[i].Id);
+                                    Assert.Equal(eComposition[i].Name, aComposition[i].Name);
+                                    Assert.Equal(eComposition[i].StarId, aComposition[i].StarId);
                                 }
                             }
                         }

diff --git a/test/EFCore.SqlServer.FunctionalTests/ManyToManyLoadSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/ManyToManyLoadSqlServerTest.cs
@@ -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]");
         }
 
 
@@ -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)
@@ -93,7 +93,7 @@ FROM [EntityOneEntityTwo] AS [e4]
     WHERE [e3].[Id] = @__p_0
 ) AS [t1] ON [t].[Id] = [t1].[EntityTwoId]
 WHERE [e].[Id] = @__p_0
-ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t1].[EntityOneId], [t1].[EntityTwoId], [t1].[Id], [t1].[EntityOneId0], [t1].[EntityTwoId0], [t1].[Id0]");
+ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t1].[EntityOneId], [t1].[EntityTwoId], [t1].[Id], [t1].[EntityOneId0], [t1].[EntityTwoId0]");
         }
 
         public override async Task Load_collection_using_Query_with_Include(bool async)
@@ -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)
@@ -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)
@@ -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], [t1].[Id], [t1].[EntityOneId], [t1].[EntityTwoId], [t1].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId], [t2].[Id]");
+ORDER BY [e].[Id], [t].[EntityOneId], [t].[EntityTwoId], [t].[Id], [t1].[Id], [t1].[EntityOneId], [t1].[EntityTwoId], [t1].[Id0], [t2].[EntityOneId], [t2].[EntityTwoId]");
         }
 
         protected override void ClearLog()