diff --git a/src/NHibernate.Test/Async/Linq/ConstantTest.cs b/src/NHibernate.Test/Async/Linq/ConstantTest.cs index b5f0f05037d..718ac40fdc2 100644 --- a/src/NHibernate.Test/Async/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Async/Linq/ConstantTest.cs @@ -10,7 +10,11 @@ using System.Collections.Generic; using System.Linq; +using System.Reflection; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Engine.Query; +using NHibernate.Linq.Visitors; +using NHibernate.Util; using NUnit.Framework; using NHibernate.Linq; @@ -130,10 +134,10 @@ public async Task ConstantNonCachedInMemberInitExpressionAsync() public async Task ConstantInNewArrayExpressionAsync() { var c1 = await ((from c in db.Categories - select new [] { c.Name, "category1" }).ToListAsync()); + select new[] { c.Name, "category1" }).ToListAsync()); var c2 = await ((from c in db.Categories - select new [] { c.Name, "category2" }).ToListAsync()); + select new[] { c.Name, "category2" }).ToListAsync()); Assert.That(c1, Has.Count.GreaterThan(0), "c1 Count"); Assert.That(c2, Has.Count.GreaterThan(0), "c2 Count"); @@ -179,7 +183,6 @@ public int GetItemValue(Product p) // Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip) [Test] - [Ignore("Not fixed yet")] public async Task ObjectConstantsAsync() { var builder = new InfoBuilder(1); @@ -200,7 +203,6 @@ private int TestFunc(Product item, int closureValue) // Adapted from NH-3673 [Test] - [Ignore("Not fixed yet")] public async Task ConstantsInFuncCallAsync() { var closureVariable = 1; @@ -213,5 +215,59 @@ public async Task ConstantsInFuncCallAsync() Assert.That(v1, Is.EqualTo(1), "v1"); Assert.That(v2, Is.EqualTo(2), "v2"); } + + [Test] + public async Task PlansAreCachedAsync() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + await ((from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName }).FirstAsync()); + Assert.That( + cache, + Has.Count.EqualTo(1), + "First query plan should be cached."); + + using (var spy = new LogSpy(queryPlanCacheType)) + { + // Should hit plan cache. + await ((from c in db.Customers + where c.CustomerId == "ANATR" + select new { c.CustomerId, c.ContactName }).FirstAsync()); + Assert.That(cache, Has.Count.EqualTo(1), "Second query should not cause a plan to be cache."); + Assert.That( + spy.GetWholeLog(), + Does + .Contain("located HQL query plan in cache") + .And.Not.Contain("unable to locate HQL query plan in cache")); + } + } + + [Test] + public async Task PlansWithNonParameterizedConstantsAreNotCachedAsync() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + await ((from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, Constant = 1 }).FirstAsync()); + Assert.That( + cache, + Has.Count.EqualTo(0), + "Query plan should not be cached."); + } } } diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH2658/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH2658/Fixture.cs new file mode 100644 index 00000000000..a2af491abc2 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH2658/Fixture.cs @@ -0,0 +1,101 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System; +using System.Collections; +using System.Collections.ObjectModel; +using System.Linq; +using System.Linq.Expressions; +using System.Reflection; +using NHibernate.Driver; +using NHibernate.Engine; +using NHibernate.Hql.Ast; +using NHibernate.Linq.Functions; +using NHibernate.Linq.Visitors; +using NHibernate.Util; +using NUnit.Framework; +using NHibernate.Linq; + +namespace NHibernate.Test.NHSpecificTest.NH2658 +{ + using System.Threading.Tasks; + + [TestFixture] + public class FixtureAsync : TestCase + { + public class DynamicPropertyGenerator : BaseHqlGeneratorForMethod + { + public DynamicPropertyGenerator() + { + // just registering for string here, but in a real implementation we'd be doing a runtime generator + SupportedMethods = new[] + { + ReflectHelper.GetMethodDefinition(() => ObjectExtensions.GetProperty(null, null)) + }; + } + + public override HqlTreeNode BuildHql( + MethodInfo method, + Expression targetObject, + ReadOnlyCollection arguments, + HqlTreeBuilder treeBuilder, + IHqlExpressionVisitor visitor) + { + var propertyName = (string) ((ConstantExpression) arguments[1]).Value; + + return treeBuilder.Dot( + visitor.Visit(arguments[0]).AsExpression(), + treeBuilder.Ident(propertyName)).AsExpression(); + } + } + + protected override string MappingsAssembly => "NHibernate.Test"; + + protected override IList Mappings => new[] { "NHSpecificTest.NH2658.Mappings.hbm.xml" }; + + protected override DebugSessionFactory BuildSessionFactory() + { + var sfi = base.BuildSessionFactory(); + + // add our linq extension + ((ISessionFactoryImplementor)sfi).Settings.LinqToHqlGeneratorsRegistry.Merge(new DynamicPropertyGenerator()); + return sfi; + } + + [Test] + public async Task Does_Not_Cache_NonParametersAsync() + { + using (var session = OpenSession()) + { + // Passes + using (var spy = new SqlLogSpy()) + { + // Query by name + await ((from p in session.Query() where p.GetProperty("Name") == "Value" select p).ToListAsync()); + + var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0); + Assert.That(spy.GetWholeLog(), Does.Contain("Name=" + paramName)); + } + + // Was failing. + // Because this query was considered the same as the top query the hql will be reused from the top statement + // even though GetProperty has a parameter that never get passed to sql or hql + using (var spy = new SqlLogSpy()) + { + // Query by description + await ((from p in session.Query() where p.GetProperty("Description") == "Value" select p).ToListAsync()); + + var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0); + Assert.That(spy.GetWholeLog(), Does.Contain("Description=" + paramName)); + } + } + } + } +} diff --git a/src/NHibernate.Test/Linq/ConstantTest.cs b/src/NHibernate.Test/Linq/ConstantTest.cs index a30118e7283..2b4be96a912 100644 --- a/src/NHibernate.Test/Linq/ConstantTest.cs +++ b/src/NHibernate.Test/Linq/ConstantTest.cs @@ -1,6 +1,10 @@ using System.Collections.Generic; using System.Linq; +using System.Reflection; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Engine.Query; +using NHibernate.Linq.Visitors; +using NHibernate.Util; using NUnit.Framework; namespace NHibernate.Test.Linq @@ -118,10 +122,10 @@ public void ConstantNonCachedInMemberInitExpression() public void ConstantInNewArrayExpression() { var c1 = (from c in db.Categories - select new [] { c.Name, "category1" }).ToList(); + select new[] { c.Name, "category1" }).ToList(); var c2 = (from c in db.Categories - select new [] { c.Name, "category2" }).ToList(); + select new[] { c.Name, "category2" }).ToList(); Assert.That(c1, Has.Count.GreaterThan(0), "c1 Count"); Assert.That(c2, Has.Count.GreaterThan(0), "c2 Count"); @@ -167,7 +171,6 @@ public int GetItemValue(Product p) // Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip) [Test] - [Ignore("Not fixed yet")] public void ObjectConstants() { var builder = new InfoBuilder(1); @@ -188,7 +191,6 @@ private int TestFunc(Product item, int closureValue) // Adapted from NH-3673 [Test] - [Ignore("Not fixed yet")] public void ConstantsInFuncCall() { var closureVariable = 1; @@ -201,5 +203,78 @@ public void ConstantsInFuncCall() Assert.That(v1, Is.EqualTo(1), "v1"); Assert.That(v2, Is.EqualTo(2), "v2"); } + + [Test] + public void ConstantInWhereDoesNotCauseManyKeys() + { + var q1 = (from c in db.Customers + where c.CustomerId == "ALFKI" + select c); + var q2 = (from c in db.Customers + where c.CustomerId == "ANATR" + select c); + var parameters1 = ExpressionParameterVisitor.Visit(q1.Expression, Sfi); + var k1 = ExpressionKeyVisitor.Visit(q1.Expression, parameters1); + var parameters2 = ExpressionParameterVisitor.Visit(q2.Expression, Sfi); + var k2 = ExpressionKeyVisitor.Visit(q2.Expression, parameters2); + + Assert.That(parameters1, Has.Count.GreaterThan(0), "parameters1"); + Assert.That(parameters2, Has.Count.GreaterThan(0), "parameters2"); + Assert.That(k2, Is.EqualTo(k1)); + } + + [Test] + public void PlansAreCached() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + (from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName }).First(); + Assert.That( + cache, + Has.Count.EqualTo(1), + "First query plan should be cached."); + + using (var spy = new LogSpy(queryPlanCacheType)) + { + // Should hit plan cache. + (from c in db.Customers + where c.CustomerId == "ANATR" + select new { c.CustomerId, c.ContactName }).First(); + Assert.That(cache, Has.Count.EqualTo(1), "Second query should not cause a plan to be cache."); + Assert.That( + spy.GetWholeLog(), + Does + .Contain("located HQL query plan in cache") + .And.Not.Contain("unable to locate HQL query plan in cache")); + } + } + + [Test] + public void PlansWithNonParameterizedConstantsAreNotCached() + { + var queryPlanCacheType = typeof(QueryPlanCache); + + var cache = (SoftLimitMRUCache) + queryPlanCacheType + .GetField("planCache", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(Sfi.QueryPlanCache); + cache.Clear(); + + (from c in db.Customers + where c.CustomerId == "ALFKI" + select new { c.CustomerId, c.ContactName, Constant = 1 }).First(); + Assert.That( + cache, + Has.Count.EqualTo(0), + "Query plan should not be cached."); + } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH2658/Entity.cs b/src/NHibernate.Test/NHSpecificTest/NH2658/Entity.cs new file mode 100644 index 00000000000..3a80f5fff8a --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH2658/Entity.cs @@ -0,0 +1,11 @@ +namespace NHibernate.Test.NHSpecificTest.NH2658 +{ + public class Product + { + public virtual string ProductId { get; set; } + + public virtual string Name { get; set; } + + public virtual string Description { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH2658/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH2658/Fixture.cs new file mode 100644 index 00000000000..2c0acacf9b3 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH2658/Fixture.cs @@ -0,0 +1,97 @@ +using System; +using System.Collections; +using System.Collections.ObjectModel; +using System.Linq; +using System.Linq.Expressions; +using System.Reflection; +using NHibernate.Driver; +using NHibernate.Engine; +using NHibernate.Hql.Ast; +using NHibernate.Linq.Functions; +using NHibernate.Linq.Visitors; +using NHibernate.Util; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH2658 +{ + public static class ObjectExtensions + { + public static T GetProperty(this object o, string propertyName) + { + // no implementation needed for this test + throw new NotImplementedException(); + } + } + + [TestFixture] + public class Fixture : TestCase + { + public class DynamicPropertyGenerator : BaseHqlGeneratorForMethod + { + public DynamicPropertyGenerator() + { + // just registering for string here, but in a real implementation we'd be doing a runtime generator + SupportedMethods = new[] + { + ReflectHelper.GetMethodDefinition(() => ObjectExtensions.GetProperty(null, null)) + }; + } + + public override HqlTreeNode BuildHql( + MethodInfo method, + Expression targetObject, + ReadOnlyCollection arguments, + HqlTreeBuilder treeBuilder, + IHqlExpressionVisitor visitor) + { + var propertyName = (string) ((ConstantExpression) arguments[1]).Value; + + return treeBuilder.Dot( + visitor.Visit(arguments[0]).AsExpression(), + treeBuilder.Ident(propertyName)).AsExpression(); + } + } + + protected override string MappingsAssembly => "NHibernate.Test"; + + protected override IList Mappings => new[] { "NHSpecificTest.NH2658.Mappings.hbm.xml" }; + + protected override DebugSessionFactory BuildSessionFactory() + { + var sfi = base.BuildSessionFactory(); + + // add our linq extension + ((ISessionFactoryImplementor)sfi).Settings.LinqToHqlGeneratorsRegistry.Merge(new DynamicPropertyGenerator()); + return sfi; + } + + [Test] + public void Does_Not_Cache_NonParameters() + { + using (var session = OpenSession()) + { + // Passes + using (var spy = new SqlLogSpy()) + { + // Query by name + (from p in session.Query() where p.GetProperty("Name") == "Value" select p).ToList(); + + var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0); + Assert.That(spy.GetWholeLog(), Does.Contain("Name=" + paramName)); + } + + // Was failing. + // Because this query was considered the same as the top query the hql will be reused from the top statement + // even though GetProperty has a parameter that never get passed to sql or hql + using (var spy = new SqlLogSpy()) + { + // Query by description + (from p in session.Query() where p.GetProperty("Description") == "Value" select p).ToList(); + + var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0); + Assert.That(spy.GetWholeLog(), Does.Contain("Description=" + paramName)); + } + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH2658/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH2658/Mappings.hbm.xml new file mode 100644 index 00000000000..8e4c8736769 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH2658/Mappings.hbm.xml @@ -0,0 +1,11 @@ + + + + + + + + + + diff --git a/src/NHibernate/Engine/Query/QueryPlanCache.cs b/src/NHibernate/Engine/Query/QueryPlanCache.cs index 09556fa5a64..9b817c5eb87 100644 --- a/src/NHibernate/Engine/Query/QueryPlanCache.cs +++ b/src/NHibernate/Engine/Query/QueryPlanCache.cs @@ -59,7 +59,11 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo log.Debug("unable to locate HQL query plan in cache; generating ({0})", queryExpression.Key); } plan = new QueryExpressionPlan(queryExpression, shallow, enabledFilters, factory); - planCache.Put(key, plan); + // 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface + if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan) + planCache.Put(key, plan); + else + log.Debug("Query plan not cacheable"); } else { @@ -110,7 +114,11 @@ 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); - planCache.Put(key, plan); + // 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface + if (!(queryExpression is NhLinqExpression linqExpression) || linqExpression.CanCachePlan) + planCache.Put(key, plan); + else + log.Debug("Query plan not cacheable"); } else { diff --git a/src/NHibernate/Linq/NhLinqExpression.cs b/src/NHibernate/Linq/NhLinqExpression.cs index fbca270693f..5b9f20fccbe 100644 --- a/src/NHibernate/Linq/NhLinqExpression.cs +++ b/src/NHibernate/Linq/NhLinqExpression.cs @@ -15,6 +15,8 @@ public class NhLinqExpression : IQueryExpression { public string Key { get; protected set; } + public bool CanCachePlan { get; private set; } = true; + public System.Type Type { get; private set; } /// @@ -78,6 +80,14 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter ParameterDescriptors = requiredHqlParameters.AsReadOnly(); + CanCachePlan = CanCachePlan && + // If some constants do not have matching HQL parameters, their values from first query will + // be embedded in the plan and reused for subsequent queries: do not cache the plan. + !ParameterValuesByName + .Keys + .Except(requiredHqlParameters.Select(p => p.Name)) + .Any(); + return ExpressionToHqlTranslationResults.Statement.AstNode; } diff --git a/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs b/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs index 1789357cfb0..540a5105494 100644 --- a/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs +++ b/src/NHibernate/Linq/Visitors/ExpressionKeyVisitor.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -83,7 +82,7 @@ protected override Expression VisitConstant(ConstantExpression expression) if (_constantToParameterMap == null) throw new InvalidOperationException("Cannot visit a constant without a constant to parameter map."); - if (_constantToParameterMap.TryGetValue(expression, out param) && insideSelectClause == false) + if (_constantToParameterMap.TryGetValue(expression, out param)) { // Nulls generate different query plans. X = variable generates a different query depending on if variable is null or not. if (param.Value == null) @@ -158,26 +157,8 @@ protected override Expression VisitMember(MemberExpression expression) return expression; } - private bool insideSelectClause; protected override Expression VisitMethodCall(MethodCallExpression expression) { - var old = insideSelectClause; - - switch (expression.Method.Name) - { - case "First": - case "FirstOrDefault": - case "Single": - case "SingleOrDefault": - case "Select": - case "GroupBy": - insideSelectClause = true; - break; - default: - insideSelectClause = false; - break; - } - Visit(expression.Object); _string.Append('.'); VisitMethod(expression.Method); @@ -185,7 +166,6 @@ protected override Expression VisitMethodCall(MethodCallExpression expression) ExpressionVisitor.Visit(expression.Arguments, AppendCommas); _string.Append(')'); - insideSelectClause = old; return expression; }