Handle IN optimizer transformation of ordering to constant

src/EFCore.Relational/Query/Pipeline/InExpressionValuesExpandingExpressionVisitor.cs

@@ -1,9 +1,11 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq.Expressions;
+using Microsoft.EntityFrameworkCore.Query.Pipeline;
 using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Storage;
 
@@ -34,6 +36,7 @@ public override Expression Visit(Expression expression)
 
                     switch (inExpression.Values)
                     {
+                        // TODO: This shouldn't be here - should be part of compilation instead (#16375)
                         case SqlConstantExpression sqlConstant:
                             {
                                 typeMapping = sqlConstant.TypeMapping;
@@ -99,6 +102,66 @@ public override Expression Visit(Expression expression)
 
                 return base.Visit(expression);
             }
+
+            protected override Expression VisitExtension(Expression extensionExpression)
+            {
+                var visited = base.VisitExtension(extensionExpression);
+                if (visited is OrderingExpression orderingExpression)
+                {
+                    // Our rewriting of InExpression above may have left an ordering which is completely
+                    // constant, which is invalid in SqlServer. QuerySqlGenerator will recognize the constant
+                    // expression and render (SELECT 1).
+                    var columnSearchingVisitor = new SearchingExpressionVisitor(typeof(ColumnExpression));
+                    columnSearchingVisitor.Visit(orderingExpression);
+                    return columnSearchingVisitor.FoundExpression == null
+                        ? new OrderingExpression(
+                            new SqlConstantExpression(Expression.Constant(1), _sqlExpressionFactory.FindMapping(typeof(int))),
+                            true)
+                        : orderingExpression;
+                }
+
+                return visited;
+            }
+        }
+
+        /// <summary>
+        /// Searches an expression tree for the first occurrence of a node meeting the given criteria, and returns that node.
+        /// </summary>
+        private class SearchingExpressionVisitor : ExpressionVisitor
+        {
+            private Func<Expression, bool> _predicate;
+
+            public Expression FoundExpression { get; private set; }
+
+            public SearchingExpressionVisitor(Func<Expression, bool> predicate)
+            {
+                _predicate = predicate;
+            }
+
+            public SearchingExpressionVisitor(Type searchForType)
+                : this(searchForType.IsInstanceOfType)
+            {
+            }
+
+            /// <summary>
+            /// Resets the visitor, making it ready to run again.
+            /// </summary>
+            public void Reset()
+            {
+                FoundExpression = null;
+            }
+
+            public override Expression Visit(Expression node)
+            {
+                // TODO: can be optimized by immediately returning null when a matching node is found (but must be handled everywhere...)
+
+                if (FoundExpression == null && _predicate(node))
+                {
+                    FoundExpression = node;
+                }
+
+                return base.Visit(node);
+            }
         }
     }
 }

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

@@ -1070,9 +1070,9 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
                 var orderings = new List<OrderingExpression>();
                 foreach (var ordering in _orderings)
                 {
-                    var orderingExpression = (SqlExpression)visitor.Visit(ordering.Expression);
-                    changed |= orderingExpression != ordering.Expression;
-                    orderings.Add(ordering.Update(orderingExpression));
+                    var newOrdering = (OrderingExpression)visitor.Visit(ordering);
+                    changed |= newOrdering != ordering;
+                    orderings.Add(newOrdering);
                 }
 
                 var offset = (SqlExpression)visitor.Visit(Offset);

test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.cs

@@ -3642,6 +3642,7 @@ FROM root c
 WHERE ((c[""Discriminator""] = ""Order"") AND ((c[""OrderID""] > 690) AND (c[""OrderID""] < 710)))");
         }
 
+        [ConditionalTheory(Skip = "Should work, requires testing etc.")]
         public override async Task OrderBy_empty_list_contains(bool isAsync)
         {
             await base.OrderBy_empty_list_contains(isAsync);
@@ -3652,6 +3653,7 @@ FROM root c
 WHERE (c[""Discriminator""] = ""Customer"")");
         }
 
+        [ConditionalTheory(Skip = "Should work, requires testing etc.")]
         public override async Task OrderBy_empty_list_does_not_contains(bool isAsync)
         {
             await base.OrderBy_empty_list_does_not_contains(isAsync);

test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs

@@ -6872,7 +6872,7 @@ public virtual Task Cast_ordered_subquery_to_base_type_using_typed_ToArray(bool
                 elementAsserter: CollectionAsserter<Gear>(e => e.Nickname, (e, a) => Assert.Equal(e.Nickname, a.Nickname)));
         }
 
-        [ConditionalTheory(Skip = "Issue#15713")]
+        [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Correlated_collection_with_complex_order_by_funcletized_to_constant_bool(bool isAsync)
         {

test/EFCore.Specification.Tests/Query/IncludeTestBase.cs

@@ -3933,7 +3933,7 @@ var orders
             }
         }
 
-        [ConditionalTheory(Skip = "Issue#15713")]
+        [ConditionalTheory]
         [InlineData(false)]
         [InlineData(true)]
         public virtual void Include_collection_OrderBy_empty_list_contains(bool useString)
@@ -3968,7 +3968,7 @@ var customers
             }
         }
 
-        [ConditionalTheory(Skip = "Issue#15713")]
+        [ConditionalTheory]
         [InlineData(false)]
         [InlineData(true)]
         public virtual void Include_collection_OrderBy_empty_list_does_not_contains(bool useString)

test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs

@@ -5735,7 +5735,7 @@ on o.CustomerID equals c.CustomerID
                     .Take(5));
         }
 
-        [ConditionalTheory(Skip = "Issue#15713")]
+        [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task OrderBy_empty_list_contains(bool isAsync)
         {
@@ -5747,7 +5747,7 @@ public virtual Task OrderBy_empty_list_contains(bool isAsync)
                 entryCount: 91);
         }
 
-        [ConditionalTheory(Skip = "Issue#15713")]
+        [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task OrderBy_empty_list_does_not_contains(bool isAsync)
         {

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

@@ -7413,19 +7413,11 @@ public override async Task Correlated_collection_with_complex_order_by_funcletiz
             await base.Correlated_collection_with_complex_order_by_funcletized_to_constant_bool(isAsync);
 
             AssertSql(
-                @"SELECT [g].[Nickname], [g].[FullName]
+                @"SELECT [g].[Nickname], [g].[SquadId], [w].[Name], [w].[Id], [w].[OwnerFullName]
 FROM [Gears] AS [g]
-WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
-ORDER BY [g].[Nickname], [g].[SquadId], [g].[FullName]",
-                //
-                @"SELECT [t].[c], [t].[Nickname], [t].[SquadId], [t].[FullName], [g.Weapons].[Name], [g.Weapons].[OwnerFullName]
-FROM [Weapons] AS [g.Weapons]
-INNER JOIN (
-    SELECT CAST(0 AS bit) AS [c], [g0].[Nickname], [g0].[SquadId], [g0].[FullName]
-    FROM [Gears] AS [g0]
-    WHERE [g0].[Discriminator] IN (N'Officer', N'Gear')
-) AS [t] ON [g.Weapons].[OwnerFullName] = [t].[FullName]
-ORDER BY [t].[c] DESC, [t].[Nickname], [t].[SquadId], [t].[FullName]");
+LEFT JOIN [Weapons] AS [w] ON [g].[FullName] = [w].[OwnerFullName]
+WHERE [g].[Discriminator] IN (N'Gear', N'Officer')
+ORDER BY [g].[Nickname], [g].[SquadId], [w].[Id]");
         }
 
         public override async Task Double_order_by_on_nullable_bool_coming_from_optional_navigation(bool isAsync)

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

@@ -1116,24 +1116,19 @@ public override void Include_collection_OrderBy_empty_list_contains(bool useStri
             AssertSql(
                 @"@__p_1='1'
 
-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].[CustomerID] LIKE N'A%'
-ORDER BY (SELECT 1), [c].[CustomerID]
-OFFSET @__p_1 ROWS",
-                //
-                @"@__p_1='1'
-
-SELECT [c.Orders].[OrderID], [c.Orders].[CustomerID], [c.Orders].[EmployeeID], [c.Orders].[OrderDate]
-FROM [Orders] AS [c.Orders]
-INNER JOIN (
-    SELECT [c0].[CustomerID], CAST(0 AS bit) AS [c]
-    FROM [Customers] AS [c0]
-    WHERE [c0].[CustomerID] LIKE N'A%'
-    ORDER BY [c], [c0].[CustomerID]
+SELECT [t].[CustomerID], [t].[Address], [t].[City], [t].[CompanyName], [t].[ContactName], [t].[ContactTitle], [t].[Country], [t].[Fax], [t].[Phone], [t].[PostalCode], [t].[Region], [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
+FROM (
+    SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region], CASE
+        WHEN CAST(1 AS bit) = CAST(0 AS bit) THEN CAST(1 AS bit)
+        ELSE CAST(0 AS bit)
+    END AS [c]
+    FROM [Customers] AS [c]
+    WHERE [c].[CustomerID] LIKE N'A%'
+    ORDER BY (SELECT 1)
     OFFSET @__p_1 ROWS
-) AS [t] ON [c.Orders].[CustomerID] = [t].[CustomerID]
-ORDER BY [t].[c], [t].[CustomerID]");
+) AS [t]
+LEFT JOIN [Orders] AS [o] ON [t].[CustomerID] = [o].[CustomerID]
+ORDER BY [t].[c], [t].[CustomerID], [o].[OrderID]");
         }
 
         public override void Include_collection_OrderBy_empty_list_does_not_contains(bool useString)
@@ -1143,24 +1138,19 @@ public override void Include_collection_OrderBy_empty_list_does_not_contains(boo
             AssertSql(
                 @"@__p_1='1'
 
-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].[CustomerID] LIKE N'A%'
-ORDER BY (SELECT 1), [c].[CustomerID]
-OFFSET @__p_1 ROWS",
-                //
-                @"@__p_1='1'
-
-SELECT [c.Orders].[OrderID], [c.Orders].[CustomerID], [c.Orders].[EmployeeID], [c.Orders].[OrderDate]
-FROM [Orders] AS [c.Orders]
-INNER JOIN (
-    SELECT [c0].[CustomerID], CAST(1 AS bit) AS [c]
-    FROM [Customers] AS [c0]
-    WHERE [c0].[CustomerID] LIKE N'A%'
-    ORDER BY [c], [c0].[CustomerID]
+SELECT [t].[CustomerID], [t].[Address], [t].[City], [t].[CompanyName], [t].[ContactName], [t].[ContactTitle], [t].[Country], [t].[Fax], [t].[Phone], [t].[PostalCode], [t].[Region], [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
+FROM (
+    SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region], CASE
+        WHEN CAST(1 AS bit) = CAST(1 AS bit) THEN CAST(1 AS bit)
+        ELSE CAST(0 AS bit)
+    END AS [c]
+    FROM [Customers] AS [c]
+    WHERE [c].[CustomerID] LIKE N'A%'
+    ORDER BY (SELECT 1)
     OFFSET @__p_1 ROWS
-) AS [t] ON [c.Orders].[CustomerID] = [t].[CustomerID]
-ORDER BY [t].[c], [t].[CustomerID]");
+) AS [t]
+LEFT JOIN [Orders] AS [o] ON [t].[CustomerID] = [o].[CustomerID]
+ORDER BY [t].[c], [t].[CustomerID], [o].[OrderID]");
         }
 
         public override void Include_collection_OrderBy_list_contains(bool useString)