From 8dd411da5a86f7f3818c252d59619f09fba0cee2 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 26 Jun 2024 13:27:36 -0700 Subject: [PATCH 1/3] Make sort order consistent in split queries between the parent query and its occurrences in subqueries for collection includes. Fixes #26808 --- .../Query/SqlExpressions/SelectExpression.cs | 18 +-- ...AdHocAdvancedMappingsQuerySqlServerTest.cs | 1 + ...tionsCollectionsSplitQuerySqlServerTest.cs | 3 +- ...plitIncludeNoTrackingQuerySqlServerTest.cs | 107 +++++++++++++---- ...NorthwindSplitIncludeQuerySqlServerTest.cs | 109 ++++++++++++++---- .../Query/OwnedEntityQuerySqlServerTest.cs | 5 +- 6 files changed, 182 insertions(+), 61 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index d57e2210403..09c0ee164e3 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -912,6 +912,16 @@ static Expression RemoveConvert(Expression expression) { var outerSelectExpression = (SelectExpression)cloningExpressionVisitor!.Visit(baseSelectExpression!); + // Deterministic orderings are applied before subquery pushdown + // so that we don't have to dig into the subquery to add them to `outerSelectExpression`. + 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 +933,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? orderingsToBeErased = null; if (containsOrdering @@ -941,13 +950,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) { diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocAdvancedMappingsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocAdvancedMappingsQuerySqlServerTest.cs index 6f0bce9d4dd..f46218f1dac 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocAdvancedMappingsQuerySqlServerTest.cs +++ b/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] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsSplitQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsSplitQuerySqlServerTest.cs index fac384c0bcd..0260fb59e9b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsCollectionsSplitQuerySqlServerTest.cs +++ b/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], ( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeNoTrackingQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeNoTrackingQuerySqlServerTest.cs index 9113c9c34ee..bcf8e84cd0c 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeNoTrackingQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeNoTrackingQuerySqlServerTest.cs @@ -24,14 +24,64 @@ public virtual void Check_all_tests_overridden() => TestHelpers.AssertAllMethodsOverridden(GetType()); public override async Task Include_collection_skip_take_no_order_by(bool async) - => Assert.Equal( - SqlServerStrings.SplitQueryOffsetWithoutOrderBy, - (await Assert.ThrowsAsync(() => base.Include_collection_skip_take_no_order_by(async))).Message); + { + await base.Include_collection_skip_take_no_order_by(async); + + AssertSql( +""" +@__p_0='10' +@__p_1='5' + +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +ORDER BY [c].[CustomerID] +OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY +""", + // + """ +@__p_0='10' +@__p_1='5' + +SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate], [c0].[CustomerID] +FROM ( + SELECT [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] + OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY +) AS [c0] +INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] +ORDER BY [c0].[CustomerID] +"""); + } public override async Task Include_collection_skip_no_order_by(bool async) - => Assert.Equal( - SqlServerStrings.SplitQueryOffsetWithoutOrderBy, - (await Assert.ThrowsAsync(() => base.Include_collection_skip_no_order_by(async))).Message); + { + await base.Include_collection_skip_no_order_by(async); + + AssertSql( +""" +@__p_0='10' + +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +ORDER BY [c].[CustomerID] +OFFSET @__p_0 ROWS +""", + // + """ +@__p_0='10' + +SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate], [c0].[CustomerID] +FROM ( + SELECT [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] + OFFSET @__p_0 ROWS +) AS [c0] +INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] +ORDER BY [c0].[CustomerID] +"""); + } public override async Task Include_reference_GroupBy_Select(bool async) { @@ -77,6 +127,7 @@ ORDER BY [o].[OrderID] SELECT TOP(1) [o].[OrderID] FROM [Orders] AS [o] WHERE [o].[OrderID] = 10248 + ORDER BY [o].[OrderID] ) AS [o1] INNER JOIN ( SELECT [o0].[OrderID], [o0].[ProductID], [o0].[Discount], [o0].[Quantity], [o0].[UnitPrice], [p].[ProductID] AS [ProductID0], [p].[Discontinued], [p].[ProductName], [p].[SupplierID], [p].[UnitPrice] AS [UnitPrice0], [p].[UnitsInStock] @@ -145,7 +196,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -183,7 +234,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c] DESC, [c0].[CustomerID] @@ -348,7 +399,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o] ON [s].[CustomerID] = [o].[CustomerID] ORDER BY [s].[CustomerID], [s].[CustomerID0] @@ -372,7 +423,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s0] INNER JOIN [Orders] AS [o0] ON [s0].[CustomerID0] = [o0].[CustomerID] ORDER BY [s0].[CustomerID], [s0].[CustomerID0] @@ -529,7 +580,7 @@ ELSE CAST(0 AS bit) END, CASE WHEN [c].[CustomerID] IS NOT NULL THEN [c].[City] ELSE N'' - END + END, [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Order Details] AS [o0] ON [s].[OrderID] = [o0].[OrderID] ORDER BY [s].[c], [s].[c0], [s].[OrderID], [s].[CustomerID] @@ -731,7 +782,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(1) [c].[CustomerID], [c].[CompanyName] FROM [Customers] AS [c] - ORDER BY [c].[CompanyName] DESC + ORDER BY [c].[CompanyName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CompanyName] DESC, [c0].[CustomerID] @@ -925,7 +976,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[EmployeeID]) + ORDER BY [o].[EmployeeID]), [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c], [c0].[CustomerID] @@ -948,7 +999,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(1) [c].[CustomerID], [c].[CompanyName] FROM [Customers] AS [c] - ORDER BY [c].[CompanyName] DESC + ORDER BY [c].[CompanyName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CompanyName] DESC, [c0].[CustomerID] @@ -1093,6 +1144,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -1142,7 +1194,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -1174,6 +1226,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -1385,7 +1438,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c] DESC, [c0].[CustomerID], [o0].[OrderID] @@ -1405,7 +1458,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] INNER JOIN [Order Details] AS [o1] ON [o0].[OrderID] = [o1].[OrderID] @@ -1589,7 +1642,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -1651,6 +1704,7 @@ ORDER BY [c].[CustomerID] FROM ( SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -1768,7 +1822,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -1909,7 +1963,7 @@ ELSE CAST(0 AS bit) END, CASE WHEN [c].[CustomerID] IS NOT NULL THEN [c].[CustomerID] ELSE N'' - END + END, [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Order Details] AS [o0] ON [s].[OrderID] = [o0].[OrderID] ORDER BY [s].[c], [s].[c0], [s].[OrderID], [s].[CustomerID] @@ -2107,7 +2161,7 @@ OFFSET @__p_0 ROWS FROM ( SELECT [c].[CustomerID], [c].[ContactName] FROM [Customers] AS [c] - ORDER BY [c].[ContactName] + ORDER BY [c].[ContactName], [c].[CustomerID] OFFSET @__p_0 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -2135,6 +2189,7 @@ ORDER BY [c].[CustomerID] FROM ( SELECT TOP(@__p_0) [c].[CustomerID] FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -2301,7 +2356,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o] ON [s].[CustomerID] = [o].[CustomerID] ORDER BY [s].[CustomerID], [s].[CustomerID0] @@ -2418,7 +2473,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(@__p_0) [c].[CustomerID], [c].[ContactTitle] FROM [Customers] AS [c] - ORDER BY [c].[ContactTitle] + ORDER BY [c].[ContactTitle], [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[ContactTitle], [c0].[CustomerID] @@ -2445,7 +2500,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(@__p_0) [c].[CustomerID], [c].[ContactName] FROM [Customers] AS [c] - ORDER BY [c].[ContactName] DESC + ORDER BY [c].[ContactName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[ContactName] DESC, [c0].[CustomerID] @@ -2551,6 +2606,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID], [o].[OrderID] @@ -2562,6 +2618,7 @@ FROM [Customers] AS [c] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] INNER JOIN [Order Details] AS [o0] ON [o].[OrderID] = [o0].[OrderID] @@ -2592,7 +2649,7 @@ OFFSET @__p_0 ROWS SELECT [c].[CustomerID], [c].[ContactTitle] FROM [Customers] AS [c] WHERE [c].[CustomerID] LIKE N'F%' - ORDER BY [c].[ContactTitle] + ORDER BY [c].[ContactTitle], [c].[CustomerID] OFFSET @__p_0 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeQuerySqlServerTest.cs index 3c7f3cd592a..713f49e31ed 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindSplitIncludeQuerySqlServerTest.cs @@ -114,7 +114,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(1) [c].[CustomerID], [c].[CompanyName] FROM [Customers] AS [c] - ORDER BY [c].[CompanyName] DESC + ORDER BY [c].[CompanyName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CompanyName] DESC, [c0].[CustomerID] @@ -122,9 +122,33 @@ ORDER BY [c].[CompanyName] DESC } public override async Task Include_collection_skip_no_order_by(bool async) - => Assert.Equal( - SqlServerStrings.SplitQueryOffsetWithoutOrderBy, - (await Assert.ThrowsAsync(() => base.Include_collection_skip_no_order_by(async))).Message); + { + await base.Include_collection_skip_no_order_by(async); + + AssertSql( +""" +@__p_0='10' + +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +ORDER BY [c].[CustomerID] +OFFSET @__p_0 ROWS +""", + // + """ +@__p_0='10' + +SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate], [c0].[CustomerID] +FROM ( + SELECT [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] + OFFSET @__p_0 ROWS +) AS [c0] +INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] +ORDER BY [c0].[CustomerID] +"""); + } public override async Task Include_collection_take_no_order_by(bool async) { @@ -146,6 +170,7 @@ ORDER BY [c].[CustomerID] FROM ( SELECT TOP(@__p_0) [c].[CustomerID] FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -153,9 +178,35 @@ ORDER BY [c0].[CustomerID] } public override async Task Include_collection_skip_take_no_order_by(bool async) - => Assert.Equal( - SqlServerStrings.SplitQueryOffsetWithoutOrderBy, - (await Assert.ThrowsAsync(() => base.Include_collection_skip_take_no_order_by(async))).Message); + { + await base.Include_collection_skip_take_no_order_by(async); + + AssertSql( +""" +@__p_0='10' +@__p_1='5' + +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +ORDER BY [c].[CustomerID] +OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY +""", + // + """ +@__p_0='10' +@__p_1='5' + +SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate], [c0].[CustomerID] +FROM ( + SELECT [c].[CustomerID] + FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] + OFFSET @__p_0 ROWS FETCH NEXT @__p_1 ROWS ONLY +) AS [c0] +INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] +ORDER BY [c0].[CustomerID] +"""); + } public override async Task Include_reference_and_collection(bool async) { @@ -289,6 +340,7 @@ SELECT TOP(1) [o].[OrderID], [c].[CustomerID] FROM [Orders] AS [o] LEFT JOIN [Customers] AS [c] ON [o].[CustomerID] = [c].[CustomerID] WHERE [o].[OrderID] = 10248 + ORDER BY [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o0] ON [s].[CustomerID] = [o0].[CustomerID] ORDER BY [s].[OrderID], [s].[CustomerID] @@ -313,6 +365,7 @@ ORDER BY [o].[OrderID] SELECT TOP(1) [o].[OrderID] FROM [Orders] AS [o] WHERE [o].[OrderID] = 10248 + ORDER BY [o].[OrderID] ) AS [o1] INNER JOIN ( SELECT [o0].[OrderID], [o0].[ProductID], [o0].[Discount], [o0].[Quantity], [o0].[UnitPrice], [p].[ProductID] AS [ProductID0], [p].[Discontinued], [p].[ProductName], [p].[SupplierID], [p].[UnitPrice] AS [UnitPrice0], [p].[UnitsInStock] @@ -374,7 +427,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c] DESC, [c0].[CustomerID] @@ -443,7 +496,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(@__p_0) [c].[CustomerID], [c].[ContactTitle] FROM [Customers] AS [c] - ORDER BY [c].[ContactTitle] + ORDER BY [c].[ContactTitle], [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[ContactTitle], [c0].[CustomerID] @@ -473,7 +526,7 @@ OFFSET @__p_0 ROWS SELECT [c].[CustomerID], [c].[ContactTitle] FROM [Customers] AS [c] WHERE [c].[CustomerID] LIKE N'F%' - ORDER BY [c].[ContactTitle] + ORDER BY [c].[ContactTitle], [c].[CustomerID] OFFSET @__p_0 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -497,7 +550,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(1) [c].[CustomerID], [c].[CompanyName] FROM [Customers] AS [c] - ORDER BY [c].[CompanyName] DESC + ORDER BY [c].[CompanyName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CompanyName] DESC, [c0].[CustomerID] @@ -534,7 +587,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[EmployeeID]) + ORDER BY [o].[EmployeeID]), [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c], [c0].[CustomerID] @@ -565,6 +618,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -1005,7 +1059,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o] ON [s].[CustomerID] = [o].[CustomerID] ORDER BY [s].[CustomerID], [s].[CustomerID0] @@ -1029,7 +1083,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s0] INNER JOIN [Orders] AS [o0] ON [s0].[CustomerID0] = [o0].[CustomerID] ORDER BY [s0].[CustomerID], [s0].[CustomerID0] @@ -1175,7 +1229,7 @@ FROM [Customers] AS [c0] ORDER BY [c0].[CustomerID] OFFSET 2 ROWS FETCH NEXT 2 ROWS ONLY ) AS [c2] - ORDER BY [c1].[CustomerID] + ORDER BY [c1].[CustomerID], [c2].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o] ON [s].[CustomerID] = [o].[CustomerID] ORDER BY [s].[CustomerID], [s].[CustomerID0] @@ -1346,6 +1400,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -1422,7 +1477,7 @@ FROM [Customers] AS [c] FROM ( SELECT TOP(@__p_0) [c].[CustomerID], [c].[ContactName] FROM [Customers] AS [c] - ORDER BY [c].[ContactName] DESC + ORDER BY [c].[ContactName] DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[ContactName] DESC, [c0].[CustomerID] @@ -1450,7 +1505,7 @@ OFFSET @__p_0 ROWS FROM ( SELECT [c].[CustomerID], [c].[ContactName] FROM [Customers] AS [c] - ORDER BY [c].[ContactName] + ORDER BY [c].[ContactName], [c].[CustomerID] OFFSET @__p_0 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -1498,7 +1553,7 @@ ELSE CAST(0 AS bit) END, CASE WHEN [c].[CustomerID] IS NOT NULL THEN [c].[City] ELSE N'' - END + END, [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Order Details] AS [o0] ON [s].[OrderID] = [o0].[OrderID] ORDER BY [s].[c], [s].[c0], [s].[OrderID], [s].[CustomerID] @@ -1535,7 +1590,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] ORDER BY [c0].[c] DESC, [c0].[CustomerID], [o0].[OrderID] @@ -1555,7 +1610,7 @@ ORDER BY ( SELECT TOP(1) [o].[OrderDate] FROM [Orders] AS [o] WHERE [c].[CustomerID] = [o].[CustomerID] - ORDER BY [o].[OrderDate] DESC) DESC + ORDER BY [o].[OrderDate] DESC) DESC, [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o0] ON [c0].[CustomerID] = [o0].[CustomerID] INNER JOIN [Order Details] AS [o1] ON [o0].[OrderID] = [o1].[OrderID] @@ -2084,7 +2139,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -2135,7 +2190,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -2186,7 +2241,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -2237,7 +2292,7 @@ SELECT [l].[value] FROM OPENJSON(@__list_0) WITH ([value] nchar(5) '$') AS [l] ) THEN CAST(1 AS bit) ELSE CAST(0 AS bit) - END + END, [c].[CustomerID] OFFSET @__p_1 ROWS ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] @@ -2611,6 +2666,7 @@ ORDER BY [c].[CustomerID] FROM ( SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID] @@ -2895,6 +2951,7 @@ ORDER BY [c].[CustomerID] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] ORDER BY [c0].[CustomerID], [o].[OrderID] @@ -2906,6 +2963,7 @@ FROM [Customers] AS [c] SELECT TOP(1) [c].[CustomerID] FROM [Customers] AS [c] WHERE [c].[CustomerID] = N'ALFKI' + ORDER BY [c].[CustomerID] ) AS [c0] INNER JOIN [Orders] AS [o] ON [c0].[CustomerID] = [o].[CustomerID] INNER JOIN [Order Details] AS [o0] ON [o].[OrderID] = [o0].[OrderID] @@ -3068,7 +3126,7 @@ ELSE CAST(0 AS bit) END, CASE WHEN [c].[CustomerID] IS NOT NULL THEN [c].[CustomerID] ELSE N'' - END + END, [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Order Details] AS [o0] ON [s].[OrderID] = [o0].[OrderID] ORDER BY [s].[c], [s].[c0], [s].[OrderID], [s].[CustomerID] @@ -3108,6 +3166,7 @@ SELECT TOP(1) [o].[OrderID], [c].[CustomerID] FROM [Orders] AS [o] LEFT JOIN [Customers] AS [c] ON [o].[CustomerID] = [c].[CustomerID] WHERE [o].[OrderID] = 10248 + ORDER BY [o].[OrderID], [c].[CustomerID] ) AS [s] INNER JOIN [Orders] AS [o0] ON [s].[CustomerID] = [o0].[CustomerID] ORDER BY [s].[OrderID], [s].[CustomerID] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs index 5eb74c50918..84bedf8fb4a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs @@ -266,7 +266,7 @@ SELECT TOP(1) [o].[Id], [o0].[Owner23211Id], [o1].[Owner23211Id] AS [Owner23211I FROM [Owner23211] AS [o] LEFT JOIN [Owned1_23211] AS [o0] ON [o].[Id] = [o0].[Owner23211Id] LEFT JOIN [Owned2_23211] AS [o1] ON [o].[Id] = [o1].[Owner23211Id] - ORDER BY [o].[Id] + ORDER BY [o].[Id], [o0].[Owner23211Id], [o1].[Owner23211Id] ) AS [s] INNER JOIN [Dependent23211] AS [d] ON [s].[Id] = [d].[Owner23211Id] ORDER BY [s].[Id], [s].[Owner23211Id], [s].[Owner23211Id0] @@ -297,7 +297,7 @@ FROM [SecondOwner23211] AS [s] SELECT TOP(1) [s].[Id], [o].[SecondOwner23211Id] FROM [SecondOwner23211] AS [s] LEFT JOIN [Owned23211] AS [o] ON [s].[Id] = [o].[SecondOwner23211Id] - ORDER BY [s].[Id] + ORDER BY [s].[Id], [o].[SecondOwner23211Id] ) AS [s1] INNER JOIN [SecondDependent23211] AS [s0] ON [s1].[Id] = [s0].[SecondOwner23211Id] ORDER BY [s1].[Id], [s1].[SecondOwner23211Id] @@ -536,6 +536,7 @@ FROM [Root] AS [r] LEFT JOIN [MiddleB] AS [m] ON [r].[Id] = [m].[RootId] LEFT JOIN [ModdleA] AS [m0] ON [r].[Id] = [m0].[RootId] WHERE [r].[Id] = 3 + ORDER BY [r].[Id], [m].[Id], [m0].[Id] ) AS [s] INNER JOIN [Leaf] AS [l0] ON [s].[Id1] = [l0].[ModdleAId] ORDER BY [s].[Id], [s].[Id0], [s].[Id1] From 4a29e305e91b20bd9f42a3bc1a7f211e2edbc850 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 29 Jul 2024 18:37:47 -0700 Subject: [PATCH 2/3] Update src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs Co-authored-by: Shay Rojansky --- .../Query/SqlExpressions/SelectExpression.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 09c0ee164e3..edf92bff7a5 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -912,8 +912,10 @@ static Expression RemoveConvert(Expression expression) { var outerSelectExpression = (SelectExpression)cloningExpressionVisitor!.Visit(baseSelectExpression!); - // Deterministic orderings are applied before subquery pushdown - // so that we don't have to dig into the subquery to add them to `outerSelectExpression`. + // 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++) { From 0c318ac97796cd62738bf8f8df52f366fc3dffdb Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 29 Jul 2024 19:13:50 -0700 Subject: [PATCH 3/3] Remove SplitQueryOffsetWithoutOrderBy error checking that can't happen anymore --- .../Query/SqlExpressions/SelectExpression.cs | 8 ++--- .../Properties/SqlServerStrings.Designer.cs | 6 ---- .../Properties/SqlServerStrings.resx | 3 -- .../SqlServerQueryTranslationPostprocessor.cs | 35 ------------------- 4 files changed, 4 insertions(+), 48 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index edf92bff7a5..d3931c36b7b 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -912,10 +912,10 @@ 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) + // 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++) { diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 00b27a4de1a..60215b821cb 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -291,12 +291,6 @@ public static string SequenceBadType(object? property, object? entityType, objec GetString("SequenceBadType", nameof(property), nameof(entityType), nameof(propertyType)), property, entityType, propertyType); - /// - /// 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. - /// - public static string SplitQueryOffsetWithoutOrderBy - => GetString("SplitQueryOffsetWithoutOrderBy"); - /// /// 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. /// diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 2f8726af548..dc00eb238cb 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -321,9 +321,6 @@ 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. - - 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. - 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. diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs index 714a20e123b..019af6c5c0a 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs +++ b/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(); /// @@ -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) /// 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); - } - } - } }