Skip to content

Commit 01fae9c

Browse files
bahusoidbahusoid
committed
Do not cache ExpandedQueryExpression query plan if it's based on not cacheable query expression
1 parent 1920603 commit 01fae9c

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;
@@ -308,5 +309,62 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync()
308309
Has.Count.EqualTo(0),
309310
"Query plan should not be cached.");
310311
}
312+
313+
[Test]
314+
public async Task PlansWithNonParameterizedConstantsAreNotCachedForExpandedQueryAsync()
315+
{
316+
var queryPlanCacheType = typeof(QueryPlanCache);
317+
318+
var cache = (SoftLimitMRUCache)
319+
queryPlanCacheType
320+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
321+
.GetValue(Sfi.QueryPlanCache);
322+
cache.Clear();
323+
324+
var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
325+
await (db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
326+
c => new {c.CustomerId, c.ContactName, Constant = 1}).FirstAsync());
327+
328+
Assert.That(
329+
cache,
330+
Has.Count.EqualTo(0),
331+
"Query plan should not be cached.");
332+
}
333+
334+
//GH-2298 - Different Update queries - same query cache plan
335+
[Test]
336+
public async Task DmlPlansForExpandedQueryAsync()
337+
{
338+
var queryPlanCacheType = typeof(QueryPlanCache);
339+
340+
var cache = (SoftLimitMRUCache)
341+
queryPlanCacheType
342+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
343+
.GetValue(Sfi.QueryPlanCache);
344+
cache.Clear();
345+
346+
using (session.BeginTransaction())
347+
{
348+
var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList();
349+
await (db.Customers.Where(x => list.Contains(x.CustomerId)).UpdateAsync(
350+
x => new Customer
351+
{
352+
CompanyName = "Constant1"
353+
}));
354+
355+
await (db.Customers.Where(x => list.Contains(x.CustomerId))
356+
.UpdateAsync(
357+
x => new Customer
358+
{
359+
ContactName = "Constant1"
360+
}));
361+
362+
Assert.That(
363+
cache.Count,
364+
//2 original queries + 2 expanded queries are expected in cache
365+
Is.EqualTo(0).Or.EqualTo(4),
366+
"Query plans should either be cached separately or not cached at all.");
367+
}
368+
}
311369
}
312370
}

‎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;
@@ -329,5 +330,62 @@ public void PlansWithNonParameterizedConstantsAreNotCached()
329330
Has.Count.EqualTo(0),
330331
"Query plan should not be cached.");
331332
}
333+
334+
[Test]
335+
public void PlansWithNonParameterizedConstantsAreNotCachedForExpandedQuery()
336+
{
337+
var queryPlanCacheType = typeof(QueryPlanCache);
338+
339+
var cache = (SoftLimitMRUCache)
340+
queryPlanCacheType
341+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
342+
.GetValue(Sfi.QueryPlanCache);
343+
cache.Clear();
344+
345+
var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
346+
db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
347+
c => new {c.CustomerId, c.ContactName, Constant = 1}).First();
348+
349+
Assert.That(
350+
cache,
351+
Has.Count.EqualTo(0),
352+
"Query plan should not be cached.");
353+
}
354+
355+
//GH-2298 - Different Update queries - same query cache plan
356+
[Test]
357+
public void DmlPlansForExpandedQuery()
358+
{
359+
var queryPlanCacheType = typeof(QueryPlanCache);
360+
361+
var cache = (SoftLimitMRUCache)
362+
queryPlanCacheType
363+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
364+
.GetValue(Sfi.QueryPlanCache);
365+
cache.Clear();
366+
367+
using (session.BeginTransaction())
368+
{
369+
var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList();
370+
db.Customers.Where(x => list.Contains(x.CustomerId)).Update(
371+
x => new Customer
372+
{
373+
CompanyName = "Constant1"
374+
});
375+
376+
db.Customers.Where(x => list.Contains(x.CustomerId))
377+
.Update(
378+
x => new Customer
379+
{
380+
ContactName = "Constant1"
381+
});
382+
383+
Assert.That(
384+
cache.Count,
385+
//2 original queries + 2 expanded queries are expected in cache
386+
Is.EqualTo(0).Or.EqualTo(4),
387+
"Query plans should either be cached separately or not cached at all.");
388+
}
389+
}
332390
}
333391
}

‎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
@@ -192,16 +192,18 @@ public override object[] ValueArray()
192192
}
193193
}
194194

195-
internal class ExpandedQueryExpression : IQueryExpression
195+
internal class ExpandedQueryExpression : IQueryExpression, ICacheableQueryExpression
196196
{
197197
private readonly IASTNode _tree;
198+
private ICacheableQueryExpression _cacheableExpression;
198199

199200
public ExpandedQueryExpression(IQueryExpression queryExpression, IASTNode tree, string key)
200201
{
201202
_tree = tree;
202203
Key = key;
203204
Type = queryExpression.Type;
204205
ParameterDescriptors = queryExpression.ParameterDescriptors;
206+
_cacheableExpression = queryExpression as ICacheableQueryExpression;
205207
}
206208

207209
#region IQueryExpression Members
@@ -218,6 +220,8 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter
218220
public IList<NamedParameterDescriptor> ParameterDescriptors { get; private set; }
219221

220222
#endregion
223+
224+
public bool CanCachePlan => _cacheableExpression?.CanCachePlan ?? true;
221225
}
222226

223227
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)