From 0b10f4d0c9a6fa85bcab975bee861e46e3792966 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Mon, 14 Oct 2019 13:41:58 -0700 Subject: [PATCH] Pushdown SelectExpression with OrderBy/Skip/Take when creating a set operation Resolves #18362 Also resolves #17239 --- .../Query/SqlExpressions/SelectExpression.cs | 16 ++++++ .../SimpleQueryCosmosTest.SetOperations.cs | 1 + .../SimpleQueryTestBase.SetOperations.cs | 15 +++++ .../SimpleQuerySqlServerTest.SetOperations.cs | 55 ++++++++++++++----- .../Query/SimpleQuerySqliteTest.cs | 6 -- 5 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index f3ceee93a7d..3669595a05e 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -477,6 +477,22 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi select1._identifier.AddRange(_identifier); _identifier.Clear(); + if (select1.Orderings.Count != 0 + || select1.Limit != null + || select1.Offset != null) + { + select1.PushdownIntoSubquery(); + select1.ClearOrdering(); + } + + if (select2.Orderings.Count != 0 + || select2.Limit != null + || select2.Offset != null) + { + select2.PushdownIntoSubquery(); + select2.ClearOrdering(); + } + var setExpression = (SetOperationBase)(setOperationType switch { SetOperationType.Except => new ExceptExpression("t", select1, select2, distinct), diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs b/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs index e1578dd920b..8f348355eae 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.SetOperations.cs @@ -41,5 +41,6 @@ public override void Include_Union_different_includes_throws() { } public override Task Client_eval_Union_FirstOrDefault(bool isAsync) => Task.CompletedTask; public override Task GroupBy_Select_Union(bool isAsync) => Task.CompletedTask; public override Task Union_over_different_projection_types(bool isAsync, string leftType, string rightType) => Task.CompletedTask; + public override Task OrderBy_Take_Union(bool isAsync) => Task.CompletedTask; } } diff --git a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs index 57e9d05bf13..89f49efeb28 100644 --- a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs +++ b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.SetOperations.cs @@ -488,5 +488,20 @@ from rightType in _supportedOperandExpressionType { "Column", "Function", "Constant", "Unary", "Binary", "ScalarSubquery" }; + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task OrderBy_Take_Union(bool isAsync) + { + return AssertQuery( + isAsync, ss => ss.Set() + .OrderBy(c => c.ContactName) + .Take(1) + .Union(ss.Set() + .OrderBy(c => c.ContactName) + .Take(1)), + entryCount: 1, + assertOrder: true); + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs index a645afd26e7..3568a7fbe19 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.SetOperations.cs @@ -176,28 +176,31 @@ public override async Task Union_Take_Union_Take(bool isAsync) AssertSql( @"@__p_0='1' -SELECT [t1].[CustomerID], [t1].[Address], [t1].[City], [t1].[CompanyName], [t1].[ContactName], [t1].[ContactTitle], [t1].[Country], [t1].[Fax], [t1].[Phone], [t1].[PostalCode], [t1].[Region] +SELECT [t2].[CustomerID], [t2].[Address], [t2].[City], [t2].[CompanyName], [t2].[ContactName], [t2].[ContactTitle], [t2].[Country], [t2].[Fax], [t2].[Phone], [t2].[PostalCode], [t2].[Region] FROM ( - SELECT TOP(@__p_0) [t0].[CustomerID], [t0].[Address], [t0].[City], [t0].[CompanyName], [t0].[ContactName], [t0].[ContactTitle], [t0].[Country], [t0].[Fax], [t0].[Phone], [t0].[PostalCode], [t0].[Region] + SELECT TOP(@__p_0) [t1].[CustomerID], [t1].[Address], [t1].[City], [t1].[CompanyName], [t1].[ContactName], [t1].[ContactTitle], [t1].[Country], [t1].[Fax], [t1].[Phone], [t1].[PostalCode], [t1].[Region] FROM ( - SELECT TOP(@__p_0) [t].[CustomerID], [t].[Address], [t].[City], [t].[CompanyName], [t].[ContactName], [t].[ContactTitle], [t].[Country], [t].[Fax], [t].[Phone], [t].[PostalCode], [t].[Region] + SELECT [t0].[CustomerID], [t0].[Address], [t0].[City], [t0].[CompanyName], [t0].[ContactName], [t0].[ContactTitle], [t0].[Country], [t0].[Fax], [t0].[Phone], [t0].[PostalCode], [t0].[Region] FROM ( - 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] - WHERE ([c].[City] = N'Berlin') AND [c].[City] IS NOT NULL - UNION - SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region] - FROM [Customers] AS [c0] - WHERE ([c0].[City] = N'London') AND [c0].[City] IS NOT NULL - ) AS [t] - ORDER BY [t].[CustomerID] + SELECT TOP(@__p_0) [t].[CustomerID], [t].[Address], [t].[City], [t].[CompanyName], [t].[ContactName], [t].[ContactTitle], [t].[Country], [t].[Fax], [t].[Phone], [t].[PostalCode], [t].[Region] + FROM ( + 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] + WHERE ([c].[City] = N'Berlin') AND [c].[City] IS NOT NULL + UNION + SELECT [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region] + FROM [Customers] AS [c0] + WHERE ([c0].[City] = N'London') AND [c0].[City] IS NOT NULL + ) AS [t] + ORDER BY [t].[CustomerID] + ) AS [t0] UNION SELECT [c1].[CustomerID], [c1].[Address], [c1].[City], [c1].[CompanyName], [c1].[ContactName], [c1].[ContactTitle], [c1].[Country], [c1].[Fax], [c1].[Phone], [c1].[PostalCode], [c1].[Region] FROM [Customers] AS [c1] WHERE ([c1].[City] = N'Mannheim') AND [c1].[City] IS NOT NULL - ) AS [t0] -) AS [t1] -ORDER BY [t1].[CustomerID]"); + ) AS [t1] +) AS [t2] +ORDER BY [t2].[CustomerID]"); } public override async Task Select_Union(bool isAsync) @@ -460,5 +463,27 @@ FROM [Order Details] AS [o] } } } + + public override async Task OrderBy_Take_Union(bool isAsync) + { + await base.OrderBy_Take_Union(isAsync); + + AssertSql( + @"@__p_0='1' + +SELECT [t].[CustomerID], [t].[Address], [t].[City], [t].[CompanyName], [t].[ContactName], [t].[ContactTitle], [t].[Country], [t].[Fax], [t].[Phone], [t].[PostalCode], [t].[Region] +FROM ( + SELECT TOP(@__p_0) [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].[ContactName] +) AS [t] +UNION +SELECT [t0].[CustomerID], [t0].[Address], [t0].[City], [t0].[CompanyName], [t0].[ContactName], [t0].[ContactTitle], [t0].[Country], [t0].[Fax], [t0].[Phone], [t0].[PostalCode], [t0].[Region] +FROM ( + SELECT TOP(@__p_0) [c0].[CustomerID], [c0].[Address], [c0].[City], [c0].[CompanyName], [c0].[ContactName], [c0].[ContactTitle], [c0].[Country], [c0].[Fax], [c0].[Phone], [c0].[PostalCode], [c0].[Region] + FROM [Customers] AS [c0] + ORDER BY [c0].[ContactName] +) AS [t0]"); + } } } diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs index 3a54ae760e0..499e7c3040e 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs @@ -1332,12 +1332,6 @@ public override Task Like_with_non_string_column_using_ToString(bool isAsync) return base.Like_with_non_string_column_using_ToString(isAsync); } - [ConditionalTheory(Skip = "Issue#17239")] - public override Task Union_Take_Union_Take(bool isAsync) - { - return base.Union_Take_Union_Take(isAsync); - } - public override async Task Member_binding_after_ctor_arguments_fails_with_client_eval(bool isAsync) { Assert.Equal(