Attempt to fix NH-3423, by simplifying expressions that compare the result of a construction expression with a null constant.

Author: oskarb

src/NHibernate.Test/Linq/WhereSubqueryTests.cs

@@ -636,5 +636,45 @@ public void ProductsWithSubqueryReturningStringFirstOrDefaultEq()
 
 			Assert.That(result.Count, Is.EqualTo(13));
 		}
+
+
+		[Test(Description = "NH-3423")]
+		public void NullComparedToNewExpressionInWhereClause()
+		{
+			// Construction will never be equal to null, so the ternary should be collapsed
+			// to just the IfFalse expression. Without this collapsing, we cannot generate HQL.
+
+			var result = db.Products
+				.Select(p => new {Name = p.Name, Pr2 = new {ReorderLevel = p.ReorderLevel}})
+				.Where(pr1 => (pr1.Pr2 == null ? (int?) null : pr1.Pr2.ReorderLevel) > 6)
+				.ToList();
+
+			Assert.That(result.Count, Is.EqualTo(45));
+		}
+
+		private class Pr2
+		{
+			public int ReorderLevel { get; set; }
+		}
+
+		private class Pr1
+		{
+			public string Name { get; set; }
+			public Pr2 Pr2 { get; set; }
+		}
+
+		[Test(Description = "NH-3423")]
+		public void NullComparedToMemberInitExpressionInWhereClause()
+		{
+			// Construction will never be equal to null, so the ternary should be collapsed
+			// to just the IfFalse expression. Without this collapsing, we cannot generate HQL.
+
+			var result = db.Products
+				.Select(p => new Pr1 { Name = p.Name, Pr2 = new Pr2 { ReorderLevel = p.ReorderLevel } })
+				.Where(pr1 => (pr1.Pr2 == null ? (int?)null : pr1.Pr2.ReorderLevel) > 6)
+				.ToList();
+
+			Assert.That(result.Count, Is.EqualTo(45));
+		}
 	}
 }

src/NHibernate/Linq/Visitors/HqlGeneratorExpressionTreeVisitor.cs

@@ -273,20 +273,14 @@ protected HqlTreeNode VisitBinaryExpression(BinaryExpression expression)
 			throw new InvalidOperationException();
 		}
 
+
 		private HqlTreeNode TranslateEqualityComparison(BinaryExpression expression, HqlExpression lhs, HqlExpression rhs, Func<HqlExpression, HqlTreeNode> applyNullComparison, Func<HqlExpression, HqlExpression, HqlTreeNode> applyRegularComparison)
 		{
 			// Check for nulls on left or right.
-			if (expression.Right is ConstantExpression && expression.Right.Type.IsNullableOrReference() &&
-				((ConstantExpression) expression.Right).Value == null)
-			{
+			if (VisitorUtil.IsNullConstant(expression.Right))
 				rhs = null;
-			}
-
-			if (expression.Left is ConstantExpression && expression.Left.Type.IsNullableOrReference() &&
-				((ConstantExpression) expression.Left).Value == null)
-			{
+			if (VisitorUtil.IsNullConstant(expression.Left))
 				lhs = null;
-			}
 
 			// Need to check for boolean equality
 			if (lhs is HqlBooleanExpression || rhs is HqlBooleanExpression)

src/NHibernate/Linq/Visitors/QueryModelVisitor.cs

@@ -236,6 +236,9 @@ public override void VisitSelectClause(SelectClause selectClause, QueryModel que
 
 		public override void VisitWhereClause(WhereClause whereClause, QueryModel queryModel, int index)
 		{
+			var visitor = new SimplifyConditionalVisitor();
+			whereClause.Predicate = visitor.VisitExpression(whereClause.Predicate);
+
 			// Visit the predicate to build the query
 			var expression = HqlGeneratorExpressionTreeVisitor.Visit(whereClause.Predicate, VisitorParameters).AsBooleanExpression();
 			if (whereClause is NhHavingClause)

src/NHibernate/Linq/Visitors/SimplifyConditionalVisitor.cs

@@ -0,0 +1,71 @@
+using System.Linq.Expressions;
+using NHibernate.Util;
+using Remotion.Linq.Parsing;
+
+namespace NHibernate.Linq.Visitors
+{
+	/// <summary>
+	/// Some conditional expressions can be redured to just their IfTrue or IfFalse part.
+	/// </summary>
+	internal class SimplifyConditionalVisitor :ExpressionTreeVisitor
+	{
+		protected override Expression VisitConditionalExpression(ConditionalExpression expression)
+		{
+			var testExpression = VisitExpression(expression.Test);
+
+			bool testExprResult;
+			if (VisitorUtil.IsBooleanConstant(testExpression, out testExprResult))
+			{
+				if (testExprResult)
+					return VisitExpression(expression.IfTrue);
+
+				return VisitExpression(expression.IfFalse);
+			}
+
+			return base.VisitConditionalExpression(expression);
+		}
+
+
+		protected override Expression VisitBinaryExpression(BinaryExpression expression)
+		{
+			// See NH-3423. Conditional expression where the test expression is a comparison
+			// of a construction expression and null will happen in WCF DS.
+
+			if (IsConstructionToNullComparison(expression))
+			{
+				// The result of a construction operation is always non-null. So if it's being compared to
+				// a null constant, we can simplify it to a boolean constant.
+				if (expression.NodeType == ExpressionType.Equal)
+					return Expression.Constant(false);
+
+				if (expression.NodeType == ExpressionType.NotEqual)
+					return Expression.Constant(true);
+			}
+
+			return base.VisitBinaryExpression(expression);
+		}
+
+
+		private static bool IsConstruction(Expression expression)
+		{
+			return expression is NewExpression || expression is MemberInitExpression;
+		}
+
+
+		private static bool IsConstructionToNullComparison(Expression expression)
+		{
+			var testExpression = expression as BinaryExpression;
+
+			if (testExpression != null)
+			{
+				if ((IsConstruction(testExpression.Left) && VisitorUtil.IsNullConstant(testExpression.Right))
+				    || (IsConstruction(testExpression.Right) && VisitorUtil.IsNullConstant(testExpression.Left)))
+				{
+					return true;
+				}
+			}
+
+			return false;
+		}
+	}
+}

src/NHibernate/Linq/Visitors/VisitorUtil.cs

@@ -3,6 +3,7 @@
 using System.Linq.Expressions;
 using System.Collections;
 using System.Reflection;
+using NHibernate.Util;
 
 namespace NHibernate.Linq.Visitors
 {
@@ -48,5 +49,27 @@ public static bool IsDynamicComponentDictionaryGetter(MethodCallExpression expre
 			string memberName;
 			return IsDynamicComponentDictionaryGetter(expression, sessionFactory, out memberName);
 		}
+
+
+		public static bool IsNullConstant(Expression expression)
+		{
+			return expression is ConstantExpression &&
+			       expression.Type.IsNullableOrReference() &&
+			       ((ConstantExpression)expression).Value == null;
+		}
+
+
+		public static bool IsBooleanConstant(Expression expression, out bool value)
+		{
+			var constantExpr = expression as ConstantExpression;
+			if (constantExpr != null && constantExpr.Type == typeof (bool))
+			{
+				value = (bool) constantExpr.Value;
+				return true;
+			}
+
+			value = false; // Dummy value.
+			return false;
+		}
 	}
 }

src/NHibernate/Linq/Visitors/WhereJoinDetector.cs

@@ -131,14 +131,14 @@ protected override Expression VisitBinaryExpression(BinaryExpression expression)
 			{
 				HandleBinaryOperation((a, b) => a.OrElse(b));
 			}
-			else if (expression.NodeType == ExpressionType.NotEqual && IsNullConstantExpression(expression.Right))
+			else if (expression.NodeType == ExpressionType.NotEqual && VisitorUtil.IsNullConstant(expression.Right))
 			{
 				// Discard result from right null.  Left is visited first, so it's below right on the stack.
 				_values.Pop();
 
 				HandleUnaryOperation(pvs => pvs.IsNotNull());
 			}
-			else if (expression.NodeType == ExpressionType.NotEqual && IsNullConstantExpression(expression.Left))
+			else if (expression.NodeType == ExpressionType.NotEqual && VisitorUtil.IsNullConstant(expression.Left))
 			{
 				// Discard result from left null.
 				var right = _values.Pop();
@@ -147,14 +147,14 @@ protected override Expression VisitBinaryExpression(BinaryExpression expression)
 
 				HandleUnaryOperation(pvs => pvs.IsNotNull());
 			}
-			else if (expression.NodeType == ExpressionType.Equal && IsNullConstantExpression(expression.Right))
+			else if (expression.NodeType == ExpressionType.Equal && VisitorUtil.IsNullConstant(expression.Right))
 			{
 				// Discard result from right null.  Left is visited first, so it's below right on the stack.
 				_values.Pop();
 
 				HandleUnaryOperation(pvs => pvs.IsNull());
 			}
-			else if (expression.NodeType == ExpressionType.Equal && IsNullConstantExpression(expression.Left))
+			else if (expression.NodeType == ExpressionType.Equal && VisitorUtil.IsNullConstant(expression.Left))
 			{
 				// Discard result from left null.
 				var right = _values.Pop();
@@ -324,12 +324,6 @@ protected override Expression VisitMemberExpression(MemberExpression expression)
 			return result;
 		}
 
-		private static bool IsNullConstantExpression(Expression expression)
-		{
-			var constant = expression as ConstantExpression;
-			return constant != null && constant.Value == null;
-		}
-
 		private void SetResultValues(ExpressionValues values)
 		{
 			_handled.Pop();

src/NHibernate/NHibernate.csproj

@@ -318,6 +318,7 @@
     <Compile Include="Linq\NestedSelects\NestedSelectRewriter.cs" />
     <Compile Include="Linq\Visitors\SelectJoinDetector.cs" />
     <Compile Include="Linq\Visitors\SelectClauseNominator.cs" />
+    <Compile Include="Linq\Visitors\SimplifyConditionalVisitor.cs" />
     <Compile Include="Linq\Visitors\SubQueryFromClauseFlattener.cs" />
     <Compile Include="Linq\Visitors\VisitorUtil.cs" />
     <Compile Include="Linq\Visitors\WhereJoinDetector.cs" />