Cache only required parts of NhLinqExpression in QueryExpressionPlan

Author: hazzik

src/NHibernate.Test/NHSpecificTest/GH3030/ByCodeFixture.cs

@@ -12,11 +12,12 @@ public class ByCodeFixture : TestCaseMappingByCode
 		protected override HbmMapping GetMappings()
 		{
 			var mapper = new ModelMapper();
-			mapper.Class<Entity>(rc =>
-			{
-				rc.Table("Entity");
-				rc.Id(x => x.Id, m => m.Generator(Generators.Assigned));
-			});
+			mapper.Class<Entity>(
+				rc =>
+				{
+					rc.Table("Entity");
+					rc.Id(x => x.Id, m => m.Generator(Generators.Assigned));
+				});
 
 			return mapper.CompileMappingForAllExplicitlyAddedEntities();
 		}
@@ -35,7 +36,7 @@ protected override void OnTearDown()
 		}
 
 		[Test]
-		public void LinqLeaksMemory()
+		public void LinqShouldNotLeakEntityParameters()
 		{
 			WeakReference sessionReference = null;
 			WeakReference firstReference = null;
@@ -64,8 +65,8 @@ public void LinqLeaksMemory()
 			Assert.That(firstReference.Target, Is.Null);
 			Assert.That(secondReference.Target, Is.Null);
 		}
-		
-		public class Entity 
+
+		public class Entity
 		{
 			public virtual int Id { get; set; }
 		}

src/NHibernate/Engine/Query/QueryExpressionPlan.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using NHibernate.Hql;
-using NHibernate.Linq;
 
 namespace NHibernate.Engine.Query
 {

src/NHibernate/Engine/Query/QueryPlanCache.cs

@@ -66,10 +66,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);
 				// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
 				if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
-					planCache.Put(key, plan);
+					planCache.Put(key, PreparePlanToCache(plan));
 				else
 					log.Debug("Query plan not cacheable");
 			}
@@ -85,23 +86,30 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo
 			return plan;
 		}
 
+		private QueryExpressionPlan PreparePlanToCache(QueryExpressionPlan plan)
+		{
+			if (plan.QueryExpression is NhLinqExpression planExpression)
+			{
+				return plan.Copy(new NhLinqExpressionCache(planExpression));
+			}
+
+			return plan;
+		}
+
 		private static QueryExpressionPlan CopyIfRequired(QueryExpressionPlan plan, IQueryExpression queryExpression)
 		{
-			var planExpression = plan.QueryExpression as NhLinqExpression;
-			var expression = queryExpression as NhLinqExpression;
-			if (planExpression != null && expression != null)
+			if (plan.QueryExpression is NhLinqExpressionCache cache && queryExpression is NhLinqExpression expression)
 			{
 				//NH-3413
 				//Here we have to use original expression.
 				//In most cases NH do not translate expression in second time, but 
 				// for cases when we have list parameters in query, like @p1.Contains(...),
 				// it does, and then it uses parameters from first try. 
-				//TODO: cache only required parts of QueryExpression
 
 				//NH-3436
 				// We have to return new instance plan with it's own query expression
-				// because other treads can override queryexpression of current plan during execution of query if we will use cached instance of plan 
-				expression.CopyExpressionTranslation(planExpression);
+				// because other treads can override query expression of current plan during execution of query if we will use cached instance of plan 
+				expression.CopyExpressionTranslation(cache);
 				plan = plan.Copy(expression);
 			}
 
@@ -124,7 +132,7 @@ public IQueryExpressionPlan GetFilterQueryPlan(IQueryExpression queryExpression,
 				plan = new FilterQueryPlan(queryExpression, collectionRole, shallow, enabledFilters, factory);
 				// 6.0 TODO: add "CanCachePlan { get; }" to IQueryExpression interface
 				if (!(queryExpression is ICacheableQueryExpression linqExpression) || linqExpression.CanCachePlan)
-					planCache.Put(key, plan);
+					planCache.Put(key, PreparePlanToCache(plan));
 				else
 					log.Debug("Query plan not cacheable");
 			}

src/NHibernate/Hql/Ast/ANTLR/ASTQueryTranslatorFactory.cs

@@ -33,7 +33,7 @@ static IQueryTranslator[] CreateQueryTranslators(
 
 			var translators = polymorphicParsers
 				.ToArray(hql => queryExpression is NhLinqExpression linqExpression
-							? new QueryTranslatorImpl(queryIdentifier, hql, filters, factory, linqExpression.NamedParameters)
+							? new QueryTranslatorImpl(queryIdentifier, hql, filters, factory, linqExpression.GetNamedParameterTypes())
 							: new QueryTranslatorImpl(queryIdentifier, hql, filters, factory));
 
 			foreach (var translator in translators)

src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs

@@ -1086,17 +1086,17 @@ IASTNode GenerateNamedParameter(IASTNode delimiterNode, IASTNode nameNode)
 			);
 
 			parameter.HqlParameterSpecification = paramSpec;
-			if (_qti.TryGetNamedParameter(name, out var namedParameter))
+			if (_qti.TryGetNamedParameterType(name, out var type, out var isGuessedType))
 			{
 				// Add the parameter type information so that we are able to calculate functions return types
 				// when the parameter is used as an argument.
-				if (namedParameter.IsGuessedType)
+				if (isGuessedType)
 				{
-					_guessedParameterTypes[paramSpec] = namedParameter.Type;
-					parameter.GuessedType = namedParameter.Type;
+					_guessedParameterTypes[paramSpec] = type;
+					parameter.GuessedType = type;
 				}
 				else
-					parameter.ExpectedType = namedParameter.Type;
+					parameter.ExpectedType = type;
 			}
 
 			_parameters.Add(paramSpec);

src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs

@@ -30,7 +30,7 @@ public partial class QueryTranslatorImpl : IFilterTranslator
 		private readonly string _queryIdentifier;
 		private readonly IASTNode _stageOneAst;
 		private readonly ISessionFactoryImplementor _factory;
-		private readonly IDictionary<string, NamedParameter> _namedParameters;
+		private readonly IDictionary<string, Tuple<IType, bool>> _namedParameters;
 
 		private bool _shallowQuery;
 		private bool _compiled;
@@ -70,7 +70,7 @@ internal QueryTranslatorImpl(
 			IASTNode parsedQuery,
 			IDictionary<string, IFilter> enabledFilters,
 			ISessionFactoryImplementor factory,
-			IDictionary<string, NamedParameter> namedParameters)
+			IDictionary<string, Tuple<IType, bool>> namedParameters)
 		{
 			_queryIdentifier = queryIdentifier;
 			_stageOneAst = parsedQuery;
@@ -474,15 +474,18 @@ private void ErrorIfDML()
 			}
 		}
 
-		internal bool TryGetNamedParameter(string name, out NamedParameter namedParameter)
+		public bool TryGetNamedParameterType(string name, out IType type, out bool isGuessedType)
 		{
-			if (_namedParameters == null)
+			if (_namedParameters == null || !_namedParameters.TryGetValue(name, out var p))
 			{
-				namedParameter = null;
+				type = null;
+				isGuessedType = false;
 				return false;
 			}
 
-			return _namedParameters.TryGetValue(name, out namedParameter);
+			type = p.Item1;
+			isGuessedType = p.Item2;
+			return true;
 		}
 	}
 

src/NHibernate/Linq/NhLinqExpression.cs

@@ -113,12 +113,18 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter
 			return DuplicateTree(ExpressionToHqlTranslationResults.Statement.AstNode);
 		}
 
-		internal void CopyExpressionTranslation(NhLinqExpression other)
+		internal void CopyExpressionTranslation(NhLinqExpressionCache cache)
 		{
-			ExpressionToHqlTranslationResults = other.ExpressionToHqlTranslationResults;
-			ParameterDescriptors = other.ParameterDescriptors;
+			ExpressionToHqlTranslationResults = cache.ExpressionToHqlTranslationResults;
+			ParameterDescriptors = cache.ParameterDescriptors;
 			// Type could have been overridden by translation.
-			Type = other.Type;
+			Type = cache.Type;
+		}
+
+		internal IDictionary<string, Tuple<IType, bool>> GetNamedParameterTypes()
+		{
+			return _constantToParameterMap.Values.Distinct()
+				.ToDictionary(p => p.Name, p => System.Tuple.Create(p.Type, p.IsGuessedType));
 		}
 
 		private static IASTNode DuplicateTree(IASTNode ast)

src/NHibernate/Linq/NhLinqExpressionCache.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using NHibernate.Engine;
+using NHibernate.Engine.Query;
+using NHibernate.Hql.Ast.ANTLR.Tree;
+
+namespace NHibernate.Linq
+{
+	internal class NhLinqExpressionCache : IQueryExpression
+	{
+		internal NhLinqExpressionCache(NhLinqExpression expression)
+		{
+			ExpressionToHqlTranslationResults = expression.ExpressionToHqlTranslationResults ?? throw new ArgumentException("NhLinqExpression is not translated");
+			Key = expression.Key;
+			Type = expression.Type;
+			ParameterDescriptors = expression.ParameterDescriptors;
+		}
+
+		public ExpressionToHqlTranslationResults ExpressionToHqlTranslationResults { get; }
+		public string Key { get; }
+		public System.Type Type { get; }
+		public IList<NamedParameterDescriptor> ParameterDescriptors { get; }
+
+		public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter)
+		{
+			return ExpressionToHqlTranslationResults.Statement.AstNode;
+		}
+	}
+}