Skip to content

NH-3944 - preliminary work, Nh clauses eliminated. #594

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

NH-3944 - preliminary work, Nh clauses eliminated. #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions src/NHibernate.Test/Linq/ByMethod/GroupByHavingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,23 @@ public void SingleKeyGroupAndCountWithHavingClause()
var hornRow = orderCounts.Single(row => row.CompanyName == "Around the Horn");
Assert.That(hornRow.OrderCount, Is.EqualTo(13));
}

[Test, Explicit("Demonstrate an unsupported case for PagingRewriter")]
public void SingleKeyGroupAndCountWithHavingClausePagingAndOuterWhere()
{
var orderCounts = db.Orders
.GroupBy(o => o.Customer.CompanyName)
.Where(g => g.Count() > 10)
.Select(g => new { CompanyName = g.Key, OrderCount = g.Count() })
.OrderBy(oc => oc.CompanyName)
.Skip(5)
.Take(10)
.Where(oc => oc.CompanyName.Contains("F"))
.ToList();

Assert.That(orderCounts, Has.Count.EqualTo(3));
var frankRow = orderCounts.Single(row => row.CompanyName == "Frankenversand");
Assert.That(frankRow.OrderCount, Is.EqualTo(15));
}
}
}
19 changes: 0 additions & 19 deletions src/NHibernate/Linq/Clauses/NhHavingClause.cs
View file
Open in desktop

This file was deleted.

62 changes: 0 additions & 62 deletions src/NHibernate/Linq/Clauses/NhJoinClause.cs
View file
Open in desktop

This file was deleted.

19 changes: 0 additions & 19 deletions src/NHibernate/Linq/Clauses/NhWithClause.cs
View file
Open in desktop

This file was deleted.

20 changes: 7 additions & 13 deletions src/NHibernate/Linq/GroupBy/AggregatingGroupByRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using NHibernate.Linq.Clauses;
using NHibernate.Linq.ReWriters;
using NHibernate.Linq.Visitors;
using Remotion.Linq;
Expand Down Expand Up @@ -43,29 +42,26 @@ public static class AggregatingGroupByRewriter
typeof (CacheableResultOperator)
};

public static void ReWrite(QueryModel queryModel)
public static void ReWrite(QueryModel queryModel, VisitorParameters parameters)
{
var subQueryExpression = queryModel.MainFromClause.FromExpression as SubQueryExpression;

if (subQueryExpression != null)
if (queryModel.MainFromClause.FromExpression is SubQueryExpression subQueryExpression)
{
var operators = subQueryExpression.QueryModel.ResultOperators
.Where(x => !QueryReferenceExpressionFlattener.FlattenableResultOperators.Contains(x.GetType()))
.ToArray();

if (operators.Length == 1)
{
var groupBy = operators[0] as GroupResultOperator;
if (groupBy != null)
if (operators[0] is GroupResultOperator groupBy)
{
FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy);
FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy, parameters);
RemoveCostantGroupByKeys(queryModel, groupBy);
}
}
}
}

private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryModel, GroupResultOperator groupBy)
private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryModel, GroupResultOperator groupBy, VisitorParameters parameters)
{
foreach (var resultOperator in queryModel.ResultOperators.Where(resultOperator => !AcceptableOuterResultOperators.Contains(resultOperator.GetType())))
{
Expand All @@ -81,11 +77,9 @@ private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryMo
clause.TransformExpressions(s => GroupBySelectClauseRewriter.ReWrite(s, groupBy, subQueryModel));

//all outer where clauses actually are having clauses
var whereClause = clause as WhereClause;
if (whereClause != null)
if (clause is WhereClause whereClause)
{
queryModel.BodyClauses.RemoveAt(i);
queryModel.BodyClauses.Insert(i, new NhHavingClause(whereClause.Predicate));
parameters.AddHavingClause(whereClause);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Juste letting them in the model for ensuring any later expression transformation get applied to them.

They will not be added as HQL where because the query model visitor now check each where clause with the visitor parameters. If they are known as having clauses, they will be added as HQL having clauses instead of where.

}
}

Expand Down
23 changes: 13 additions & 10 deletions src/NHibernate/Linq/GroupBy/PagingRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ namespace NHibernate.Linq.GroupBy
{
internal static class PagingRewriter
{
private static readonly System.Type[] PagingResultOperators = new[]
{
typeof (SkipResultOperator),
typeof (TakeResultOperator),
};
private static readonly System.Type[] PagingResultOperators =
new[]
{
typeof (SkipResultOperator),
typeof (TakeResultOperator),
};

public static void ReWrite(QueryModel queryModel)
{
var subQueryExpression = queryModel.MainFromClause.FromExpression as SubQueryExpression;

if (subQueryExpression != null &&
if (queryModel.MainFromClause.FromExpression is SubQueryExpression subQueryExpression &&
subQueryExpression.QueryModel.ResultOperators.All(x => PagingResultOperators.Contains(x.GetType())))
{
FlattenSubQuery(subQueryExpression, queryModel);
Expand All @@ -28,7 +27,7 @@ public static void ReWrite(QueryModel queryModel)

private static void FlattenSubQuery(SubQueryExpression subQueryExpression, QueryModel queryModel)
{
// we can not flattern subquery if outer query has body clauses.
// we can not flatten subquery if outer query has body clauses.
var subQueryModel = subQueryExpression.QueryModel;
var subQueryMainFromClause = subQueryModel.MainFromClause;
if (queryModel.BodyClauses.Count == 0)
Expand All @@ -46,9 +45,13 @@ private static void FlattenSubQuery(SubQueryExpression subQueryExpression, Query
{
var cro = new ContainsResultOperator(new QuerySourceReferenceExpression(subQueryMainFromClause));

// Cloning may cause having/join/with clauses listed in VisitorParameters to no more be matched.
// Not a problem for now, because those clauses imply a projection, which is not supported
// by the "new WhereClause(new SubQueryExpression(newSubQueryModel))" below. See
// SingleKeyGroupAndCountWithHavingClausePagingAndOuterWhere test by example.
Copy link
Member Author

@fredericDelaporte fredericDelaporte Apr 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion was tested by adding following test to NHibernate.Test.Linq.ByMethod.GroupByHavingTests:

[Test, Explicit("Demonstrate an unsupported case for PagingRewriter")]
public void SingleKeyGroupAndCountWithHavingClausePagingAndOuterWhere()
{
	var orderCounts = db.Orders
		.GroupBy(o => o.Customer.CompanyName)
		.Where(g => g.Count() > 10)
		.Select(g => new { CompanyName = g.Key, OrderCount = g.Count() })
		.OrderBy(oc => oc.CompanyName)
		.Skip(5)
		.Take(10)
		.Where(oc => oc.CompanyName.Contains("F"))
		.ToList();

	Assert.That(orderCounts, Has.Count.EqualTo(3));
	var frankRow = orderCounts.Single(row => row.CompanyName == "Frankenversand");
	Assert.That(frankRow.OrderCount, Is.EqualTo(15));
}

The WhereClause line yields an ArgumentException with message

The items of the input sequence of type
'<>f__AnonymousType6`2[System.String,System.Int32]' are not
compatible with the item expression of type
'System.Linq.IGrouping`2[System.String,NHibernate.DomainModel.Northwind.Entities.Order]'.

Which lets me think this flattening process does not support projection (the Select(g =>), which are needed for introducing having or left join clauses.

I have included that test case in this PR just now.

var newSubQueryModel = subQueryModel.Clone();
newSubQueryModel.ResultOperators.Add(cro);
newSubQueryModel.ResultTypeOverride = typeof (bool);
newSubQueryModel.ResultTypeOverride = typeof(bool);

var where = new WhereClause(new SubQueryExpression(newSubQueryModel));
queryModel.BodyClauses.Add(where);
Expand Down
Loading