Skip to content

Commit f3e68d3

Browse files
fredericDelaportefredericDelaporte
authored
Do not cache query plan with non-parameterized constants. (#1544)
 * Fixes #1330 (NH-3673) * Obsoletes #866 (NH-2658) * Fixes #1363 (NH-2500)
1 parent 083a928 commit f3e68d3

File tree

9 files changed

+380
-31
lines changed

9 files changed

+380
-31
lines changed

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

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010

1111
using System.Collections.Generic;
1212
using System.Linq;
13+
using System.Reflection;
1314
using NHibernate.DomainModel.Northwind.Entities;
15+
using NHibernate.Engine.Query;
16+
using NHibernate.Linq.Visitors;
17+
using NHibernate.Util;
1418
using NUnit.Framework;
1519
using NHibernate.Linq;
1620

@@ -130,10 +134,10 @@ public async Task ConstantNonCachedInMemberInitExpressionAsync()
130134
public async Task ConstantInNewArrayExpressionAsync()
131135
{
132136
var c1 = await ((from c in db.Categories
133-
select new [] { c.Name, "category1" }).ToListAsync());
137+
select new[] { c.Name, "category1" }).ToListAsync());
134138

135139
var c2 = await ((from c in db.Categories
136-
select new [] { c.Name, "category2" }).ToListAsync());
140+
select new[] { c.Name, "category2" }).ToListAsync());
137141

138142
Assert.That(c1, Has.Count.GreaterThan(0), "c1 Count");
139143
Assert.That(c2, Has.Count.GreaterThan(0), "c2 Count");
@@ -179,7 +183,6 @@ public int GetItemValue(Product p)
179183

180184
// Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip)
181185
[Test]
182-
[Ignore("Not fixed yet")]
183186
public async Task ObjectConstantsAsync()
184187
{
185188
var builder = new InfoBuilder(1);
@@ -200,7 +203,6 @@ private int TestFunc(Product item, int closureValue)
200203

201204
// Adapted from NH-3673
202205
[Test]
203-
[Ignore("Not fixed yet")]
204206
public async Task ConstantsInFuncCallAsync()
205207
{
206208
var closureVariable = 1;
@@ -213,5 +215,59 @@ public async Task ConstantsInFuncCallAsync()
213215
Assert.That(v1, Is.EqualTo(1), "v1");
214216
Assert.That(v2, Is.EqualTo(2), "v2");
215217
}
218+
219+
[Test]
220+
public async Task PlansAreCachedAsync()
221+
{
222+
var queryPlanCacheType = typeof(QueryPlanCache);
223+
224+
var cache = (SoftLimitMRUCache)
225+
queryPlanCacheType
226+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
227+
.GetValue(Sfi.QueryPlanCache);
228+
cache.Clear();
229+
230+
await ((from c in db.Customers
231+
where c.CustomerId == "ALFKI"
232+
select new { c.CustomerId, c.ContactName }).FirstAsync());
233+
Assert.That(
234+
cache,
235+
Has.Count.EqualTo(1),
236+
"First query plan should be cached.");
237+
238+
using (var spy = new LogSpy(queryPlanCacheType))
239+
{
240+
// Should hit plan cache.
241+
await ((from c in db.Customers
242+
where c.CustomerId == "ANATR"
243+
select new { c.CustomerId, c.ContactName }).FirstAsync());
244+
Assert.That(cache, Has.Count.EqualTo(1), "Second query should not cause a plan to be cache.");
245+
Assert.That(
246+
spy.GetWholeLog(),
247+
Does
248+
.Contain("located HQL query plan in cache")
249+
.And.Not.Contain("unable to locate HQL query plan in cache"));
250+
}
251+
}
252+
253+
[Test]
254+
public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync()
255+
{
256+
var queryPlanCacheType = typeof(QueryPlanCache);
257+
258+
var cache = (SoftLimitMRUCache)
259+
queryPlanCacheType
260+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
261+
.GetValue(Sfi.QueryPlanCache);
262+
cache.Clear();
263+
264+
await ((from c in db.Customers
265+
where c.CustomerId == "ALFKI"
266+
select new { c.CustomerId, c.ContactName, Constant = 1 }).FirstAsync());
267+
Assert.That(
268+
cache,
269+
Has.Count.EqualTo(0),
270+
"Query plan should not be cached.");
271+
}
216272
}
217273
}

‎src/NHibernate.Test/Async/NHSpecificTest/NH2658/Fixture.cs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System;
12+
using System.Collections;
13+
using System.Collections.ObjectModel;
14+
using System.Linq;
15+
using System.Linq.Expressions;
16+
using System.Reflection;
17+
using NHibernate.Driver;
18+
using NHibernate.Engine;
19+
using NHibernate.Hql.Ast;
20+
using NHibernate.Linq.Functions;
21+
using NHibernate.Linq.Visitors;
22+
using NHibernate.Util;
23+
using NUnit.Framework;
24+
using NHibernate.Linq;
25+
26+
namespace NHibernate.Test.NHSpecificTest.NH2658
27+
{
28+
using System.Threading.Tasks;
29+
30+
[TestFixture]
31+
public class FixtureAsync : TestCase
32+
{
33+
public class DynamicPropertyGenerator : BaseHqlGeneratorForMethod
34+
{
35+
public DynamicPropertyGenerator()
36+
{
37+
// just registering for string here, but in a real implementation we'd be doing a runtime generator
38+
SupportedMethods = new[]
39+
{
40+
ReflectHelper.GetMethodDefinition(() => ObjectExtensions.GetProperty<string>(null, null))
41+
};
42+
}
43+
44+
public override HqlTreeNode BuildHql(
45+
MethodInfo method,
46+
Expression targetObject,
47+
ReadOnlyCollection<Expression> arguments,
48+
HqlTreeBuilder treeBuilder,
49+
IHqlExpressionVisitor visitor)
50+
{
51+
var propertyName = (string) ((ConstantExpression) arguments[1]).Value;
52+
53+
return treeBuilder.Dot(
54+
visitor.Visit(arguments[0]).AsExpression(),
55+
treeBuilder.Ident(propertyName)).AsExpression();
56+
}
57+
}
58+
59+
protected override string MappingsAssembly => "NHibernate.Test";
60+
61+
protected override IList Mappings => new[] { "NHSpecificTest.NH2658.Mappings.hbm.xml" };
62+
63+
protected override DebugSessionFactory BuildSessionFactory()
64+
{
65+
var sfi = base.BuildSessionFactory();
66+
67+
// add our linq extension
68+
((ISessionFactoryImplementor)sfi).Settings.LinqToHqlGeneratorsRegistry.Merge(new DynamicPropertyGenerator());
69+
return sfi;
70+
}
71+
72+
[Test]
73+
public async Task Does_Not_Cache_NonParametersAsync()
74+
{
75+
using (var session = OpenSession())
76+
{
77+
// Passes
78+
using (var spy = new SqlLogSpy())
79+
{
80+
// Query by name
81+
await ((from p in session.Query<Product>() where p.GetProperty<string>("Name") == "Value" select p).ToListAsync());
82+
83+
var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0);
84+
Assert.That(spy.GetWholeLog(), Does.Contain("Name=" + paramName));
85+
}
86+
87+
// Was failing.
88+
// Because this query was considered the same as the top query the hql will be reused from the top statement
89+
// even though GetProperty has a parameter that never get passed to sql or hql
90+
using (var spy = new SqlLogSpy())
91+
{
92+
// Query by description
93+
await ((from p in session.Query<Product>() where p.GetProperty<string>("Description") == "Value" select p).ToListAsync());
94+
95+
var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0);
96+
Assert.That(spy.GetWholeLog(), Does.Contain("Description=" + paramName));
97+
}
98+
}
99+
}
100+
}
101+
}

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

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
using System.Collections.Generic;
22
using System.Linq;
3+
using System.Reflection;
34
using NHibernate.DomainModel.Northwind.Entities;
5+
using NHibernate.Engine.Query;
6+
using NHibernate.Linq.Visitors;
7+
using NHibernate.Util;
48
using NUnit.Framework;
59

610
namespace NHibernate.Test.Linq
@@ -118,10 +122,10 @@ public void ConstantNonCachedInMemberInitExpression()
118122
public void ConstantInNewArrayExpression()
119123
{
120124
var c1 = (from c in db.Categories
121-
select new [] { c.Name, "category1" }).ToList();
125+
select new[] { c.Name, "category1" }).ToList();
122126

123127
var c2 = (from c in db.Categories
124-
select new [] { c.Name, "category2" }).ToList();
128+
select new[] { c.Name, "category2" }).ToList();
125129

126130
Assert.That(c1, Has.Count.GreaterThan(0), "c1 Count");
127131
Assert.That(c2, Has.Count.GreaterThan(0), "c2 Count");
@@ -167,7 +171,6 @@ public int GetItemValue(Product p)
167171

168172
// Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip)
169173
[Test]
170-
[Ignore("Not fixed yet")]
171174
public void ObjectConstants()
172175
{
173176
var builder = new InfoBuilder(1);
@@ -188,7 +191,6 @@ private int TestFunc(Product item, int closureValue)
188191

189192
// Adapted from NH-3673
190193
[Test]
191-
[Ignore("Not fixed yet")]
192194
public void ConstantsInFuncCall()
193195
{
194196
var closureVariable = 1;
@@ -201,5 +203,78 @@ public void ConstantsInFuncCall()
201203
Assert.That(v1, Is.EqualTo(1), "v1");
202204
Assert.That(v2, Is.EqualTo(2), "v2");
203205
}
206+
207+
[Test]
208+
public void ConstantInWhereDoesNotCauseManyKeys()
209+
{
210+
var q1 = (from c in db.Customers
211+
where c.CustomerId == "ALFKI"
212+
select c);
213+
var q2 = (from c in db.Customers
214+
where c.CustomerId == "ANATR"
215+
select c);
216+
var parameters1 = ExpressionParameterVisitor.Visit(q1.Expression, Sfi);
217+
var k1 = ExpressionKeyVisitor.Visit(q1.Expression, parameters1);
218+
var parameters2 = ExpressionParameterVisitor.Visit(q2.Expression, Sfi);
219+
var k2 = ExpressionKeyVisitor.Visit(q2.Expression, parameters2);
220+
221+
Assert.That(parameters1, Has.Count.GreaterThan(0), "parameters1");
222+
Assert.That(parameters2, Has.Count.GreaterThan(0), "parameters2");
223+
Assert.That(k2, Is.EqualTo(k1));
224+
}
225+
226+
[Test]
227+
public void PlansAreCached()
228+
{
229+
var queryPlanCacheType = typeof(QueryPlanCache);
230+
231+
var cache = (SoftLimitMRUCache)
232+
queryPlanCacheType
233+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
234+
.GetValue(Sfi.QueryPlanCache);
235+
cache.Clear();
236+
237+
(from c in db.Customers
238+
where c.CustomerId == "ALFKI"
239+
select new { c.CustomerId, c.ContactName }).First();
240+
Assert.That(
241+
cache,
242+
Has.Count.EqualTo(1),
243+
"First query plan should be cached.");
244+
245+
using (var spy = new LogSpy(queryPlanCacheType))
246+
{
247+
// Should hit plan cache.
248+
(from c in db.Customers
249+
where c.CustomerId == "ANATR"
250+
select new { c.CustomerId, c.ContactName }).First();
251+
Assert.That(cache, Has.Count.EqualTo(1), "Second query should not cause a plan to be cache.");
252+
Assert.That(
253+
spy.GetWholeLog(),
254+
Does
255+
.Contain("located HQL query plan in cache")
256+
.And.Not.Contain("unable to locate HQL query plan in cache"));
257+
}
258+
}
259+
260+
[Test]
261+
public void PlansWithNonParameterizedConstantsAreNotCached()
262+
{
263+
var queryPlanCacheType = typeof(QueryPlanCache);
264+
265+
var cache = (SoftLimitMRUCache)
266+
queryPlanCacheType
267+
.GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic)
268+
.GetValue(Sfi.QueryPlanCache);
269+
cache.Clear();
270+
271+
(from c in db.Customers
272+
where c.CustomerId == "ALFKI"
273+
select new { c.CustomerId, c.ContactName, Constant = 1 }).First();
274+
Assert.That(
275+
cache,
276+
Has.Count.EqualTo(0),
277+
"Query plan should not be cached.");
278+
}
204279
}
205280
}

‎src/NHibernate.Test/NHSpecificTest/NH2658/Entity.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
namespace NHibernate.Test.NHSpecificTest.NH2658
2+
{
3+
public class Product
4+
{
5+
public virtual string ProductId { get; set; }
6+
7+
public virtual string Name { get; set; }
8+
9+
public virtual string Description { get; set; }
10+
}
11+
}

0 commit comments

Comments
 (0)