Pushdown SelectExpression with OrderBy/Skip/Take when creating a set …

…operation Resolves #18362 Also resolves #17239
dotnet · Oct 15, 2019 · 40837a7 · 40837a7
1 parent 59fbd88
commit 40837a7
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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<Customer>()
+                    .OrderBy(c => c.ContactName)
+                    .Take(1)
+                    .Union(ss.Set<Customer>()
+                        .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
@@ -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
@@ -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(