LINQ: Prevent using server-side Distinct() together with projection methods executed locally, since the distinct would operate on the values before the local method is called.

Author: oskarb

src/NHibernate.Test/Linq/ByMethod/DistinctTests.cs

@@ -11,6 +11,7 @@ public class DistinctTests : LinqTestCase
 	{
 		public class OrderDto
 		{
+			public string ShipCountry { get; set; }
 			public DateTime? ShippingDate { get; set; }
 			public DateTime? OrderDate { get; set; }
 		}
@@ -80,9 +81,29 @@ public void DistinctOnTypeProjectionTwoProperty()
 		}
 
 		[Test]
-		public void DistinctOnTypeProjectionWithCustomProjectionMethods()
+		public void DistinctOnTypeProjectionWithHqlMethodIsOk()
 		{
-			//NH-2645
+			// Sort of related to NH-2645.
+
+			OrderDto[] result = db.Orders
+				.Select(x => new OrderDto
+				{
+					ShipCountry = x.ShippingAddress.Country.ToLower(),     // Should be translated to HQL/SQL.
+					ShippingDate = x.ShippingDate,
+					OrderDate = x.OrderDate.Value.Date,
+				})
+				.Distinct()
+				.ToArray();
+
+			result.Length.Should().Be.EqualTo(824);
+		}
+
+		[Test]
+		[ExpectedException(typeof(NotSupportedException), ExpectedMessage = "Cannot use distinct on result that depends on methods for which no SQL equivalent exist.")]
+		public void DistinctOnTypeProjectionWithCustomProjectionMethodsIsBlocked1()
+		{
+			// Sort of related to NH-2645.
+
 			OrderDto[] result = db.Orders
 				.Select(x => new OrderDto
 								 {
@@ -91,8 +112,23 @@ public void DistinctOnTypeProjectionWithCustomProjectionMethods()
 								 })
 				.Distinct()
 				.ToArray();
+		}
 
-			result.Length.Should().Be.EqualTo(774);
+
+		[Test]
+		[ExpectedException(typeof(NotSupportedException), ExpectedMessage = "Cannot use distinct on result that depends on methods for which no SQL equivalent exist.")]
+		public void DistinctOnTypeProjectionWithCustomProjectionMethodsIsBlocked2()
+		{
+			// Sort of related to NH-2645.
+
+			OrderDto[] result = db.Orders
+				.Select(x => new OrderDto
+				{
+					ShippingDate = x.ShippingDate,
+					OrderDate = x.OrderDate.Value.AddMonths(5),  // As of 2012-01-25, AddMonths() is executed locally.
+				})
+				.Distinct()
+				.ToArray();
 		}
 	}
 }

src/NHibernate/Linq/Visitors/SelectClauseNominator.cs

@@ -6,11 +6,27 @@
 
 namespace NHibernate.Linq.Visitors
 {
+	/// <summary>
+	/// Analyze the select clause to determine what parts can be translated
+	/// fully to HQL, and some other properties of the clause.
+	/// </summary>
 	class SelectClauseHqlNominator : ExpressionTreeVisitor
 	{
 		private readonly ILinqToHqlGeneratorsRegistry _functionRegistry;
 
-		private HashSet<Expression> _candidates;
+		/// <summary>
+		/// The expression parts that can be converted to pure HQL.
+		/// </summary>
+		public HashSet<Expression> HqlCandidates { get; private set; }
+
+		/// <summary>
+		/// If true after an expression have been analyzed, the
+		/// expression as a whole contain at least one method call which
+		/// cannot be converted to a registered function, i.e. it must
+		/// be executed client side.
+		/// </summary>
+		public bool ContainsUntranslatedMethodCalls { get; private set; }
+
 		private bool _canBeCandidate;
 		Stack<bool> _stateStack;
 
@@ -19,16 +35,15 @@ public SelectClauseHqlNominator(VisitorParameters parameters)
 			_functionRegistry = parameters.SessionFactory.Settings.LinqToHqlGeneratorsRegistry;
 		}
 
-		internal HashSet<Expression> Nominate(Expression expression)
+		internal void Visit(Expression expression)
 		{
-			_candidates = new HashSet<Expression>();
+			HqlCandidates = new HashSet<Expression>();
+			ContainsUntranslatedMethodCalls = false;
 			_canBeCandidate = true;
 			_stateStack = new Stack<bool>();
 			_stateStack.Push(false);
 
 			VisitExpression(expression);
-
-			return _candidates;
 		}
 
 		public override Expression VisitExpression(Expression expression)
@@ -38,6 +53,17 @@ public override Expression VisitExpression(Expression expression)
 				var projectConstantsInHql = _stateStack.Peek() ||
 											expression != null && IsRegisteredFunction(expression);
 
+				// Set some flags, unless we already have proper values for them:
+				//    projectConstantsInHql if they are inside a method call executed server side.
+				//    ContainsUntranslatedMethodCalls if a method call must be executed locally.
+				var isMethodCall = expression != null && expression.NodeType == ExpressionType.Call;
+				if (isMethodCall && (!projectConstantsInHql || !ContainsUntranslatedMethodCalls))
+				{
+					var isRegisteredFunction = IsRegisteredFunction(expression);
+					projectConstantsInHql = projectConstantsInHql || isRegisteredFunction;
+					ContainsUntranslatedMethodCalls = ContainsUntranslatedMethodCalls || !isRegisteredFunction;
+				}
+
 				_stateStack.Push(projectConstantsInHql);
 
 				if (expression == null)
@@ -49,7 +75,7 @@ public override Expression VisitExpression(Expression expression)
 
 				if (CanBeEvaluatedInHqlStatementShortcut(expression))
 				{
-					_candidates.Add(expression);
+					HqlCandidates.Add(expression);
 					return expression;
 				}
 
@@ -59,7 +85,7 @@ public override Expression VisitExpression(Expression expression)
 				{
 					if (CanBeEvaluatedInHqlSelectStatement(expression, projectConstantsInHql))
 					{
-						_candidates.Add(expression);
+						HqlCandidates.Add(expression);
 					}
 					else
 					{

src/NHibernate/Linq/Visitors/SelectClauseVisitor.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
@@ -40,7 +41,16 @@ public void Visit(Expression expression)
 			}
 
 			// Find the sub trees that can be expressed purely in HQL
-			_hqlNodes = new SelectClauseHqlNominator(_parameters).Nominate(expression);
+			var nominator = new SelectClauseHqlNominator(_parameters);
+			nominator.Visit(expression);
+			_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,
+			// but prevent their use together with server-side distinct, since it may
+			// end up being wrong.
+			if (distinct != null && nominator.ContainsUntranslatedMethodCalls)
+				throw new NotSupportedException("Cannot use distinct on result that depends on methods for which no SQL equivalent exist.");
 
 			// Now visit the tree
 			var projection = VisitExpression(expression);