NH-3850 - Fix for polymorphic Linq query results aggregators failures. Causes a possible breaking change of Sum: Sum will no more yields null when there was nothing to sum, but will yield 0.

Author: fredericDelaporte

src/NHibernate/Linq/EnumerableHelper.cs

@@ -29,7 +29,7 @@ public static MethodInfo GetMethodDefinition<TSource>(Expression<Action<TSource>
 		public static MethodInfo GetMethod<TSource>(Expression<Action<TSource>> method)
 		{
 			if (method == null)
-				throw new ArgumentNullException("method");
+				throw new ArgumentNullException(nameof(method));
 
 			return ((MethodCallExpression)method.Body).Method;
 		}
@@ -54,11 +54,40 @@ public static MethodInfo GetMethodDefinition(Expression<System.Action> method)
 		public static MethodInfo GetMethod(Expression<System.Action> method)
 		{
 			if (method == null)
-				throw new ArgumentNullException("method");
+				throw new ArgumentNullException(nameof(method));
 
 			return ((MethodCallExpression)method.Body).Method;
 		}
 
+		/// <summary>
+		/// Get the <see cref="MethodInfo"/> for a public overload of a given method if the method does not match
+		/// given parameter types, otherwise directly yield the given method.
+		/// </summary>
+		/// <param name="method">The method for which finding an overload.</param>
+		/// <param name="parameterTypes">The arguments types of the overload to get.</param>
+		/// <returns>The <see cref="MethodInfo"/> of the method.</returns>
+		/// <remarks>Whenever possible, use GetMethod() instead for performance reasons.</remarks>
+		public static MethodInfo GetMethodOverload(MethodInfo method, System.Type[] parameterTypes)
+		{
+			if (method == null)
+				throw new ArgumentNullException(nameof(method));
+			if (parameterTypes == null)
+				throw new ArgumentNullException(nameof(parameterTypes));
+
+			if (ParameterTypesMatch(method.GetParameters(), parameterTypes))
+				return method;
+
+			var overload = method.DeclaringType.GetMethod(method.Name,
+				(method.IsStatic ? BindingFlags.Static : BindingFlags.Instance) | BindingFlags.Public,
+				null, parameterTypes, null);
+
+			if (overload == null)
+				throw new InvalidOperationException(
+					$"No overload found for method '{method.DeclaringType.Name}.{method.Name}' and parameter types '{string.Join(", ", parameterTypes.Select(t => t.Name))}'");
+
+			return overload;
+		}
+
 		/// <summary>
 		/// Gets the field or property to be accessed.
 		/// </summary>
@@ -70,7 +99,7 @@ public static MemberInfo GetProperty<TSource, TResult>(Expression<Func<TSource,
 		{
 			if (property == null)
 			{
-				throw new ArgumentNullException("property");
+				throw new ArgumentNullException(nameof(property));
 			}
 			return ((MemberExpression)property.Body).Member;
 		}
@@ -91,31 +120,8 @@ internal static System.Type GetPropertyOrFieldType(this MemberInfo memberInfo)
 
 			return null;
 		}
-	}
-	
-	[Obsolete("Please use ReflectionHelper instead")]
-	public static class EnumerableHelper
-	{
-		public static MethodInfo GetMethod(string name, System.Type[] parameterTypes)
-		{
-			return typeof(Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public)
-				.Where(m => m.Name == name &&
-							ParameterTypesMatch(m.GetParameters(), parameterTypes))
-				.Single();
-		}
-
-		public static MethodInfo GetMethod(string name, System.Type[] parameterTypes, System.Type[] genericTypeParameters)
-		{
-			return typeof(Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public)
-				.Where(m => m.Name == name &&
-							m.ContainsGenericParameters &&
-							m.GetGenericArguments().Count() == genericTypeParameters.Length &&
-							ParameterTypesMatch(m.GetParameters(), parameterTypes))
-				.Single()
-				.MakeGenericMethod(genericTypeParameters);
-		}
 
-		private static bool ParameterTypesMatch(ParameterInfo[] parameters, System.Type[] types)
+		internal static bool ParameterTypesMatch(ParameterInfo[] parameters, System.Type[] types)
 		{
 			if (parameters.Length != types.Length)
 			{
@@ -141,4 +147,27 @@ private static bool ParameterTypesMatch(ParameterInfo[] parameters, System.Type[
 			return true;
 		}
 	}
+
+	[Obsolete("Please use ReflectionHelper instead")]
+	public static class EnumerableHelper
+	{
+		public static MethodInfo GetMethod(string name, System.Type[] parameterTypes)
+		{
+			return typeof(Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public)
+				.Where(m => m.Name == name &&
+							ReflectionHelper.ParameterTypesMatch(m.GetParameters(), parameterTypes))
+				.Single();
+		}
+
+		public static MethodInfo GetMethod(string name, System.Type[] parameterTypes, System.Type[] genericTypeParameters)
+		{
+			return typeof(Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public)
+				.Where(m => m.Name == name &&
+							m.ContainsGenericParameters &&
+							m.GetGenericArguments().Count() == genericTypeParameters.Length &&
+							ReflectionHelper.ParameterTypesMatch(m.GetParameters(), parameterTypes))
+				.Single()
+				.MakeGenericMethod(genericTypeParameters);
+		}
+	}
 }

src/NHibernate/Linq/ExpressionToHqlTranslationResults.cs

@@ -16,11 +16,18 @@ public class ExpressionToHqlTranslationResults
 		public Delegate PostExecuteTransformer { get; private set; }
 		public List<Action<IQuery, IDictionary<string, Tuple<object, IType>>>> AdditionalCriteria { get; private set; }
 
+		/// <summary>
+		/// If execute result type does not match expected final result type (implying a post execute transformer
+		/// will yield expected result type), the intermediate execute type.
+		/// </summary>
+		public System.Type ExecuteResultTypeOverride { get; private set; }
+
 		public ExpressionToHqlTranslationResults(HqlTreeNode statement, 
 			IList<LambdaExpression> itemTransformers, 
 			IList<LambdaExpression> listTransformers,
 			IList<LambdaExpression> postExecuteTransformers,
-			List<Action<IQuery, IDictionary<string, Tuple<object, IType>>>> additionalCriteria)
+			List<Action<IQuery, IDictionary<string, Tuple<object, IType>>>> additionalCriteria,
+			System.Type executeResultTypeOverride)
 		{
 			Statement = statement;
 
@@ -35,6 +42,7 @@ public ExpressionToHqlTranslationResults(HqlTreeNode statement,
 			}
 
 			AdditionalCriteria = additionalCriteria;
+			ExecuteResultTypeOverride = executeResultTypeOverride;
 		}
 
 		private static TDelegate MergeLambdasAndCompile<TDelegate>(IList<LambdaExpression> itemTransformers) 

src/NHibernate/Linq/IntermediateHqlTree.cs

@@ -50,6 +50,12 @@ public HqlTreeNode Root
 			}
 		}
 
+		/// <summary>
+		/// If execute result type does not match expected final result type (implying a post execute transformer
+		/// will yield expected result type), the intermediate execute type.
+		/// </summary>
+		public System.Type ExecuteResultTypeOverride { get; set; }
+
 		public HqlTreeBuilder TreeBuilder { get; private set; }
 
 		public IntermediateHqlTree(bool root)
@@ -62,10 +68,11 @@ public IntermediateHqlTree(bool root)
 		public ExpressionToHqlTranslationResults GetTranslation()
 		{
 			return new ExpressionToHqlTranslationResults(Root,
-														 _itemTransformers,
-														 _listTransformers,
-														 _postExecuteTransformers,
-														 _additionalCriteria);
+				_itemTransformers,
+				_listTransformers,
+				_postExecuteTransformers,
+				_additionalCriteria,
+				ExecuteResultTypeOverride);
 		}
 
 		public void AddDistinctRootOperator()

src/NHibernate/Linq/NhLinqExpression.cs

@@ -64,7 +64,10 @@ public IASTNode Translate(ISessionFactoryImplementor sessionFactory, bool filter
 			var queryModel = NhRelinqQueryParser.Parse(_expression);
 			var visitorParameters = new VisitorParameters(sessionFactory, _constantToParameterMap, requiredHqlParameters, querySourceNamer);
 
-			ExpressionToHqlTranslationResults = QueryModelVisitor.GenerateHqlQuery(queryModel, visitorParameters, true);
+			ExpressionToHqlTranslationResults = QueryModelVisitor.GenerateHqlQuery(queryModel, visitorParameters, true, ReturnType);
+
+			if (ExpressionToHqlTranslationResults.ExecuteResultTypeOverride != null)
+				Type = ExpressionToHqlTranslationResults.ExecuteResultTypeOverride;
 
 			ParameterDescriptors = requiredHqlParameters.AsReadOnly();
 

src/NHibernate/Linq/Visitors/HqlGeneratorExpressionTreeVisitor.cs

@@ -542,7 +542,7 @@ protected HqlTreeNode VisitConditionalExpression(ConditionalExpression expressio
 
 		protected HqlTreeNode VisitSubQueryExpression(SubQueryExpression expression)
 		{
-			ExpressionToHqlTranslationResults query = QueryModelVisitor.GenerateHqlQuery(expression.QueryModel, _parameters, false);
+			ExpressionToHqlTranslationResults query = QueryModelVisitor.GenerateHqlQuery(expression.QueryModel, _parameters, false, null);
 			return query.Statement;
 		}
 

src/NHibernate/Linq/Visitors/QueryModelVisitor.cs

@@ -1,15 +1,17 @@
 using System;
-using System.Collections;
-using System.Collections.Generic;
+using System.Linq;
 using System.Linq.Expressions;
+using System.Reflection;
 using NHibernate.Hql.Ast;
 using NHibernate.Linq.Clauses;
+using NHibernate.Linq.Expressions;
 using NHibernate.Linq.GroupBy;
 using NHibernate.Linq.GroupJoin;
 using NHibernate.Linq.NestedSelects;
 using NHibernate.Linq.ResultOperators;
 using NHibernate.Linq.ReWriters;
 using NHibernate.Linq.Visitors.ResultOperatorProcessors;
+using NHibernate.Util;
 using Remotion.Linq;
 using Remotion.Linq.Clauses;
 using Remotion.Linq.Clauses.ResultOperators;
@@ -20,7 +22,8 @@ namespace NHibernate.Linq.Visitors
 {
 	public class QueryModelVisitor : QueryModelVisitorBase
 	{
-		public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel queryModel, VisitorParameters parameters, bool root)
+		public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel queryModel, VisitorParameters parameters, bool root,
+			NhLinqExpressionReturnType? rootReturnType)
 		{
 			NestedSelectRewriter.ReWrite(queryModel, parameters.SessionFactory);
 
@@ -29,7 +32,7 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 
 			// Merge aggregating result operators (distinct, count, sum etc) into the select clause
 			MergeAggregatingResultsRewriter.ReWrite(queryModel);
-			
+
 			// Swap out non-aggregating group-bys
 			NonAggregatingGroupByRewriter.ReWrite(queryModel);
 
@@ -79,7 +82,7 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 			// Identify and name query sources
 			QuerySourceIdentifier.Visit(parameters.QuerySourceNamer, queryModel);
 
-			var visitor = new QueryModelVisitor(parameters, root, queryModel)
+			var visitor = new QueryModelVisitor(parameters, root, queryModel, rootReturnType)
 			{
 				RewrittenOperatorResult = result,
 			};
@@ -89,7 +92,9 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 		}
 
 		private readonly IntermediateHqlTree _hqlTree;
+		private readonly NhLinqExpressionReturnType? _rootReturnType;
 		private static readonly ResultOperatorMap ResultOperatorMap;
+
 		private bool _serverSide = true;
 
 		public VisitorParameters VisitorParameters { get; private set; }
@@ -128,16 +133,64 @@ static QueryModelVisitor()
 			ResultOperatorMap.Add<CastResultOperator, ProcessCast>();
 		}
 
-		private QueryModelVisitor(VisitorParameters visitorParameters, bool root, QueryModel queryModel)
+		private QueryModelVisitor(VisitorParameters visitorParameters, bool root, QueryModel queryModel,
+			NhLinqExpressionReturnType? rootReturnType)
 		{
 			VisitorParameters = visitorParameters;
 			Model = queryModel;
+			_rootReturnType = root ? rootReturnType : null;
 			_hqlTree = new IntermediateHqlTree(root);
 		}
 
 		private void Visit()
 		{
 			VisitQueryModel(Model);
+
+			if (_rootReturnType == NhLinqExpressionReturnType.Scalar && Model.ResultTypeOverride != null)
+			{
+				// NH-3850: handle polymorphic scalar results aggregation
+				switch ((NhExpressionType)Model.SelectClause.Selector.NodeType)
+				{
+					case NhExpressionType.Average:
+						// Polymorphic case complex to handle and not implemented. (HQL query must be reshaped for adding
+						// additional data to allow a meaningful overall average computation.)
+						// Leaving it untouched for allowing non polymorphic cases to work.
+						break;
+					case NhExpressionType.Max:
+						AddPostExecuteTransformerForResultAggregate(ReflectionCache.QueryableMethods.MaxDefinition);
+						break;
+					case NhExpressionType.Min:
+						AddPostExecuteTransformerForResultAggregate(ReflectionCache.QueryableMethods.MinDefinition);
+						break;
+					case NhExpressionType.Count:
+					// Count results have to be summed.
+					case NhExpressionType.Sum:
+						// Sum has no suitable generic overload, throw in one Sum candidate, then the code using it
+						// will check and adjust it if needed (AddPostExecuteTransformerForResultAggregate does that).
+						AddPostExecuteTransformerForResultAggregate(ReflectionCache.QueryableMethods.SumOnInt);
+						break;
+				}
+			}
+		}
+
+		private void AddPostExecuteTransformerForResultAggregate(MethodInfo aggregateMethodTemplate)
+		{
+			var resultType = Model.ResultTypeOverride;
+			var inputListType = typeof(IQueryable<>).MakeGenericType(resultType);
+			MethodInfo aggregateMethod;
+			if (aggregateMethodTemplate.IsGenericMethodDefinition)
+			{
+				aggregateMethod = aggregateMethodTemplate.MakeGenericMethod(new[] { resultType });
+			}
+			else
+			{
+				// Ensure we use the right overload.
+				aggregateMethod = ReflectionHelper.GetMethodOverload(aggregateMethodTemplate, new[] { inputListType });
+			}
+
+			var inputList = Expression.Parameter(inputListType, "inputList");
+			var aggregateCall = Expression.Call(aggregateMethod, inputList);
+			_hqlTree.AddPostExecuteTransformer(Expression.Lambda(aggregateCall, inputList));
 		}
 
 		public override void VisitMainFromClause(MainFromClause fromClause, QueryModel queryModel)

src/NHibernate/Linq/Visitors/ResultOperatorProcessors/ProcessAggregate.cs

@@ -19,20 +19,22 @@ public void Process(AggregateResultOperator resultOperator, QueryModelVisitor qu
 				resultOperator.Func.Parameters[0],
 				paramExpr);
 
-			var inputList = Expression.Parameter(typeof(IEnumerable<>).MakeGenericType(typeof(object)), "inputList");
-
-			var castToItem = ReflectionCache.EnumerableMethods.CastDefinition.MakeGenericMethod(new[] { inputType });
-			var castToItemExpr = Expression.Call(castToItem, inputList);
-
+			// NH-3850: changed from list transformer (working on IEnumerable<object>) to post execute
+			// transformer (working on IEnumerable<inputType>) for globally aggregating polymorphic results
+			// instead of aggregating results for each class separately and yielding only the first.
+			// If the aggregation relies on ordering, final result will still be wrong due to
+			// polymorphic results being union-ed without re-ordering. (This is a limitation of all polymorphic
+			// queries, this is not specific to LINQ provider.)
+			var inputList = Expression.Parameter(typeof(IEnumerable<>).MakeGenericType(inputType), "inputList");
 			var aggregate = ReflectionCache.EnumerableMethods.AggregateDefinition.MakeGenericMethod(inputType);
-
 			MethodCallExpression call = Expression.Call(
 				aggregate,
-				castToItemExpr,
+				inputList,
 				accumulatorFunc
 				);
-
-			tree.AddListTransformer(Expression.Lambda(call, inputList));
+			tree.AddPostExecuteTransformer(Expression.Lambda(call, inputList));
+			// There is no more a list transformer yielding an IList<resultType>, but this aggregate case
+			// have inputType = resultType, so no further action is required.
 		}
 	}
 }