From 9dcfdffd123ed393ec692a8eac4ce4bb8edc482c Mon Sep 17 00:00:00 2001
From: Smit Patel <smitpatel@users.noreply.github.com>
Date: Wed, 26 Aug 2020 10:42:13 -0700
Subject: [PATCH] Query: Pushdown into subquery when applying over Skip after
 Distinct without order by

Since we add OrderBy(Select 1) for skip, Distinct needs to have ordering column in the projection

Resolves #8523
---
 .../Query/SqlExpressions/SelectExpression.cs  |  3 +-
 .../Query/ComplexNavigationsQueryTestBase.cs  | 26 +++++++++++++
 .../ComplexNavigationsQuerySqlServerTest.cs   | 37 +++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
index 098e90888f3..d87b4f2bef9 100644
--- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
+++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -656,7 +656,8 @@ public void ApplyOffset([NotNull] SqlExpression sqlExpression)
             Check.NotNull(sqlExpression, nameof(sqlExpression));
 
             if (Limit != null
-                || Offset != null)
+                || Offset != null
+                || (IsDistinct && Orderings.Count == 0))
             {
                 PushdownIntoSubquery();
             }
diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
index 0bc3fe62bb5..5b075e1e03b 100644
--- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
+++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs
@@ -5619,5 +5619,31 @@ public virtual Task Member_over_null_check_ternary_and_nested_anonymous_type(boo
                     }
                 });
         }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Distinct_skip_without_orderby(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => from l1 in ss.Set<Level1>()
+                      where l1.Id < 3
+                      select (from l3 in ss.Set<Level3>()
+                              orderby l3.Id
+                              select l3).Distinct().Skip(1).OrderBy(e => e.Id).FirstOrDefault().Name);
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Distinct_take_without_orderby(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => from l1 in ss.Set<Level1>()
+                      where l1.Id < 3
+                      select (from l3 in ss.Set<Level3>()
+                              orderby l3.Id
+                              select l3).Distinct().Take(1).OrderBy(e => e.Id).FirstOrDefault().Name);
+        }
     }
 }
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
index b767e02b0f7..fe3cc6ec8f9 100644
--- a/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
+++ b/test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
@@ -5860,6 +5860,43 @@ FROM [LevelOne] AS [l]
 ORDER BY [t].[Id], [l0].[Id]");
         }
 
+        public override async Task Distinct_skip_without_orderby(bool async)
+        {
+            await base.Distinct_skip_without_orderby(async);
+
+            AssertSql(
+                @"SELECT (
+    SELECT TOP(1) [t0].[Name]
+    FROM (
+        SELECT [t].[Id], [t].[Level2_Optional_Id], [t].[Level2_Required_Id], [t].[Name], [t].[OneToMany_Optional_Inverse3Id], [t].[OneToMany_Optional_Self_Inverse3Id], [t].[OneToMany_Required_Inverse3Id], [t].[OneToMany_Required_Self_Inverse3Id], [t].[OneToOne_Optional_PK_Inverse3Id], [t].[OneToOne_Optional_Self3Id]
+        FROM (
+            SELECT DISTINCT [l].[Id], [l].[Level2_Optional_Id], [l].[Level2_Required_Id], [l].[Name], [l].[OneToMany_Optional_Inverse3Id], [l].[OneToMany_Optional_Self_Inverse3Id], [l].[OneToMany_Required_Inverse3Id], [l].[OneToMany_Required_Self_Inverse3Id], [l].[OneToOne_Optional_PK_Inverse3Id], [l].[OneToOne_Optional_Self3Id]
+            FROM [LevelThree] AS [l]
+        ) AS [t]
+        ORDER BY (SELECT 1)
+        OFFSET 1 ROWS
+    ) AS [t0]
+    ORDER BY [t0].[Id])
+FROM [LevelOne] AS [l0]
+WHERE [l0].[Id] < 3");
+        }
+
+        public override async Task Distinct_take_without_orderby(bool async)
+        {
+            await base.Distinct_take_without_orderby(async);
+
+            AssertSql(
+                @"SELECT (
+    SELECT TOP(1) [t].[Name]
+    FROM (
+        SELECT DISTINCT TOP(1) [l].[Id], [l].[Level2_Optional_Id], [l].[Level2_Required_Id], [l].[Name], [l].[OneToMany_Optional_Inverse3Id], [l].[OneToMany_Optional_Self_Inverse3Id], [l].[OneToMany_Required_Inverse3Id], [l].[OneToMany_Required_Self_Inverse3Id], [l].[OneToOne_Optional_PK_Inverse3Id], [l].[OneToOne_Optional_Self3Id]
+        FROM [LevelThree] AS [l]
+    ) AS [t]
+    ORDER BY [t].[Id])
+FROM [LevelOne] AS [l0]
+WHERE [l0].[Id] < 3");
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
     }