Remove OrderByClause from query models with Contains, All and Any result operators (#2510)

Author: hazzik

src/NHibernate.Test/Async/Linq/WhereSubqueryTests.cs

@@ -487,6 +487,21 @@ public async Task OrdersWithSubquery11Async()
 			Assert.That(productsNotInLargestOrders.Count, Is.EqualTo(49), nameof(productsNotInLargestOrders));
 		}
 
+		[Test]
+		public async Task OrdersWithSubquery11AAsync()
+		{
+			var ordersQuery = db.Orders
+			                    .OrderByDescending(x => x.OrderLines.Count).ThenBy(x => x.OrderId);
+
+			var orderLineQuery = ordersQuery.SelectMany(x => x.OrderLines);
+			var productsNotInLargestOrders = await (db.Products
+			                                   .Where(x => orderLineQuery.All(p => p.Product != x))
+			                                   .OrderBy(x => x.ProductId)
+			                                   .ToListAsync());
+
+			Assert.That(productsNotInLargestOrders.Count, Is.EqualTo(0), nameof(productsNotInLargestOrders));
+		}
+
 		[Test(Description = "NH-2654")]
 		public async Task CategoriesWithDiscountedProductsAsync()
 		{

src/NHibernate.Test/Linq/WhereSubqueryTests.cs

@@ -519,7 +519,7 @@ public void OrdersWithSubquery9()
 
 			var ordersQuery = db.Orders
 			                    .Where(x => x.Employee.EmployeeId > 5)
-			                    .OrderBy(x => x.OrderId)
+			                    .OrderByDescending(x => x.OrderId)
 			                    .Take(2);
 
 			var orderLinesFuture = db.OrderLines
@@ -531,7 +531,26 @@ public void OrdersWithSubquery9()
 			var orderLines = orderLinesFuture.ToList();
 
 			Assert.That(orders.Count, Is.EqualTo(2), nameof(orders));
-			Assert.That(orderLines.Count, Is.EqualTo(6), nameof(orderLines));
+			Assert.That(orderLines.Count, Is.EqualTo(4), nameof(orderLines));
+		}
+
+		[Test]
+		public void OrdersWithSubquery9A()
+		{
+			var ordersQuery = db.Orders
+			                    .Where(x => x.Employee.EmployeeId > 5)
+			                    .OrderByDescending(x => x.OrderId);
+
+			var orderLinesFuture = db.OrderLines
+			                         .Where(x => ordersQuery.Any(o => o == x.Order))
+			                         .OrderBy(x => x.Id)
+			                         .ToFuture();
+
+			var orders = ordersQuery.ToFuture().ToList();
+			var orderLines = orderLinesFuture.ToList();
+
+			Assert.That(orders.Count, Is.EqualTo(286), nameof(orders));
+			Assert.That(orderLines.Count, Is.EqualTo(711), nameof(orderLines));
 		}
 
 		[Test(Description = "GH2479")]
@@ -542,7 +561,7 @@ public void OrdersWithSubquery10()
 
 			var ordersQuery = db.Orders
 			                    .Where(x => x.Employee.EmployeeId > 5)
-			                    .OrderBy(x => x.OrderId)
+			                    .OrderByDescending(x => x.OrderId)
 			                    .Take(2);
 
 			var productsQuery = ordersQuery.SelectMany(x => x.OrderLines).Select(x => x.Product);
@@ -555,7 +574,27 @@ public void OrdersWithSubquery10()
 			var products = productsFuture.ToList();
 
 			Assert.That(orders.Count, Is.EqualTo(2), nameof(orders));
-			Assert.That(products.Count, Is.EqualTo(6), nameof(products));
+			Assert.That(products.Count, Is.EqualTo(4), nameof(products));
+		}
+
+		[Test]
+		public void OrdersWithSubquery10A()
+		{
+			var ordersQuery = db.Orders
+			                    .Where(x => x.Employee.EmployeeId > 5)
+			                    .OrderByDescending(x => x.OrderId);
+
+			var productsQuery = ordersQuery.SelectMany(x => x.OrderLines).Select(x => x.Product);
+			var productsFuture = db.Products
+			                       .Where(x => productsQuery.Contains(x))
+			                       .OrderBy(x => x.ProductId)
+			                       .ToFuture();
+
+			var orders = ordersQuery.ToFuture().ToList();
+			var products = productsFuture.ToList();
+
+			Assert.That(orders.Count, Is.EqualTo(286), nameof(orders));
+			Assert.That(products.Count, Is.EqualTo(77), nameof(products));
 		}
 
 		[Test(Description = "GH2479")]
@@ -579,6 +618,21 @@ public void OrdersWithSubquery11()
 			Assert.That(productsNotInLargestOrders.Count, Is.EqualTo(49), nameof(productsNotInLargestOrders));
 		}
 
+		[Test]
+		public void OrdersWithSubquery11A()
+		{
+			var ordersQuery = db.Orders
+			                    .OrderByDescending(x => x.OrderLines.Count).ThenBy(x => x.OrderId);
+
+			var orderLineQuery = ordersQuery.SelectMany(x => x.OrderLines);
+			var productsNotInLargestOrders = db.Products
+			                                   .Where(x => orderLineQuery.All(p => p.Product != x))
+			                                   .OrderBy(x => x.ProductId)
+			                                   .ToList();
+
+			Assert.That(productsNotInLargestOrders.Count, Is.EqualTo(0), nameof(productsNotInLargestOrders));
+		}
+
 		[Test(Description = "NH-2654")]
 		public void CategoriesWithDiscountedProducts()
 		{

src/NHibernate/Linq/GroupBy/PagingRewriter.cs

@@ -53,8 +53,7 @@ private static void FlattenSubQuery(SubQueryExpression subQueryExpression, Query
 				var where = new WhereClause(new SubQueryExpression(newSubQueryModel));
 				queryModel.BodyClauses.Add(where);
 
-				if (!queryModel.BodyClauses.OfType<OrderByClause>().Any() && 
-					!(queryModel.ResultOperators.Count == 1 && queryModel.ResultOperators.All(x => x is AnyResultOperator || x is ContainsResultOperator || x is AllResultOperator)))
+				if (!queryModel.BodyClauses.OfType<OrderByClause>().Any())
 				{
 					var orderByClauses = subQueryModel.BodyClauses.OfType<OrderByClause>();
 					foreach (var orderByClause in orderByClauses)

src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs

@@ -19,11 +19,27 @@ public static void ReWrite(QueryModel queryModel)
 			rewriter.VisitQueryModel(queryModel);
 		}
 
+		internal static void RemoveUnnecessaryOrderByClauses(QueryModel queryModel)
+		{
+			if (queryModel.ResultOperators.Count == 1 &&
+			    queryModel.ResultOperators.All(
+				    r => r is ContainsResultOperator || r is AnyResultOperator || r is AllResultOperator))
+			{
+				// For these operators, we can remove any order-by clause
+				var bodyClauses = queryModel.BodyClauses;
+				for (int i = bodyClauses.Count - 1; i >= 0; i--)
+				{
+					if (bodyClauses[i] is OrderByClause)
+						bodyClauses.RemoveAt(i);
+				}
+			}
+		}
+
 		public override void VisitResultOperator(ResultOperatorBase resultOperator, QueryModel queryModel, int index)
 		{
 			if (resultOperator is CountResultOperator || resultOperator is LongCountResultOperator)
 			{
-				// For count operators, we can remove any order-by result operators
+				// For count operators, we can remove any order-by clause
 				var bodyClauses = queryModel.BodyClauses.OfType<OrderByClause>().ToList();
 				foreach (var orderby in bodyClauses)
 				{

src/NHibernate/Linq/Visitors/QueryModelVisitor.cs

@@ -65,6 +65,9 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 			// Rewrite paging
 			PagingRewriter.ReWrite(queryModel);
 
+			//Remove unnecessary order-by clauses
+			RemoveUnnecessaryBodyOperators.RemoveUnnecessaryOrderByClauses(queryModel);
+
 			// Flatten pointless subqueries
 			QueryReferenceExpressionFlattener.ReWrite(queryModel);