From 2ad7dcda893ed5fbbd7d89850a0cf2cb3ac2de8d Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Wed, 2 Sep 2020 16:13:22 +1200 Subject: [PATCH 1/7] Remove OrderByClause from queries with Contains, All and Any result operators --- .../Linq/WhereSubqueryTests.cs | 57 +++++++++++++++++++ .../RemoveUnnecessaryBodyOperators.cs | 8 ++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs b/src/NHibernate.Test/Linq/WhereSubqueryTests.cs index 9f951546406..016ee68d128 100644 --- a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs +++ b/src/NHibernate.Test/Linq/WhereSubqueryTests.cs @@ -534,6 +534,25 @@ public void OrdersWithSubquery9() Assert.That(orderLines.Count, Is.EqualTo(6), nameof(orderLines)); } + [Test] + public void OrdersWithSubquery9A() + { + var ordersQuery = db.Orders + .Where(x => x.Employee.EmployeeId > 5) + .OrderBy(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")] public void OrdersWithSubquery10() { @@ -558,6 +577,26 @@ public void OrdersWithSubquery10() Assert.That(products.Count, Is.EqualTo(6), nameof(products)); } + [Test] + public void OrdersWithSubquery10A() + { + var ordersQuery = db.Orders + .Where(x => x.Employee.EmployeeId > 5) + .OrderBy(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(orders.Count, Is.EqualTo(286), nameof(orders)); + } + [Test(Description = "GH2479")] public void OrdersWithSubquery11() { @@ -579,6 +618,24 @@ public void OrdersWithSubquery11() Assert.That(productsNotInLargestOrders.Count, Is.EqualTo(49), nameof(productsNotInLargestOrders)); } + [Test] + public void OrdersWithSubquery11A() + { + //if (Dialect is MsSqlCeDialect) + // Assert.Ignore("MS SQL CE does not support sorting on a subquery."); + + 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() { diff --git a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs index 86c57c70cd8..3bbaf455ae4 100644 --- a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs +++ b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs @@ -21,9 +21,13 @@ public static void ReWrite(QueryModel queryModel) public override void VisitResultOperator(ResultOperatorBase resultOperator, QueryModel queryModel, int index) { - if (resultOperator is CountResultOperator || resultOperator is LongCountResultOperator) + if (resultOperator is CountResultOperator || + resultOperator is LongCountResultOperator || + resultOperator is ContainsResultOperator || + resultOperator is AnyResultOperator || + resultOperator is AllResultOperator) { - // For count operators, we can remove any order-by result operators + // For these operators, we can remove any order-by result operators var bodyClauses = queryModel.BodyClauses.OfType().ToList(); foreach (var orderby in bodyClauses) { From 4128c9a720da7526c95722607d43eebe21130877 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Wed, 2 Sep 2020 16:17:38 +1200 Subject: [PATCH 2/7] Improve comment --- src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs index 3bbaf455ae4..e57df3f04fb 100644 --- a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs +++ b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs @@ -27,7 +27,7 @@ resultOperator is ContainsResultOperator || resultOperator is AnyResultOperator || resultOperator is AllResultOperator) { - // For these operators, we can remove any order-by result operators + // For these operators, we can remove any order-by clause var bodyClauses = queryModel.BodyClauses.OfType().ToList(); foreach (var orderby in bodyClauses) { From 6504244144ac45ebde5ec3241b6442e7cdb29e10 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Wed, 2 Sep 2020 21:08:14 +1200 Subject: [PATCH 3/7] Update tests to use order-by descending --- src/NHibernate.Test/Linq/WhereSubqueryTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs b/src/NHibernate.Test/Linq/WhereSubqueryTests.cs index 016ee68d128..c0f2cf25d6d 100644 --- a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs +++ b/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,7 @@ 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] @@ -539,7 +539,7 @@ public void OrdersWithSubquery9A() { var ordersQuery = db.Orders .Where(x => x.Employee.EmployeeId > 5) - .OrderBy(x => x.OrderId); + .OrderByDescending(x => x.OrderId); var orderLinesFuture = db.OrderLines .Where(x => ordersQuery.Any(o => o == x.Order)) @@ -561,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); @@ -574,7 +574,7 @@ 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] @@ -582,7 +582,7 @@ public void OrdersWithSubquery10A() { var ordersQuery = db.Orders .Where(x => x.Employee.EmployeeId > 5) - .OrderBy(x => x.OrderId); + .OrderByDescending(x => x.OrderId); var productsQuery = ordersQuery.SelectMany(x => x.OrderLines).Select(x => x.Product); var productsFuture = db.Products @@ -594,7 +594,7 @@ public void OrdersWithSubquery10A() var products = productsFuture.ToList(); Assert.That(orders.Count, Is.EqualTo(286), nameof(orders)); - Assert.That(orders.Count, Is.EqualTo(286), nameof(orders)); + Assert.That(products.Count, Is.EqualTo(77), nameof(products)); } [Test(Description = "GH2479")] From b4f6c65aba2a449998eb56bf113f558288e4f384 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Wed, 2 Sep 2020 21:16:01 +1200 Subject: [PATCH 4/7] Fix --- .../RemoveUnnecessaryBodyOperators.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs index e57df3f04fb..469c511c0c5 100644 --- a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs +++ b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs @@ -16,18 +16,25 @@ public static void ReWrite(QueryModel queryModel) { var rewriter = new RemoveUnnecessaryBodyOperators(); + if (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.OfType().ToList(); + foreach (var orderby in bodyClauses) + { + queryModel.BodyClauses.Remove(orderby); + } + } + rewriter.VisitQueryModel(queryModel); } public override void VisitResultOperator(ResultOperatorBase resultOperator, QueryModel queryModel, int index) { - if (resultOperator is CountResultOperator || - resultOperator is LongCountResultOperator || - resultOperator is ContainsResultOperator || - resultOperator is AnyResultOperator || - resultOperator is AllResultOperator) + if (resultOperator is CountResultOperator || resultOperator is LongCountResultOperator) { - // For these operators, we can remove any order-by clause + // For count operators, we can remove any order-by clause var bodyClauses = queryModel.BodyClauses.OfType().ToList(); foreach (var orderby in bodyClauses) { From aecd6a40a7a70670b221f0184d54e13597f20ab7 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Wed, 2 Sep 2020 13:01:43 +0300 Subject: [PATCH 5/7] Fix --- .../Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs index 469c511c0c5..205a2960523 100644 --- a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs +++ b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs @@ -16,14 +16,16 @@ public static void ReWrite(QueryModel queryModel) { var rewriter = new RemoveUnnecessaryBodyOperators(); - if (queryModel.ResultOperators.All( - r => r is ContainsResultOperator || r is AnyResultOperator || r is AllResultOperator)) + 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.OfType().ToList(); - foreach (var orderby in bodyClauses) + var bodyClauses = queryModel.BodyClauses; + for (int i = bodyClauses.Count - 1; i >= 0; i--) { - queryModel.BodyClauses.Remove(orderby); + if (bodyClauses[i] is OrderByClause) + bodyClauses.RemoveAt(i); } } From 4a5a5535c9d3cac8ff04724076c731b9729c385a Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Wed, 2 Sep 2020 23:37:54 +1200 Subject: [PATCH 6/7] Do not do work twice --- src/NHibernate/Linq/GroupBy/PagingRewriter.cs | 3 +-- .../Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs | 11 +++++++---- src/NHibernate/Linq/Visitors/QueryModelVisitor.cs | 3 +++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/NHibernate/Linq/GroupBy/PagingRewriter.cs b/src/NHibernate/Linq/GroupBy/PagingRewriter.cs index 690330eade0..4ba3b05d5f4 100644 --- a/src/NHibernate/Linq/GroupBy/PagingRewriter.cs +++ b/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().Any() && - !(queryModel.ResultOperators.Count == 1 && queryModel.ResultOperators.All(x => x is AnyResultOperator || x is ContainsResultOperator || x is AllResultOperator))) + if (!queryModel.BodyClauses.OfType().Any()) { var orderByClauses = subQueryModel.BodyClauses.OfType(); foreach (var orderByClause in orderByClauses) diff --git a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs index 205a2960523..60aee2f66ef 100644 --- a/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs +++ b/src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs @@ -16,9 +16,14 @@ public static void ReWrite(QueryModel queryModel) { var rewriter = new RemoveUnnecessaryBodyOperators(); + 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)) + 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; @@ -28,8 +33,6 @@ public static void ReWrite(QueryModel queryModel) bodyClauses.RemoveAt(i); } } - - rewriter.VisitQueryModel(queryModel); } public override void VisitResultOperator(ResultOperatorBase resultOperator, QueryModel queryModel, int index) diff --git a/src/NHibernate/Linq/Visitors/QueryModelVisitor.cs b/src/NHibernate/Linq/Visitors/QueryModelVisitor.cs index 040e9b38932..a57a685e5ff 100644 --- a/src/NHibernate/Linq/Visitors/QueryModelVisitor.cs +++ b/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); From f6ad8832407611dc3b84b0de154904295c973f14 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Thu, 3 Sep 2020 01:35:51 +1200 Subject: [PATCH 7/7] Generate Async and remove commented code --- .../Async/Linq/WhereSubqueryTests.cs | 15 +++++++++++++++ src/NHibernate.Test/Linq/WhereSubqueryTests.cs | 3 --- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/WhereSubqueryTests.cs b/src/NHibernate.Test/Async/Linq/WhereSubqueryTests.cs index fcd730b4314..650a300de6f 100644 --- a/src/NHibernate.Test/Async/Linq/WhereSubqueryTests.cs +++ b/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() { diff --git a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs b/src/NHibernate.Test/Linq/WhereSubqueryTests.cs index c0f2cf25d6d..63051c56cca 100644 --- a/src/NHibernate.Test/Linq/WhereSubqueryTests.cs +++ b/src/NHibernate.Test/Linq/WhereSubqueryTests.cs @@ -621,9 +621,6 @@ public void OrdersWithSubquery11() [Test] public void OrdersWithSubquery11A() { - //if (Dialect is MsSqlCeDialect) - // Assert.Ignore("MS SQL CE does not support sorting on a subquery."); - var ordersQuery = db.Orders .OrderByDescending(x => x.OrderLines.Count).ThenBy(x => x.OrderId);