NH-3816 - Nominate GroupBy key expressions directly and resolve only those to the database

* Similar intent to the original solution for NH-3797
* Nominations are done via a custom expression type so that join re-writing can still occur on the nominated expressions

Author: PleasantD

src/NHibernate.Test/Linq/JoinTests.cs

@@ -273,11 +273,13 @@ public void OrderLinesWithSelectingCustomerIdInCaseShouldProduceOneJoin()
 			}
 		}
 
-		[Test(Description = "NH-3801")]
+		[Test(Description = "NH-3801"), Ignore("This is an ideal case, but not possible without better join detection")]
 		public void OrderLinesWithSelectingCustomerInCaseShouldProduceOneJoin()
 		{
 			using (var spy = new SqlLogSpy())
 			{
+				// Without nominating the conditional to the select clause (and placing it in SQL)
+				// [l.Order.Customer] will be selected in its entirety, creating a second join 
 				(from l in db.OrderLines
 				 select new { CustomerKnown = l.Order.Customer == null ? 0 : 1, l.Order.OrderDate }).ToList();
 
@@ -299,11 +301,13 @@ public void OrderLinesWithSelectingCustomerNameInCaseShouldProduceTwoJoins()
 			}
 		}
 
-		[Test(Description = "NH-3801")]
+		[Test(Description = "NH-3801"), Ignore("This is an ideal case, but not possible without better join detection")]
 		public void OrderLinesWithSelectingCustomerNameInCaseShouldProduceTwoJoinsAlternate()
 		{
 			using (var spy = new SqlLogSpy())
 			{
+				// Without nominating the conditional to the select clause (and placing it in SQL)
+				// [l.Order.Customer] will be selected in its entirety, creating a second join 
 				(from l in db.OrderLines
 				 select new { CustomerKnown = l.Order.Customer == null ? "unknown" : l.Order.Customer.CompanyName, l.Order.OrderDate }).ToList();
 

src/NHibernate.Test/Linq/SelectionTests.cs

@@ -387,6 +387,76 @@ public void CanSelectCollection()
 			Assert.AreEqual(5, orders[0].Count);
 		}
 
+		[Test]
+		public void CanSelectConditionalKnownTypes()
+		{
+     		var moreThanTwoOrderLinesBool = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? true : false }).ToList();
+			Assert.That(moreThanTwoOrderLinesBool.Count(x => x.HasMoreThanTwo == true), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNBool = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? true : (bool?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNBool.Count(x => x.HasMoreThanTwo == true), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesShort = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? (short)1 : (short)0 }).ToList();
+			Assert.That(moreThanTwoOrderLinesShort.Count(x => x.HasMoreThanTwo == 1), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNShort = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? (short?)1 : (short?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNShort.Count(x => x.HasMoreThanTwo == 1), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesInt = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1 : 0 }).ToList();
+			Assert.That(moreThanTwoOrderLinesInt.Count(x => x.HasMoreThanTwo == 1), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNInt = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1 : (int?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNInt.Count(x => x.HasMoreThanTwo == 1), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesDecimal = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1m : 0m }).ToList();
+			Assert.That(moreThanTwoOrderLinesDecimal.Count(x => x.HasMoreThanTwo == 1m), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNDecimal = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1m : (decimal?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNDecimal.Count(x => x.HasMoreThanTwo == 1m), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesSingle = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1f : 0f }).ToList();
+			Assert.That(moreThanTwoOrderLinesSingle.Count(x => x.HasMoreThanTwo == 1f), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNSingle = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1f : (float?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNSingle.Count(x => x.HasMoreThanTwo == 1f), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesDouble = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1d : 0d }).ToList();
+			Assert.That(moreThanTwoOrderLinesDouble.Count(x => x.HasMoreThanTwo == 1d), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNDouble = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? 1d : (double?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNDouble.Count(x => x.HasMoreThanTwo == 1d), Is.EqualTo(410));
+			
+			var moreThanTwoOrderLinesString = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? "yes" : "no" }).ToList();
+			Assert.That(moreThanTwoOrderLinesString.Count(x => x.HasMoreThanTwo == "yes"), Is.EqualTo(410));
+
+			var now = DateTime.Now.Date;
+			var moreThanTwoOrderLinesDateTime = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? o.OrderDate.Value : now }).ToList();
+			Assert.That(moreThanTwoOrderLinesDateTime.Count(x => x.HasMoreThanTwo != now), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNDateTime = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? o.OrderDate : null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNDateTime.Count(x => x.HasMoreThanTwo != null), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesGuid = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? o.Shipper.Reference : Guid.Empty }).ToList();
+			Assert.That(moreThanTwoOrderLinesGuid.Count(x => x.HasMoreThanTwo != Guid.Empty), Is.EqualTo(410));
+
+			var moreThanTwoOrderLinesNGuid = db.Orders.Select(o => new { Id = o.OrderId, HasMoreThanTwo = o.OrderLines.Count() > 2 ? o.Shipper.Reference : (Guid?)null }).ToList();
+			Assert.That(moreThanTwoOrderLinesNGuid.Count(x => x.HasMoreThanTwo != null), Is.EqualTo(410));
+		}
+
+		[Test]
+		public void CanSelectConditionalEntity()
+		{
+			var fatherInsteadOfChild = db.Animals.Select(a => a.Father.SerialNumber == "5678" ? a.Father : a).ToList();
+			Assert.That(fatherInsteadOfChild, Has.Exactly(2).With.Property("SerialNumber").EqualTo("5678"));
+		}
+
+		[Test]
+		public void CanSelectConditionalObject()
+		{
+			var fatherIsKnown = db.Animals.Select(a => new { a.SerialNumber, Superior = a.Father.SerialNumber, FatherIsKnown = a.Father.SerialNumber == "5678" ? (object)true : (object)false }).ToList();
+			Assert.That(fatherIsKnown, Has.Exactly(1).With.Property("FatherIsKnown").True);
+		}
+
 		public class Wrapper<T>
 		{
 			public T item;

src/NHibernate/Linq/Expressions/NhExpressionType.cs

@@ -9,6 +9,7 @@ public enum NhExpressionType
 		Count,
 		Distinct,
 		New,
-	    Star
+	    Star,
+		Nominator
 	}
 }

src/NHibernate/Linq/Expressions/NhNominatedExpression.cs

@@ -0,0 +1,34 @@
+using System.Linq.Expressions;
+using Remotion.Linq.Clauses.Expressions;
+using Remotion.Linq.Parsing;
+
+namespace NHibernate.Linq.Expressions
+{
+	/// <summary>
+	/// Represents an expression that has been nominated for direct inclusion in the SELECT clause.
+	/// This bypasses the standard nomination process and assumes that the expression can be converted 
+	/// directly to SQL.
+	/// </summary>
+	/// <remarks>
+	/// Used in the nomination of GroupBy key expressions to ensure that matching select clauses
+	/// are generated the same way.
+	/// </remarks>
+	internal class NhNominatedExpression : ExtensionExpression
+	{
+		public Expression Expression { get; private set; }
+
+		public NhNominatedExpression(Expression expression) : base(expression.Type, (ExpressionType)NhExpressionType.Nominator)
+		{
+			Expression = expression;
+		}
+
+		protected override Expression VisitChildren(ExpressionTreeVisitor visitor)
+		{
+			var newExpression = visitor.VisitExpression(Expression);
+
+			return newExpression != Expression
+				? new NhNominatedExpression(newExpression)
+				: this;
+		}
+	}
+}

src/NHibernate/Linq/GroupBy/GroupBySelectClauseRewriter.cs

@@ -22,11 +22,13 @@ public static Expression ReWrite(Expression expression, GroupResultOperator grou
 
 		private readonly GroupResultOperator _groupBy;
 		private readonly QueryModel _model;
+		private readonly Expression _nominatedKeySelector;
 
 		private GroupBySelectClauseRewriter(GroupResultOperator groupBy, QueryModel model)
 		{
 			_groupBy = groupBy;
 			_model = model;
+			_nominatedKeySelector = GroupKeyNominator.Visit(groupBy);
 		}
 
 		protected override Expression VisitQuerySourceReferenceExpression(QuerySourceReferenceExpression expression)
@@ -53,7 +55,8 @@ protected override Expression VisitMemberExpression(MemberExpression expression)
 
 			if (expression.IsGroupingKeyOf(_groupBy))
 			{
-				return _groupBy.KeySelector;
+				// If we have referenced the Key, then return the nominated key expression
+				return _nominatedKeySelector;
 			}
 
 			var elementSelector = _groupBy.ElementSelector;

src/NHibernate/Linq/GroupBy/GroupKeyNominator.cs

@@ -0,0 +1,74 @@
+using System.Linq;
+using System.Linq.Expressions;
+using NHibernate.Linq.Expressions;
+using Remotion.Linq.Clauses.Expressions;
+using Remotion.Linq.Clauses.ResultOperators;
+using Remotion.Linq.Parsing;
+
+namespace NHibernate.Linq.GroupBy
+{
+	/// <summary>
+	/// This class nominates sub-expression trees on the GroupBy Key expression
+	/// for inclusion in the Select clause.
+	/// </summary>
+	internal class GroupKeyNominator : ExpressionTreeVisitor
+	{
+		private GroupKeyNominator() { }
+
+		private bool _requiresRootNomination;
+		private bool _transformed;
+		private int _depth;
+
+		public static Expression Visit(GroupResultOperator groupBy)
+		{
+			return VisitInternal(groupBy.KeySelector);
+		}
+
+		private static Expression VisitInternal(Expression expr)
+		{
+			return new GroupKeyNominator().VisitExpression(expr);
+		}
+
+		public override Expression VisitExpression(Expression expression)
+		{
+			_depth++;
+			var expr = base.VisitExpression(expression);
+			_depth--;
+
+			// At the root expression, wrap it in the nominator expression if needed
+			if (_depth == 0 && !_transformed && _requiresRootNomination)
+			{
+				expr = new NhNominatedExpression(expr);
+			}
+			return expr;
+		}
+
+		protected override Expression VisitNewArrayExpression(NewArrayExpression expression)
+		{
+			_transformed = true;
+			// Transform each initializer recursively (to allow for nested initializers)
+			return Expression.NewArrayInit(expression.Type.GetElementType(), expression.Expressions.Select(VisitInternal));
+		}
+
+		protected override Expression VisitNewExpression(NewExpression expression)
+		{
+			_transformed = true;
+			// Transform each initializer recursively (to allow for nested initializers)
+			return Expression.New(expression.Constructor, expression.Arguments.Select(VisitInternal), expression.Members);
+		}
+
+		protected override Expression VisitQuerySourceReferenceExpression(QuerySourceReferenceExpression expression)
+		{
+			// If the (sub)expression contains a QuerySourceReference, then the entire expression should be nominated
+			_requiresRootNomination = true;
+			return base.VisitQuerySourceReferenceExpression(expression);
+		}
+
+		protected override Expression VisitSubQueryExpression(SubQueryExpression expression)
+		{
+			// If the (sub)expression contains a QuerySourceReference, then the entire expression should be nominated
+			_requiresRootNomination = true;
+			return base.VisitSubQueryExpression(expression);
+		}
+	}
+}

src/NHibernate/Linq/GroupResultOperatorExtensions.cs

@@ -12,11 +12,19 @@ internal static class GroupResultOperatorExtensions
 	{
 		public static IEnumerable<Expression> ExtractKeyExpressions(this GroupResultOperator groupResult)
 		{
-			if (groupResult.KeySelector is NewExpression)
-				return (groupResult.KeySelector as NewExpression).Arguments;
-			if (groupResult.KeySelector is NewArrayExpression)
-				return (groupResult.KeySelector as NewArrayExpression).Expressions;
-			return new [] { groupResult.KeySelector };
+			return groupResult.KeySelector.ExtractKeyExpressions();
+		}
+
+		private static IEnumerable<Expression> ExtractKeyExpressions(this Expression expr)
+		{
+			// Recursively extract key expressions from nested initializers 
+			// --> new object[] { ((object)new object[] { x.A, x.B }), x.C }
+			// --> x.A, x.B, x.C
+			if (expr is NewExpression)
+				return (expr as NewExpression).Arguments.SelectMany(ExtractKeyExpressions);
+			if (expr is NewArrayExpression)
+				return (expr as NewArrayExpression).Expressions.SelectMany(ExtractKeyExpressions);
+			return new[] { expr };
 		}
 	}
 }

src/NHibernate/Linq/Visitors/HqlGeneratorExpressionTreeVisitor.cs

@@ -135,6 +135,8 @@ protected HqlTreeNode VisitExpression(Expression expression)
 							return VisitNhDistinct((NhDistinctExpression) expression);
 						case NhExpressionType.Star:
 							return VisitNhStar((NhStarExpression) expression);
+						case NhExpressionType.Nominator:
+							return VisitExpression(((NhNominatedExpression) expression).Expression);
 							//case NhExpressionType.New:
 							//    return VisitNhNew((NhNewExpression)expression);
 					}

src/NHibernate/Linq/Visitors/MemberExpressionJoinDetector.cs

@@ -1,6 +1,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq.Expressions;
+using NHibernate.Linq.Expressions;
 using NHibernate.Linq.ReWriters;
 using Remotion.Linq.Clauses;
 using Remotion.Linq.Clauses.Expressions;
@@ -19,6 +20,7 @@ internal class MemberExpressionJoinDetector : ExpressionTreeVisitor
 		private readonly IJoiner _joiner;
 
 		private bool _requiresJoinForNonIdentifier;
+		private bool _preventJoinsInConditionalTest;
 		private bool _hasIdentifier;
 		private int _memberExpressionDepth;
 
@@ -61,7 +63,7 @@ protected override Expression VisitSubQueryExpression(SubQueryExpression express
 		protected override Expression VisitConditionalExpression(ConditionalExpression expression)
 		{
 			var oldRequiresJoinForNonIdentifier = _requiresJoinForNonIdentifier;
-			_requiresJoinForNonIdentifier = false;
+			_requiresJoinForNonIdentifier = !_preventJoinsInConditionalTest && _requiresJoinForNonIdentifier;
 			var newTest = VisitExpression(expression.Test);
 			_requiresJoinForNonIdentifier = oldRequiresJoinForNonIdentifier;
 			var newFalse = VisitExpression(expression.IfFalse);
@@ -71,8 +73,21 @@ protected override Expression VisitConditionalExpression(ConditionalExpression e
 			return expression;
 		}
 
+		protected override Expression VisitExtensionExpression(ExtensionExpression expression)
+		{
+			// Nominated expressions need to prevent joins on non-Identifier member expressions 
+			// (for the test expression of conditional expressions only)
+			// Otherwise an extra join is created and the GroupBy and Select clauses will not match
+			var old = _preventJoinsInConditionalTest;
+			_preventJoinsInConditionalTest = (NhExpressionType)expression.NodeType == NhExpressionType.Nominator;
+			var expr = base.VisitExtensionExpression(expression);
+			_preventJoinsInConditionalTest = old;
+			return expr;
+		}
+
 		public void Transform(SelectClause selectClause)
 		{
+			// The select clause typically requires joins for non-Identifier member access
 			_requiresJoinForNonIdentifier = true;
 			selectClause.TransformExpressions(VisitExpression);
 			_requiresJoinForNonIdentifier = false;