Merge pull request #442 from PleasantD/NH-3816

NH-3816 - Fix Select with Conditional

Author: hazzik

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.Test/NHSpecificTest/NH3818/Fixture.cs

@@ -0,0 +1,70 @@
+using System;
+using System.Linq;
+using System.Linq.Dynamic;
+using NHibernate.Linq;
+using NUnit.Framework;
+
+namespace NHibernate.Test.NHSpecificTest.NH3818
+{
+	[TestFixture]
+	public class Fixture : BugTestCase
+	{
+		public override string BugNumber
+		{
+            get { return "NH3818"; }
+		}
+
+        [Test]
+        public void SelectConditionalValuesTest()
+        {
+            using (var spy = new SqlLogSpy())
+            using (var session = OpenSession())
+            using (session.BeginTransaction())
+            {
+                var days = 33;
+
+                var cat = new MyLovelyCat
+                {
+                    GUID = Guid.NewGuid(),
+                    Birthdate = DateTime.Now.AddDays(-days),
+                    Color = "Black",
+                    Name = "Kitty",
+                    Price = 0
+                };
+                session.Save(cat);
+                
+                session.Flush();
+
+                var catInfo =
+                    session.Query<MyLovelyCat>()
+                        .Select(o => new
+                        {
+                            o.Color,
+                            AliveDays = (int)(DateTime.Now - o.Birthdate).TotalDays,
+                            o.Name,
+                            o.Price,
+                        })
+                        .Single();
+
+                //Console.WriteLine(spy.ToString());
+                Assert.That(catInfo.AliveDays == days);
+
+                var catInfo2 =
+                    session.Query<MyLovelyCat>()
+                        .Select(o => new
+                        {
+                            o.Color,
+                            AliveDays = o.Price > 0 ? (DateTime.Now - o.Birthdate).TotalDays : 0,
+                            o.Name,
+                            o.Price,
+                        })
+                        .Single();
+
+                //Console.WriteLine(spy.ToString());
+                Assert.That(catInfo2.AliveDays == 0);
+
+            }
+        }
+
+    }
+}

src/NHibernate.Test/NHSpecificTest/NH3818/Mappings.hbm.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8" ?> 
+<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
+	assembly="NHibernate.Test"
+	namespace="NHibernate.Test.NHSpecificTest.NH3818"
+	default-lazy="false"
+  default-access="property">
+
+  <class name="MyLovelyCat" table="MyLovelyCat">
+    <id column="GUID" name="GUID"></id>
+    <property column="Name" name="Name" not-null="true"/>
+    <property column="Color" name="Color" not-null="true"/>
+    <property column="Birthdate" name="Birthdate" not-null="true"/>
+    <property column="Price" name="Price" not-null="true"/>
+  </class>
+
+</hibernate-mapping>

src/NHibernate.Test/NHSpecificTest/NH3818/MyLovelyCat.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace NHibernate.Test.NHSpecificTest.NH3818
+{
+    class MyLovelyCat
+    {
+        public virtual Guid GUID { get; set; }
+        
+        public virtual String Name { get; set; }
+        public virtual String Color { get; set; }
+        public virtual DateTime Birthdate { get; set; }
+        public virtual Decimal Price { get; set; }
+    }
+}

src/NHibernate.Test/NHibernate.Test.csproj

@@ -872,6 +872,8 @@
     <Compile Include="NHSpecificTest\NH3727\Entity.cs" />
     <Compile Include="NHSpecificTest\NH3727\FixtureByCode.cs" />
     <Compile Include="NHSpecificTest\NH3795\Fixture.cs" />
+    <Compile Include="NHSpecificTest\NH3818\Fixture.cs" />
+    <Compile Include="NHSpecificTest\NH3818\MyLovelyCat.cs" />
     <Compile Include="NHSpecificTest\NH646\Domain.cs" />
     <Compile Include="NHSpecificTest\NH646\Fixture.cs" />
     <Compile Include="NHSpecificTest\NH3234\Fixture.cs" />
@@ -3145,6 +3147,7 @@
     <EmbeddedResource Include="NHSpecificTest\NH1291AnonExample\Mappings.hbm.xml" />
   </ItemGroup>
   <ItemGroup>
+    <EmbeddedResource Include="NHSpecificTest\NH3818\Mappings.hbm.xml" />
     <EmbeddedResource Include="NHSpecificTest\NH3666\Mappings.hbm.xml">
       <SubType>Designer</SubType>
     </EmbeddedResource>

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);
+		}
+	}
+}