Make sort order consistent in split queries (#34097)

Fixes #26808

src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

@@ -912,6 +912,18 @@ static Expression RemoveConvert(Expression expression)
                         {
                             var outerSelectExpression = (SelectExpression)cloningExpressionVisitor!.Visit(baseSelectExpression!);
 
+                            // Inject deterministic orderings (the identifier columns) to both the main query and the split query.
+                            // Note that just below we pushdown the split query if it has limit/offset/distinct/groupby; this ensures
+                            // that the orderings are also propagated to that split subquery if it has limit/offset, which ensures that
+                            // that subquery returns the same rows as the main query (#26808)
+                            var actualParentIdentifier = _identifier.Take(outerSelectExpression._identifier.Count).ToList();
+                            for (var j = 0; j < actualParentIdentifier.Count; j++)
+                            {
+                                AppendOrdering(new OrderingExpression(actualParentIdentifier[j].Column, ascending: true));
+                                outerSelectExpression.AppendOrdering(
+                                    new OrderingExpression(outerSelectExpression._identifier[j].Column, ascending: true));
+                            }
+
                             if (outerSelectExpression.Limit != null
                                 || outerSelectExpression.Offset != null
                                 || outerSelectExpression.IsDistinct
@@ -923,7 +935,6 @@ static Expression RemoveConvert(Expression expression)
                                 innerSelectExpression = sqlRemappingVisitor.Remap(innerSelectExpression);
                             }
 
-                            var actualParentIdentifier = _identifier.Take(outerSelectExpression._identifier.Count).ToList();
                             var containsOrdering = innerSelectExpression.Orderings.Count > 0;
                             List<OrderingExpression>? orderingsToBeErased = null;
                             if (containsOrdering
@@ -941,13 +952,6 @@ static Expression RemoveConvert(Expression expression)
                             outerSelectExpression._aliasForClientProjections.AddRange(innerSelectExpression._aliasForClientProjections);
                             innerSelectExpression = outerSelectExpression;
 
-                            for (var j = 0; j < actualParentIdentifier.Count; j++)
-                            {
-                                AppendOrdering(new OrderingExpression(actualParentIdentifier[j].Column, ascending: true));
-                                innerSelectExpression.AppendOrdering(
-                                    new OrderingExpression(innerSelectExpression._identifier[j].Column, ascending: true));
-                            }
-
                             // Copy over any nested ordering if there were any
                             if (containsOrdering)
                             {

src/EFCore.SqlServer/Properties/SqlServerStrings.resx

@@ -321,9 +321,6 @@
   <data name="SequenceBadType" xml:space="preserve">
     <value>SQL Server sequences cannot be used to generate values for the property '{property}' on entity type '{entityType}' because the property type is '{propertyType}'. Sequences can only be used with integer properties.</value>
   </data>
-  <data name="SplitQueryOffsetWithoutOrderBy" xml:space="preserve">
-    <value>The query uses 'Skip' without specifying ordering and uses split query mode. This generates incorrect results. Either provide ordering or run query in single query mode using `AsSingleQuery()`. See https://go.microsoft.com/fwlink/?linkid=2196526 for more information.</value>
-  </data>
   <data name="TemporalAllEntitiesMappedToSameTableMustBeTemporal" xml:space="preserve">
     <value>Entity type '{entityType}' should be marked as temporal because it shares table mapping with another entity that has been marked as temporal. Alternatively, other entity types that share the same table must be non-temporal.</value>
   </data>

src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs

@@ -17,7 +17,6 @@ public class SqlServerQueryTranslationPostprocessor : RelationalQueryTranslation
 {
     private readonly SqlServerJsonPostprocessor _jsonPostprocessor;
     private readonly SqlServerAggregateOverSubqueryPostprocessor _aggregatePostprocessor;
-    private readonly SkipWithoutOrderByInSplitQueryVerifier _skipWithoutOrderByInSplitQueryVerifier = new();
     private readonly SqlServerSqlTreePruner _pruner = new();
 
     /// <summary>
@@ -49,7 +48,6 @@ public override Expression Process(Expression query)
 
         var query2 = _jsonPostprocessor.Process(query1);
         var query3 = _aggregatePostprocessor.Visit(query2);
-        _skipWithoutOrderByInSplitQueryVerifier.Visit(query3);
 
         return query3;
     }
@@ -72,37 +70,4 @@ protected override Expression ProcessTypeMappings(Expression expression)
     /// </summary>
     protected override Expression Prune(Expression query)
         => _pruner.Prune(query);
-
-    private sealed class SkipWithoutOrderByInSplitQueryVerifier : ExpressionVisitor
-    {
-        [return: NotNullIfNotNull(nameof(expression))]
-        public override Expression? Visit(Expression? expression)
-        {
-            switch (expression)
-            {
-                case ShapedQueryExpression shapedQueryExpression:
-                    Visit(shapedQueryExpression.ShaperExpression);
-                    return shapedQueryExpression;
-
-                case RelationalSplitCollectionShaperExpression relationalSplitCollectionShaperExpression:
-                    foreach (var table in relationalSplitCollectionShaperExpression.SelectExpression.Tables)
-                    {
-                        Visit(table);
-                    }
-
-                    Visit(relationalSplitCollectionShaperExpression.InnerShaper);
-
-                    return relationalSplitCollectionShaperExpression;
-
-                case SelectExpression { Offset: not null, Orderings.Count: 0 }:
-                    throw new InvalidOperationException(SqlServerStrings.SplitQueryOffsetWithoutOrderBy);
-
-                case UpdateExpression or DeleteExpression:
-                    return expression;
-
-                default:
-                    return base.Visit(expression);
-            }
-        }
-    }
 }

test/EFCore.SqlServer.FunctionalTests/Query/AdHocAdvancedMappingsQuerySqlServerTest.cs

@@ -277,6 +277,7 @@ ORDER BY [o].[Id]
     SELECT TOP(1) [o].[Id]
     FROM [Offers] AS [o]
     WHERE [o].[Id] = 1
+    ORDER BY [o].[Id]
 ) AS [o0]
 INNER JOIN (
     SELECT [v].[Id], [v].[NestedId], [v].[OfferId], [v].[payment_brutto], [v].[payment_netto], [n].[Id] AS [Id0], [n].[payment_brutto] AS [payment_brutto0], [n].[payment_netto] AS [payment_netto0]

test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsSplitQuerySqlServerTest.cs

@@ -3437,6 +3437,7 @@ ORDER BY [l].[Id]
     SELECT TOP(1) [l].[Id]
     FROM [LevelOne] AS [l]
     WHERE [l].[Id] = 1
+    ORDER BY [l].[Id]
 ) AS [l3]
 INNER JOIN [LevelTwo] AS [l2] ON [l3].[Id] = [l2].[OneToMany_Optional_Inverse2Id]
 ORDER BY [l3].[Id]
@@ -3965,7 +3966,7 @@ SELECT TOP(1) [l].[Id], [l0].[Id] AS [Id0], [l].[Name]
     FROM [LevelOne] AS [l]
     LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
     WHERE [l].[Id] = 2
-    ORDER BY [l].[Name]
+    ORDER BY [l].[Name], [l].[Id], [l0].[Id]
 ) AS [s]
 CROSS APPLY (
     SELECT [l21].[Id], [l21].[Date], [l21].[Name], [l21].[OneToMany_Optional_Self_Inverse1Id], [l21].[OneToMany_Required_Self_Inverse1Id], [l21].[OneToOne_Optional_Self1Id], (