NH-3801 - Improve the handling of conditional expressions in select clauses

- Conditional statements are now generated in SQL for select clauses
- Case statements in the select clause don't generate unexpected or implicit joins for the test expression
- Combined the logic from SelectJoinDetector and ResultOperatorAndOrderByJoinDetector, also handles case statement join logic
- Removed most of the changes from NH-3797 (pull request nhibernate#432) as they are now unnecessary because the select clause now does the right thing

Author: PleasantD

src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs

@@ -407,7 +407,7 @@ private void DereferenceEntity(EntityType entityType, bool implicitJoin, string
 			}
 			else 
 			{
-				joinIsNeeded = generateJoin || ( Walker.IsInSelect || Walker.IsInFrom );
+				joinIsNeeded = generateJoin || ( (Walker.IsInSelect && !Walker.IsInCase ) || Walker.IsInFrom );
 			}
 
 			if ( joinIsNeeded ) 

src/NHibernate/Linq/GroupBy/AggregatingGroupByRewriter.cs

@@ -1,11 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Linq.Expressions;
 using NHibernate.Linq.Clauses;
 using NHibernate.Linq.ReWriters;
 using NHibernate.Linq.Visitors;
-using NHibernate.Util;
 using Remotion.Linq;
 using Remotion.Linq.Clauses.Expressions;
 using Remotion.Linq.Clauses.ResultOperators;
@@ -44,7 +42,7 @@ public static class AggregatingGroupByRewriter
 				typeof (CacheableResultOperator)
 			};
 
-		public static void ReWrite(QueryModel queryModel, IList<Expression> groupByKeys)
+		public static void ReWrite(QueryModel queryModel)
 		{
 			var subQueryExpression = queryModel.MainFromClause.FromExpression as SubQueryExpression;
 
@@ -59,7 +57,6 @@ public static void ReWrite(QueryModel queryModel, IList<Expression> groupByKeys)
 					var groupBy = operators[0] as GroupResultOperator;
 					if (groupBy != null)
 					{
-						groupBy.ExtractKeyExpressions(groupByKeys);
 						FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy);
 					}
 				}

src/NHibernate/Linq/GroupResultOperatorExtensions.cs

@@ -15,37 +15,36 @@ internal interface IIsEntityDecider
 	public class AddJoinsReWriter : QueryModelVisitorBase, IIsEntityDecider
 	{
 		private readonly ISessionFactoryImplementor _sessionFactory;
-		private readonly SelectJoinDetector _selectJoinDetector;
-		private readonly ResultOperatorAndOrderByJoinDetector _resultOperatorAndOrderByJoinDetector;
+		private readonly MemberExpressionJoinDetector _memberExpressionJoinDetector;
 		private readonly WhereJoinDetector _whereJoinDetector;
 
 		private AddJoinsReWriter(ISessionFactoryImplementor sessionFactory, QueryModel queryModel)
 		{
 			_sessionFactory = sessionFactory;
 			var joiner = new Joiner(queryModel);
-			_selectJoinDetector = new SelectJoinDetector(this, joiner);
-			_resultOperatorAndOrderByJoinDetector = new ResultOperatorAndOrderByJoinDetector(this, joiner);
+			_memberExpressionJoinDetector = new MemberExpressionJoinDetector(this, joiner);
 			_whereJoinDetector = new WhereJoinDetector(this, joiner);
 		}
 
-		public static void ReWrite(QueryModel queryModel, ISessionFactoryImplementor sessionFactory)
+		public static void ReWrite(QueryModel queryModel, VisitorParameters parameters)
 		{
-			new AddJoinsReWriter(sessionFactory, queryModel).VisitQueryModel(queryModel);
+			var visitor = new AddJoinsReWriter(parameters.SessionFactory, queryModel);
+			visitor.VisitQueryModel(queryModel);
 		}
 
 		public override void VisitSelectClause(SelectClause selectClause, QueryModel queryModel)
 		{
-			_selectJoinDetector.Transform(selectClause);
+			_memberExpressionJoinDetector.Transform(selectClause);
 		}
 
 		public override void VisitOrdering(Ordering ordering, QueryModel queryModel, OrderByClause orderByClause, int index)
 		{
-			_resultOperatorAndOrderByJoinDetector.Transform(ordering);
+			_memberExpressionJoinDetector.Transform(ordering);
 		}
 
 		public override void VisitResultOperator(ResultOperatorBase resultOperator, QueryModel queryModel, int index)
 		{
-			_resultOperatorAndOrderByJoinDetector.Transform(resultOperator);
+			_memberExpressionJoinDetector.Transform(resultOperator);
 		}
 
 		public override void VisitWhereClause(WhereClause whereClause, QueryModel queryModel, int index)

src/NHibernate/Linq/ReWriters/AddJoinsReWriter.cs

@@ -60,14 +60,14 @@ public void MakeInnerIfJoined(string key)
 		public bool CanAddJoin(Expression expression)
 		{
 			var source = QuerySourceExtractor.GetQuerySource(expression);
-			
-			if (_queryModel.MainFromClause == source) 
+
+			if (_queryModel.MainFromClause == source)
 				return true;
-			
+
 			var bodyClause = source as IBodyClause;
-			if (bodyClause != null && _queryModel.BodyClauses.Contains(bodyClause)) 
+			if (bodyClause != null && _queryModel.BodyClauses.Contains(bodyClause))
 				return true;
-			
+
 			var resultOperatorBase = source as ResultOperatorBase;
 			return resultOperatorBase != null && _queryModel.ResultOperators.Contains(resultOperatorBase);
 		}

src/NHibernate/Linq/Visitors/JoinBuilder.cs

@@ -0,0 +1,91 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq.Expressions;
+using NHibernate.Linq.ReWriters;
+using Remotion.Linq.Clauses;
+using Remotion.Linq.Clauses.Expressions;
+using Remotion.Linq.Parsing;
+
+namespace NHibernate.Linq.Visitors
+{
+	/// <summary>
+	/// Detects joins in Select, OrderBy and Results (GroupBy) clauses.
+	/// Replaces them with appropriate joins, maintaining reference equality between different clauses.
+	/// This allows extracted GroupBy key expression to also be replaced so that they can continue to match replaced Select expressions
+	/// </summary>
+	internal class MemberExpressionJoinDetector : ExpressionTreeVisitor
+	{
+		private readonly IIsEntityDecider _isEntityDecider;
+		private readonly IJoiner _joiner;
+
+		private bool _requiresJoinForNonIdentifier;
+		private bool _hasIdentifier;
+		private int _memberExpressionDepth;
+
+		public MemberExpressionJoinDetector(IIsEntityDecider isEntityDecider, IJoiner joiner)
+		{
+			_isEntityDecider = isEntityDecider;
+			_joiner = joiner;
+		}
+
+		protected override Expression VisitMemberExpression(MemberExpression expression)
+		{
+			var isIdentifier = _isEntityDecider.IsIdentifier(expression.Expression.Type, expression.Member.Name);
+			if (isIdentifier)
+				_hasIdentifier = true;
+			if (!isIdentifier)
+				_memberExpressionDepth++;
+
+			var result = base.VisitMemberExpression(expression);
+			
+			if (!isIdentifier)
+				_memberExpressionDepth--;
+
+			if (_isEntityDecider.IsEntity(expression.Type) &&
+				((_requiresJoinForNonIdentifier && !_hasIdentifier) || _memberExpressionDepth > 0) &&
+				_joiner.CanAddJoin(expression))
+			{
+				var key = ExpressionKeyVisitor.Visit(expression, null);
+				return _joiner.AddJoin(result, key);
+			}
+
+			return result;
+		}
+
+		protected override Expression VisitSubQueryExpression(SubQueryExpression expression)
+		{
+			expression.QueryModel.TransformExpressions(VisitExpression);
+			return expression;
+		}
+
+		protected override Expression VisitConditionalExpression(ConditionalExpression expression)
+		{
+			var oldRequiresJoinForNonIdentifier = _requiresJoinForNonIdentifier;
+			_requiresJoinForNonIdentifier = false;
+			var newTest = VisitExpression(expression.Test);
+			_requiresJoinForNonIdentifier = oldRequiresJoinForNonIdentifier;
+			var newFalse = VisitExpression(expression.IfFalse);
+			var newTrue = VisitExpression(expression.IfTrue);
+			if ((newTest != expression.Test) || (newFalse != expression.IfFalse) || (newTrue != expression.IfTrue))
+				return Expression.Condition(newTest, newTrue, newFalse);
+			return expression;
+		}
+
+		public void Transform(SelectClause selectClause)
+		{
+			_requiresJoinForNonIdentifier = true;
+			selectClause.TransformExpressions(VisitExpression);
+			_requiresJoinForNonIdentifier = false;
+		}
+
+		public void Transform(ResultOperatorBase resultOperator)
+		{
+			resultOperator.TransformExpressions(VisitExpression);
+		}
+
+		public void Transform(Ordering ordering)
+		{
+			ordering.TransformExpressions(VisitExpression);
+		}
+	}
+}

src/NHibernate/Linq/Visitors/MemberExpressionJoinDetector.cs

@@ -34,7 +34,7 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 			NonAggregatingGroupByRewriter.ReWrite(queryModel);
 
 			// Rewrite aggregate group-by statements
-			AggregatingGroupByRewriter.ReWrite(queryModel, parameters.GroupByKeys);
+			AggregatingGroupByRewriter.ReWrite(queryModel);
 
 			// Rewrite aggregating group-joins
 			AggregatingGroupJoinRewriter.ReWrite(queryModel);
@@ -57,7 +57,7 @@ public static ExpressionToHqlTranslationResults GenerateHqlQuery(QueryModel quer
 			ArrayIndexExpressionFlattener.ReWrite(queryModel);
 
 			// Add joins for references
-			AddJoinsReWriter.ReWrite(queryModel, parameters.SessionFactory);
+			AddJoinsReWriter.ReWrite(queryModel, parameters);
 
 			// Move OrderBy clauses to end
 			MoveOrderByToEndRewriter.ReWrite(queryModel);
@@ -238,7 +238,7 @@ public override void VisitSelectClause(SelectClause selectClause, QueryModel que
 		{
 			CurrentEvaluationType = selectClause.GetOutputDataInfo();
 
-			var visitor = new SelectClauseVisitor(typeof(object[]), VisitorParameters, VisitorParameters.GroupByKeys);
+			var visitor = new SelectClauseVisitor(typeof(object[]), VisitorParameters);
 
 			visitor.Visit(selectClause.Selector);
 

src/NHibernate/Linq/Visitors/QueryModelVisitor.cs

@@ -64,6 +64,12 @@ public override Expression VisitExpression(Expression expression)
 					ContainsUntranslatedMethodCalls = ContainsUntranslatedMethodCalls || !isRegisteredFunction;
 				}
 
+				// Attempt to project conditionals to the DB to prevent inner joins, or more projection work on result than is needed
+				if (expression != null && expression.NodeType == ExpressionType.Conditional)
+				{
+					projectConstantsInHql = true;
+				}
+
 				_stateStack.Push(projectConstantsInHql);
 
 				if (expression == null)

src/NHibernate/Linq/Visitors/ResultOperatorAndOrderByJoinDetector.cs

@@ -18,12 +18,11 @@ public class SelectClauseVisitor : ExpressionTreeVisitor
 		private List<HqlExpression> _hqlTreeNodes = new List<HqlExpression>();
 		private readonly HqlGeneratorExpressionTreeVisitor _hqlVisitor;
 
-		public SelectClauseVisitor(System.Type inputType, VisitorParameters parameters, IEnumerable<Expression> groupByKeys)
+		public SelectClauseVisitor(System.Type inputType, VisitorParameters parameters)
 		{
 			_inputParameter = Expression.Parameter(inputType, "input");
 			_parameters = parameters;
 			_hqlVisitor = new HqlGeneratorExpressionTreeVisitor(_parameters);
-			_hqlNodes = new HashSet<Expression>(groupByKeys);
 		}
 
 		public LambdaExpression ProjectionExpression { get; private set; }
@@ -44,7 +43,7 @@ public void Visit(Expression expression)
 			// Find the sub trees that can be expressed purely in HQL
 			var nominator = new SelectClauseHqlNominator(_parameters);
 			nominator.Visit(expression);
-			_hqlNodes.UnionWith(nominator.HqlCandidates);
+			_hqlNodes = nominator.HqlCandidates;
 
 			// Linq2SQL ignores calls to local methods. Linq2EF seems to not support
 			// calls to local methods at all. For NHibernate we support local methods,