NH-2319 - Fix plan retrieved from cache case. To be squashed.

Author: fredericDelaporte

src/NHibernate/Engine/Query/FilterQueryPlan.cs

@@ -10,17 +10,23 @@ namespace NHibernate.Engine.Query
 	[Serializable]
 	public class FilterQueryPlan : QueryExpressionPlan
 	{
-		private readonly string collectionRole;
-
 		public FilterQueryPlan(IQueryExpression queryExpression, string collectionRole, bool shallow, IDictionary<string, IFilter> enabledFilters, ISessionFactoryImplementor factory)
 			: base(queryExpression, collectionRole, shallow, enabledFilters, factory)
 		{
-			this.collectionRole = collectionRole;
+			CollectionRole = collectionRole;
 		}
 
-		public string CollectionRole
+		protected FilterQueryPlan(FilterQueryPlan source, IQueryExpression expression)
+			: base (source, expression)
+		{
+			CollectionRole = source.CollectionRole;
+		}
+
+		public string CollectionRole { get; }
+
+		public override QueryExpressionPlan Copy(IQueryExpression expression)
 		{
-			get { return collectionRole; }
+			return new FilterQueryPlan(this, expression);
 		}
 	}
-}
+}

src/NHibernate/Engine/Query/QueryExpressionPlan.cs

@@ -8,7 +8,7 @@ namespace NHibernate.Engine.Query
 	[Serializable]
 	public class QueryExpressionPlan : HQLQueryPlan, IQueryExpressionPlan
 	{
-		public IQueryExpression QueryExpression { get; private set; }
+		public IQueryExpression QueryExpression { get; }
 
 		public QueryExpressionPlan(IQueryExpression queryExpression, string collectionRole, bool shallow, IDictionary<string, IFilter> enabledFilters, ISessionFactoryImplementor factory)
 			: this(queryExpression.Key, CreateTranslators(queryExpression, collectionRole, shallow, enabledFilters, factory))
@@ -27,7 +27,7 @@ protected QueryExpressionPlan(string key, IQueryTranslator[] translators)
 		{
 		}
 
-		private QueryExpressionPlan(HQLQueryPlan source, IQueryExpression expression) 
+		protected QueryExpressionPlan(HQLQueryPlan source, IQueryExpression expression) 
 			: base(source)
 		{
 			QueryExpression = expression;
@@ -38,9 +38,9 @@ protected static IQueryTranslator[] CreateTranslators(IQueryExpression queryExpr
 			return factory.Settings.QueryTranslatorFactory.CreateQueryTranslators(queryExpression, collectionRole, shallow, enabledFilters, factory);
 		}
 
-		public QueryExpressionPlan Copy(IQueryExpression expression)
+		public virtual QueryExpressionPlan Copy(IQueryExpression expression)
 		{
 			return new QueryExpressionPlan(this, expression);
 		}
 	}
-}
+}

src/NHibernate/Engine/Query/QueryPlanCache.cs

@@ -67,23 +67,30 @@ public IQueryExpressionPlan GetHQLQueryPlan(IQueryExpression queryExpression, bo
 				{
 					log.Debug("located HQL query plan in cache (" + queryExpression.Key + ")");
 				}
-				var planExpression = plan.QueryExpression as NhLinqExpression;
-				var expression = queryExpression as NhLinqExpression;
-				if (planExpression != null && expression != null)
-				{
-					//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);
-					plan = plan.Copy(expression);
-				}
+				plan = CopyIfRequired(plan, queryExpression);
+			}
+
+			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)
+			{
+				//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);
+				plan = plan.Copy(expression);
 			}
 
 			return plan;
@@ -97,23 +104,18 @@ public IQueryExpressionPlan GetFilterQueryPlan(string filterString, string colle
 		public IQueryExpressionPlan GetFilterQueryPlan(IQueryExpression queryExpression, string collectionRole, bool shallow, IDictionary<string, IFilter> enabledFilters)
 		{
 			var key = new FilterQueryPlanKey(queryExpression.Key, collectionRole, shallow, enabledFilters);
-			var plan = (IQueryExpressionPlan) planCache[key];
+			var plan = (QueryExpressionPlan) planCache[key];
 
 			if (plan == null)
 			{
-				if (log.IsDebugEnabled)
-				{
-					log.Debug(string.Format("unable to locate collection-filter query plan in cache; generating ({0} : {1})", collectionRole, queryExpression.Key));
-				}
+				log.DebugFormat("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);
 			}
 			else
 			{
-				if (log.IsDebugEnabled)
-				{
-					log.Debug(string.Format("located collection-filter query plan in cache ({0} : {1})", collectionRole, queryExpression.Key));
-				}
+				log.DebugFormat("located collection-filter query plan in cache ({0} : {1})", collectionRole, queryExpression.Key);
+				plan = CopyIfRequired(plan, queryExpression);
 			}
 
 			return plan;