Avoid re-using query plan embedding different constant values.

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");
@@ -175,13 +179,19 @@ public int GetItemValue(Product p)
 			{
 				return _value;
 			}
+
+			// Workaround for having a different key per different instances.
+			public override string ToString()
+			{
+				return base.ToString() + _value;
+			}
 		}
 
 		// Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip)
 		[Test]
-		[Ignore("Not fixed yet")]
 		public async Task ObjectConstantsAsync()
 		{
+			// Fixed with a workaround, see InfoBuilder above.
 			var builder = new InfoBuilder(1);
 			var v1 = await ((from p in db.Products
 			          select builder.GetItemValue(p)).FirstAsync());
@@ -200,7 +210,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 +222,64 @@ 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, Constant = 1 }).FirstAsync());
+			Assert.That(
+				cache,
+				Has.Count.EqualTo(2),
+				"First query plan should be cached with a non-refined key and a refined one.");
+
+			using (var spy = new LogSpy(queryPlanCacheType))
+			{
+				// Should hit non-refined key but miss refined key.
+				await ((from c in db.Customers
+				 where c.CustomerId == "ANATR"
+				 select new { c.CustomerId, c.ContactName, Constant = 2 }).FirstAsync());
+				Assert.That(cache, Has.Count.EqualTo(3), "Second query plan should be cached only with its refined key.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Contain("Key was refined and is no more matching")
+						.And.Contain("unable to locate HQL query plan in cache"));
+
+				spy.Appender.Clear();
+				// Should hit non-refined key entry directly.
+				await ((from c in db.Customers
+				 where c.CustomerId == "ANATR"
+				 select new { c.CustomerId, c.ContactName, Constant = 1 }).FirstAsync());
+				Assert.That(cache, Has.Count.EqualTo(3), "Third query plan should not be additionnaly cached.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Not.Contain("Key was refined and is no more matching"));
+
+				spy.Appender.Clear();
+				// Should hit non-refined key then hit refined key.
+				await ((from c in db.Customers
+				 where c.CustomerId == "ALFKI"
+				 select new { c.CustomerId, c.ContactName, Constant = 2 }).FirstAsync());
+				Assert.That(cache, Has.Count.EqualTo(3), "Fourth query plan should not be additionnaly cached.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Contain("Key was refined and is no more matching")
+						.And.Not.Contain("unable to locate HQL query plan in cache"));
+			}
+		}
 	}
 }

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));
+				}
+
+				//FAILS
+				//Because this query is 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");
@@ -163,13 +167,19 @@ public int GetItemValue(Product p)
 			{
 				return _value;
 			}
+
+			// Workaround for having a different key per different instances.
+			public override string ToString()
+			{
+				return base.ToString() + _value;
+			}
 		}
 
 		// Adapted from NH-2500 first test case by Andrey Titov (file NHTest3.zip)
 		[Test]
-		[Ignore("Not fixed yet")]
 		public void ObjectConstants()
 		{
+			// Fixed with a workaround, see InfoBuilder above.
 			var builder = new InfoBuilder(1);
 			var v1 = (from p in db.Products
 			          select builder.GetItemValue(p)).First();
@@ -188,7 +198,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 +210,83 @@ 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, Constant = 1 }).First();
+			Assert.That(
+				cache,
+				Has.Count.EqualTo(2),
+				"First query plan should be cached with a non-refined key and a refined one.");
+
+			using (var spy = new LogSpy(queryPlanCacheType))
+			{
+				// Should hit non-refined key but miss refined key.
+				(from c in db.Customers
+				 where c.CustomerId == "ANATR"
+				 select new { c.CustomerId, c.ContactName, Constant = 2 }).First();
+				Assert.That(cache, Has.Count.EqualTo(3), "Second query plan should be cached only with its refined key.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Contain("Key was refined and is no more matching")
+						.And.Contain("unable to locate HQL query plan in cache"));
+
+				spy.Appender.Clear();
+				// Should hit non-refined key entry directly.
+				(from c in db.Customers
+				 where c.CustomerId == "ANATR"
+				 select new { c.CustomerId, c.ContactName, Constant = 1 }).First();
+				Assert.That(cache, Has.Count.EqualTo(3), "Third query plan should not be additionnaly cached.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Not.Contain("Key was refined and is no more matching"));
+
+				spy.Appender.Clear();
+				// Should hit non-refined key then hit refined key.
+				(from c in db.Customers
+				 where c.CustomerId == "ALFKI"
+				 select new { c.CustomerId, c.ContactName, Constant = 2 }).First();
+				Assert.That(cache, Has.Count.EqualTo(3), "Fourth query plan should not be additionnaly cached.");
+				Assert.That(
+					spy.GetWholeLog(),
+					Does
+						.Contain("located HQL query plan in cache")
+						.And.Contain("Key was refined and is no more matching")
+						.And.Not.Contain("unable to locate HQL query plan in cache"));
+			}
+		}
 	}
 }

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