Skip to content

Do not cache ExpandedQueryExpression query plan if it's based on not cacheable query expression #2300

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 1 commit into from
Jan 14, 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
24 changes: 23 additions & 1 deletion src/NHibernate.Test/Async/Linq/ConstantTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using NHibernate.Criterion;
using NHibernate.DomainModel.Northwind.Entities;
using NHibernate.Engine.Query;
using NHibernate.Linq;
using NHibernate.Linq.Visitors;
using NHibernate.Util;
using NUnit.Framework;
using NHibernate.Linq;

namespace NHibernate.Test.Linq
{
Expand Down Expand Up @@ -269,5 +270,26 @@ public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync()
Has.Count.EqualTo(0),
"Query plan should not be cached.");
}

[Test]
public async Task PlansWithNonParameterizedConstantsAreNotCachedForExpandedQueryAsync()
{
var queryPlanCacheType = typeof(QueryPlanCache);

var cache = (SoftLimitMRUCache)
queryPlanCacheType
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(Sfi.QueryPlanCache);
cache.Clear();

var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
await (db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
c => new {c.CustomerId, c.ContactName, Constant = 1}).FirstAsync());

Assert.That(
cache,
Has.Count.EqualTo(0),
"Query plan should not be cached.");
}
}
}
59 changes: 59 additions & 0 deletions src/NHibernate.Test/Linq/ConstantTest.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using NHibernate.Criterion;
using NHibernate.DomainModel.Northwind.Entities;
using NHibernate.Engine.Query;
using NHibernate.Linq;
using NHibernate.Linq.Visitors;
using NHibernate.Util;
using NUnit.Framework;
Expand Down Expand Up @@ -276,5 +278,62 @@ public void PlansWithNonParameterizedConstantsAreNotCached()
Has.Count.EqualTo(0),
"Query plan should not be cached.");
}

[Test]
public void PlansWithNonParameterizedConstantsAreNotCachedForExpandedQuery()
{
var queryPlanCacheType = typeof(QueryPlanCache);

var cache = (SoftLimitMRUCache)
queryPlanCacheType
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(Sfi.QueryPlanCache);
cache.Clear();

var ids = new[] {"ANATR", "UNKNOWN"}.ToList();
db.Customers.Where(x => ids.Contains(x.CustomerId)).Select(
c => new {c.CustomerId, c.ContactName, Constant = 1}).First();

Assert.That(
cache,
Has.Count.EqualTo(0),
"Query plan should not be cached.");
}

//GH-2298 - Different Update queries - same query cache plan
[Test]
public void DmlPlansForExpandedQuery()
{
var queryPlanCacheType = typeof(QueryPlanCache);

var cache = (SoftLimitMRUCache)
queryPlanCacheType
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(Sfi.QueryPlanCache);
cache.Clear();

using (session.BeginTransaction())
{
var list = new[] {"UNKNOWN", "UNKNOWN2"}.ToList();
db.Customers.Where(x => list.Contains(x.CustomerId)).Update(
x => new Customer
{
CompanyName = "Constant1"
});

db.Customers.Where(x => list.Contains(x.CustomerId))
.Update(
x => new Customer
{
ContactName = "Constant1"
});

Assert.That(
cache.Count,
//2 original queries + 2 expanded queries are expected in cache
Is.EqualTo(0).Or.EqualTo(4),
"Query plans should either be cached separately or not cached at all.");
}
}
}
}
4 changes: 2 additions & 2 deletions src/NHibernate/Engine/Query/QueryPlanCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo
}
plan = new QueryExpressionPlan(queryExpression, shallow, enabledFilters, factory);
// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
planCache.Put(key, plan);
else
log.Debug("Query plan not cacheable");
Expand Down Expand Up @@ -117,7 +117,7 @@ public IQueryExpressionPlan GetFilterQueryPlan(IQueryExpression queryExpression,
log.Debug("unable to locate collection-filter query plan in cache; generating ({0} : {1})", collectionRole, queryExpression.Key);
plan = new FilterQueryPlan(queryExpression, collectionRole, shallow, enabledFilters, factory);
// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan)
if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
planCache.Put(key, plan);
else
log.Debug("Query plan not cacheable");
Expand Down
8 changes: 7 additions & 1 deletion src/NHibernate/IQueryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@

namespace NHibernate
{
//TODO 6.0: Merge into IQueryExpression
internal interface ICacheableQueryExpression
{
bool CanCachePlan { get; }
}

public interface IQueryExpression
{
IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter);
string Key { get; }
System.Type Type { get; }
IList<NamedParameterDescriptor> ParameterDescriptors { get; }
}
}
}
6 changes: 5 additions & 1 deletion src/NHibernate/Impl/ExpressionQueryImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,18 @@ public override object[] ValueArray()
}
}

internal class ExpandedQueryExpression : IQueryExpression
internal class ExpandedQueryExpression : IQueryExpression, ICacheableQueryExpression
{
private readonly IASTNode _tree;
private ICacheableQueryExpression _cacheableExpression;

public ExpandedQueryExpression(IQueryExpression queryExpression, IASTNode tree, string key)
{
_tree = tree;
Key = key;
Type = queryExpression.Type;
ParameterDescriptors = queryExpression.ParameterDescriptors;
_cacheableExpression = queryExpression as ICacheableQueryExpression;
}

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

#endregion

public bool CanCachePlan => _cacheableExpression?.CanCachePlan ?? true;
}

internal class ParameterExpander
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Linq/NhLinqExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace NHibernate.Linq
{
public class NhLinqExpression : IQueryExpression
public class NhLinqExpression : IQueryExpression, ICacheableQueryExpression
{
public string Key { get; protected set; }

Expand Down