Skip to content

Commit 3df5ff8

Browse files
hazzikhazzik
committed
NH-2504 - Fix calling Cacheable before GroupBy
1 parent 76ca563 commit 3df5ff8

File tree

5 files changed

+90
-53
lines changed

5 files changed

+90
-53
lines changed

‎src/NHibernate.Test/Linq/QueryCacheableTests.cs

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -115,35 +115,64 @@ public void CacheableRegionBeforeOtherClauses()
115115
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1));
116116
}
117117

118-
[Test]
119-
public void GroupByQueryIsCacheable()
120-
{
121-
Sfi.Statistics.Clear();
122-
Sfi.QueryCache.Clear();
123-
124-
var c = db
125-
.Customers
126-
.GroupBy(x => x.Address.Country)
127-
.Select(x => x.Key)
128-
.Cacheable()
129-
.ToList();
130-
131-
c = db
132-
.Customers
133-
.GroupBy(x => x.Address.Country)
134-
.Select(x => x.Key)
135-
.ToList();
136-
137-
c = db
138-
.Customers
139-
.GroupBy(x => x.Address.Country)
140-
.Select(x => x.Key)
141-
.Cacheable()
142-
.ToList();
143-
144-
Assert.That(Sfi.Statistics.QueryExecutionCount, Is.EqualTo(2));
145-
Assert.That(Sfi.Statistics.QueryCachePutCount, Is.EqualTo(1));
146-
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1));
147-
}
148-
}
118+
[Test]
119+
public void GroupByQueryIsCacheable()
120+
{
121+
Sfi.Statistics.Clear();
122+
Sfi.QueryCache.Clear();
123+
124+
var c = db
125+
.Customers
126+
.GroupBy(x => x.Address.Country)
127+
.Select(x=>x.Key)
128+
.Cacheable()
129+
.ToList();
130+
131+
c = db
132+
.Customers
133+
.GroupBy(x => x.Address.Country)
134+
.Select(x => x.Key)
135+
.ToList();
136+
137+
c = db
138+
.Customers
139+
.GroupBy(x => x.Address.Country)
140+
.Select(x => x.Key)
141+
.Cacheable()
142+
.ToList();
143+
144+
Assert.That(Sfi.Statistics.QueryExecutionCount, Is.EqualTo(2));
145+
Assert.That(Sfi.Statistics.QueryCachePutCount, Is.EqualTo(1));
146+
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1));
147+
}
148+
149+
[Test]
150+
public void GroupByQueryIsCacheable2()
151+
{
152+
Sfi.Statistics.Clear();
153+
Sfi.QueryCache.Clear();
154+
155+
var c = db
156+
.Customers.Cacheable()
157+
.GroupBy(x => x.Address.Country)
158+
.Select(x => x.Key)
159+
.ToList();
160+
161+
c = db
162+
.Customers
163+
.GroupBy(x => x.Address.Country)
164+
.Select(x => x.Key)
165+
.ToList();
166+
167+
c = db
168+
.Customers.Cacheable()
169+
.GroupBy(x => x.Address.Country)
170+
.Select(x => x.Key)
171+
.ToList();
172+
173+
Assert.That(Sfi.Statistics.QueryExecutionCount, Is.EqualTo(2));
174+
Assert.That(Sfi.Statistics.QueryCachePutCount, Is.EqualTo(1));
175+
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1));
176+
}
177+
}
149178
}

‎src/NHibernate/Linq/GroupBy/AggregatingGroupByRewriter.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Linq;
44
using NHibernate.Linq.Clauses;
5+
using NHibernate.Linq.ReWriters;
56
using NHibernate.Linq.Visitors;
67
using Remotion.Linq;
78
using Remotion.Linq.Clauses.Expressions;
@@ -45,25 +46,32 @@ public static void ReWrite(QueryModel queryModel)
4546
{
4647
var subQueryExpression = queryModel.MainFromClause.FromExpression as SubQueryExpression;
4748

48-
if ((subQueryExpression != null) &&
49-
(subQueryExpression.QueryModel.ResultOperators.Count == 1) &&
50-
(subQueryExpression.QueryModel.ResultOperators[0] is GroupResultOperator))
49+
if (subQueryExpression != null)
5150
{
52-
FlattenSubQuery(queryModel, subQueryExpression.QueryModel);
51+
var operators = subQueryExpression.QueryModel.ResultOperators
52+
.Where(x => !QueryReferenceExpressionFlattener.FlattenableResultOperators.Contains(x.GetType()))
53+
.ToArray();
54+
55+
if (operators.Length == 1)
56+
{
57+
var groupBy = operators[0] as GroupResultOperator;
58+
if (groupBy != null)
59+
{
60+
FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy);
61+
}
62+
}
5363
}
5464
}
5565

56-
private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryModel)
66+
private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryModel, GroupResultOperator groupBy)
5767
{
5868
foreach (var resultOperator in queryModel.ResultOperators.Where(resultOperator => !AcceptableOuterResultOperators.Contains(resultOperator.GetType())))
5969
{
6070
throw new NotImplementedException("Cannot use group by with the " + resultOperator.GetType().Name + " result operator.");
6171
}
6272

6373
// Move the result operator up.
64-
var groupBy = (GroupResultOperator) subQueryModel.ResultOperators[0];
65-
66-
queryModel.ResultOperators.Insert(0, groupBy);
74+
SubQueryFromClauseFlattener.InsertResultOperators(subQueryModel.ResultOperators, queryModel);
6775

6876
for (var i = 0; i < queryModel.BodyClauses.Count; i++)
6977
{

‎src/NHibernate/Linq/NhRelinqQueryParser.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public CacheableExpressionNode(MethodCallExpressionParseInfo parseInfo, Constant
128128

129129
public override Expression Resolve(ParameterExpression inputParameter, Expression expressionToBeResolved, ClauseGenerationContext clauseGenerationContext)
130130
{
131-
throw new NotImplementedException();
131+
return Source.Resolve(inputParameter, expressionToBeResolved, clauseGenerationContext);
132132
}
133133

134134
protected override ResultOperatorBase CreateResultOperator(ClauseGenerationContext clauseGenerationContext)

‎src/NHibernate/Linq/ReWriters/QueryReferenceExpressionFlattener.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ public class QueryReferenceExpressionFlattener : ExpressionTreeVisitor
1212
{
1313
private readonly QueryModel _model;
1414

15-
private static readonly System.Type[] FlattenableResultOperators = new[]
16-
{
17-
typeof(CacheableResultOperator),
18-
typeof (TimeoutResultOperator),
19-
};
15+
internal static readonly System.Type[] FlattenableResultOperators =
16+
{
17+
typeof (CacheableResultOperator),
18+
typeof (TimeoutResultOperator),
19+
};
2020

2121
private QueryReferenceExpressionFlattener(QueryModel model)
2222
{
@@ -33,7 +33,7 @@ protected override Expression VisitSubQueryExpression(SubQueryExpression subQuer
3333
{
3434
var subQueryModel = subQuery.QueryModel;
3535
var hasBodyClauses = subQueryModel.BodyClauses.Count > 0;
36-
if(hasBodyClauses)
36+
if (hasBodyClauses)
3737
{
3838
return base.VisitSubQueryExpression(subQuery);
3939
}

‎src/NHibernate/Linq/Visitors/SubQueryFromClauseFlattener.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
22
using System.Linq;
33
using Remotion.Linq;
44
using Remotion.Linq.Clauses;
5-
using Remotion.Linq.Clauses.ExpressionTreeVisitors;
65
using Remotion.Linq.Clauses.Expressions;
6+
using Remotion.Linq.Clauses.ExpressionTreeVisitors;
77
using Remotion.Linq.EagerFetching;
88

99
namespace NHibernate.Linq.Visitors
1010
{
1111
public class SubQueryFromClauseFlattener : QueryModelVisitorBase
1212
{
13-
private static readonly System.Type[] FlattenableResultOperators = new[]
14-
{
15-
typeof (FetchOneRequest),
16-
typeof (FetchManyRequest),
17-
};
13+
private static readonly System.Type[] FlattenableResultOperators =
14+
{
15+
typeof (FetchOneRequest),
16+
typeof (FetchManyRequest)
17+
};
1818

1919
public static void ReWrite(QueryModel queryModel)
2020
{
@@ -80,7 +80,7 @@ private static void FlattenSubQuery(SubQueryExpression subQueryExpression, FromC
8080
queryModel.TransformExpressions(ex => ReferenceReplacingExpressionTreeVisitor.ReplaceClauseReferences(ex, innerBodyClauseMapping, false));
8181
}
8282

83-
private static void InsertResultOperators(IEnumerable<ResultOperatorBase> resultOperators, QueryModel queryModel)
83+
internal static void InsertResultOperators(IEnumerable<ResultOperatorBase> resultOperators, QueryModel queryModel)
8484
{
8585
var index = 0;
8686
foreach (var bodyClause in resultOperators)

0 commit comments

Comments
 (0)