Do not cache query plan with non-parameterized constants.

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.");
+		}
 	}
 }

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

@@ -0,0 +1,101 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by AsyncGenerator.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+
+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<string>(null, null))
+				};
+			}
+
+			public override HqlTreeNode BuildHql(
+				MethodInfo method,
+				Expression targetObject,
+				ReadOnlyCollection<Expression> 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<Product>() where p.GetProperty<string>("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<Product>() where p.GetProperty<string>("Description") == "Value" select p).ToListAsync());
+
+					var paramName = ((ISqlParameterFormatter) Sfi.ConnectionProvider.Driver).GetParameterName(0);
+					Assert.That(spy.GetWholeLog(), Does.Contain("Description=" + paramName));
+				}
+			}
+		}
+	}
+}

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.");
+		}
 	}
 }

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; }
+	}
+}