Skip to content

Commit 2abf75b

Browse files
bahusoidbahusoid
committed
Do not cache ExpandedQueryExpression query plan if it's based on not cacheable query expression
1 parent f7b64c5 commit 2abf75b

File tree

6 files changed

+131
-5
lines changed

6 files changed

+131
-5
lines changed

‎src/NHibernate.Test/Async/Linq/ConstantTest.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Collections.Generic;
1212
using System.Linq;
1313
using System.Reflection;
14+
using NHibernate.Criterion;
1415
using NHibernate.DomainModel.Northwind.Entities;
1516
using NHibernate.Engine.Query;
1617
using NHibernate.Linq.Visitors;
@@ -269,5 +270,62 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync()
269270
Has.Count.EqualTo(0),
270271
"Query plan should not be cached.");
271272
}
273+
274+
[Test]
275+
public async Task PlansWithNonParameterizedConstantsAreNotCachedForExpandedQueryAsync()
276+
{
277+
var queryPlanCacheType = typeof(QueryPlanCache);
278+
279+
var cache = (SoftLimitMRUCache)
280+
queryPlanCacheType
281+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
282+
.GetValue(Sfi.QueryPlanCache);
283+
cache.Clear();
284+
285+
var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
286+
await (db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
287+
c => new {c.CustomerId, c.ContactName, Constant = 1}).FirstAsync());
288+
289+
Assert.That(
290+
cache,
291+
Has.Count.EqualTo(0),
292+
"Query plan should not be cached.");
293+
}
294+
295+
//GH-2298 - Different Update queries - same query cache plan
296+
[Test]
297+
public async Task DmlPlansForExpandedQueryAsync()
298+
{
299+
var queryPlanCacheType = typeof(QueryPlanCache);
300+
301+
var cache = (SoftLimitMRUCache)
302+
queryPlanCacheType
303+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
304+
.GetValue(Sfi.QueryPlanCache);
305+
cache.Clear();
306+
307+
using (session.BeginTransaction())
308+
{
309+
var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList();
310+
await (db.Customers.Where(x => list.Contains(x.CustomerId)).UpdateAsync(
311+
x => new Customer
312+
{
313+
CompanyName = "Constant1"
314+
}));
315+
316+
await (db.Customers.Where(x => list.Contains(x.CustomerId))
317+
.UpdateAsync(
318+
x => new Customer
319+
{
320+
ContactName = "Constant1"
321+
}));
322+
323+
Assert.That(
324+
cache.Count,
325+
//2 original queries + 2 expanded queries are expected in cache
326+
Is.EqualTo(0).Or.EqualTo(4),
327+
"Query plans should either be cached separately or not cached at all.");
328+
}
329+
}
272330
}
273331
}

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections.Generic;
22
using System.Linq;
33
using System.Reflection;
4+
using NHibernate.Criterion;
45
using NHibernate.DomainModel.Northwind.Entities;
56
using NHibernate.Engine.Query;
67
using NHibernate.Linq.Visitors;
@@ -276,5 +277,62 @@ public void PlansWithNonParameterizedConstantsAreNotCached()
276277
Has.Count.EqualTo(0),
277278
"Query plan should not be cached.");
278279
}
280+
281+
[Test]
282+
public void PlansWithNonParameterizedConstantsAreNotCachedForExpandedQuery()
283+
{
284+
var queryPlanCacheType = typeof(QueryPlanCache);
285+
286+
var cache = (SoftLimitMRUCache)
287+
queryPlanCacheType
288+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
289+
.GetValue(Sfi.QueryPlanCache);
290+
cache.Clear();
291+
292+
var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
293+
db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
294+
c => new {c.CustomerId, c.ContactName, Constant = 1}).First();
295+
296+
Assert.That(
297+
cache,
298+
Has.Count.EqualTo(0),
299+
"Query plan should not be cached.");
300+
}
301+
302+
//GH-2298 - Different Update queries - same query cache plan
303+
[Test]
304+
public void DmlPlansForExpandedQuery()
305+
{
306+
var queryPlanCacheType = typeof(QueryPlanCache);
307+
308+
var cache = (SoftLimitMRUCache)
309+
queryPlanCacheType
310+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
311+
.GetValue(Sfi.QueryPlanCache);
312+
cache.Clear();
313+
314+
using (session.BeginTransaction())
315+
{
316+
var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList();
317+
db.Customers.Where(x => list.Contains(x.CustomerId)).Update(
318+
x => new Customer
319+
{
320+
CompanyName = "Constant1"
321+
});
322+
323+
db.Customers.Where(x => list.Contains(x.CustomerId))
324+
.Update(
325+
x => new Customer
326+
{
327+
ContactName = "Constant1"
328+
});
329+
330+
Assert.That(
331+
cache.Count,
332+
//2 original queries + 2 expanded queries are expected in cache
333+
Is.EqualTo(0).Or.EqualTo(4),
334+
"Query plans should either be cached separately or not cached at all.");
335+
}
336+
}
279337
}
280338
}

‎src/NHibernate/Engine/Query/QueryPlanCache.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo
6262
}
6363
plan = new QueryExpressionPlan(queryExpression, shallow, enabledFilters, factory);
6464
// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
65-
if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
65+
if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
6666
planCache.Put(key, plan);
6767
else
6868
log.Debug("Query plan not cacheable");
@@ -117,7 +117,7 @@ public IQueryExpressionPlan GetFilterQueryPlan(IQueryExpression queryExpression,
117117
log.Debug("unable to locate collection-filter query plan in cache; generating ({0} : {1})", collectionRole, queryExpression.Key);
118118
plan = new FilterQueryPlan(queryExpression, collectionRole, shallow, enabledFilters, factory);
119119
// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
120-
if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
120+
if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
121121
planCache.Put(key, plan);
122122
else
123123
log.Debug("Query plan not cacheable");

‎src/NHibernate/IQueryExpression.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,17 @@
55

66
namespace NHibernate
77
{
8+
//TODO 6.0: Merge into IQueryExpression
9+
internal interface ICacheableQueryExpression
10+
{
11+
bool CanCachePlan { get; }
12+
}
13+
814
public interface IQueryExpression
915
{
1016
IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter);
1117
string Key { get; }
1218
System.Type Type { get; }
1319
IList<NamedParameterDescriptor> ParameterDescriptors { get; }
1420
}
15-
}
21+
}

‎src/NHibernate/Impl/ExpressionQueryImpl.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,18 @@ public override object[] ValueArray()
150150
}
151151
}
152152

153-
internal class ExpandedQueryExpression : IQueryExpression
153+
internal class ExpandedQueryExpression : IQueryExpression, ICacheableQueryExpression
154154
{
155155
private readonly IASTNode _tree;
156+
private ICacheableQueryExpression _cacheableExpression;
156157

157158
public ExpandedQueryExpression(IQueryExpression queryExpression, IASTNode tree, string key)
158159
{
159160
_tree = tree;
160161
Key = key;
161162
Type = queryExpression.Type;
162163
ParameterDescriptors = queryExpression.ParameterDescriptors;
164+
_cacheableExpression = queryExpression as ICacheableQueryExpression;
163165
}
164166

165167
#region IQueryExpression Members
@@ -176,6 +178,8 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter
176178
public IList<NamedParameterDescriptor> ParameterDescriptors { get; private set; }
177179

178180
#endregion
181+
182+
public bool CanCachePlan => _cacheableExpression?.CanCachePlan ?? true;
179183
}
180184

181185
internal class ParameterExpander

‎src/NHibernate/Linq/NhLinqExpression.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace NHibernate.Linq
1313
{
14-
public class NhLinqExpression : IQueryExpression
14+
public class NhLinqExpression : IQueryExpression, ICacheableQueryExpression
1515
{
1616
public string Key { get; protected set; }
1717

0 commit comments

Comments
 (0)