Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/NHibernate.Test/Async/Linq/WhereSubqueryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
62 changes: 58 additions & 4 deletions src/NHibernate.Test/Linq/WhereSubqueryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")]
Expand All @@ -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);
Expand All @@ -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")]
Expand All @@ -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()
{
Expand Down
3 changes: 1 addition & 2 deletions src/NHibernate/Linq/GroupBy/PagingRewriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 17 additions & 1 deletion src/NHibernate/Linq/ReWriters/RemoveUnnecessaryBodyOperators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate/Linq/Visitors/QueryModelVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down