NH-4009 - Handling db-only methods without exceptions

Author: fredericDelaporte

doc/reference/modules/query_linq.xml

@@ -67,6 +67,10 @@ using NHibernate.Linq;]]></programlisting>
     session.Query<Cat>()
         .Where(c => c.BirthDate == DateTime.Today.MappedAs(NHibernateUtil.Date))
         .ToList();]]></programlisting>
+    <programlisting><![CDATA[IList<Cat> cats =
+    session.Query<Cat>()
+        .Where(c => c.Name == "Max".MappedAs(TypeFactory.Basic("AnsiString(200)")))
+        .ToList();]]></programlisting>
   </sect1>
 
   <sect1 id="querylinq-supportedmethods">
@@ -656,11 +660,20 @@ IList<Cat> cats =
     }
 }]]></programlisting>
       <para>
-        The method call will always be translated to SQL if at least one of the parameters of the
-        method call has its value originating from an entity. Otherwise, the Linq provider will try to
-        evaluate the method call with .Net runtime instead. Since NHibernate 5.0, if this runtime
-        evaluation fails (throws an exception), then the method call will be translated to SQL too.
+        Since NHibernate v5.0, the Linq provider will no more evaluate in-memory the method call
+        even when it does not depend on the queried data. If you wish to have the method call evaluated
+        before querying whenever possible, and then replaced in the query by its resulting value, specify
+        <literal>LinqExtensionPreEvaluation.AllowPreEvaluation</literal> on the attribute.
       </para>
+      <programlisting><![CDATA[public static class CustomLinqExtensions
+{
+    [LinqExtensionMethod("dbo.aCustomFunction", LinqExtensionPreEvaluation.AllowPreEvaluation)]
+    public static string ACustomFunction(this string input, string otherInput)
+    {
+        // In-memory evaluation implementation.
+        return input.Replace(otherInput, "blah");
+    }
+}]]></programlisting>
     </sect2>
 
     <sect2 id="querylinq-extending-generator">
@@ -767,6 +780,13 @@ cfg.LinqToHqlGeneratorsRegistry<ExtendedLinqToHqlGeneratorsRegistry>();
       <para>
         (Of course, the same result could be obtained with <literal>(DateTime?)(c.BirthDate)</literal>.)
       </para>
+      <para>
+        By default, the Linq provider will try to evaluate the method call with .Net runtime
+        whenever possible, instead of translating it to SQL. It will not do it if at least one
+        of the parameters of the method call has its value originating from an entity, or if
+        the method is marked with the <literal>NoPreEvaluation</literal> attribute (available
+        since NHibernate 5.0).
+      </para>
     </sect2>
   </sect1>
 </chapter>

src/NHibernate.Test/Linq/ExtensionMethods.cs

@@ -1,19 +1,38 @@
-using NHibernate.Linq;
+using System;
+using NHibernate.Linq;
 
 namespace NHibernate.Test.Linq
 {
-    static class ExtensionMethods
-    {
-        [LinqExtensionMethod("Replace")]
-        public static string ReplaceExtension(this string subject, string search, string replaceWith)
-        {
-            return null;
-        }
+	static class ExtensionMethods
+	{
+		[LinqExtensionMethod("Replace")]
+		public static string ReplaceExtension(this string subject, string search, string replaceWith)
+		{
+			throw new InvalidOperationException("To be translated to SQL only");
+		}
 
-        [LinqExtensionMethod]
-        public static string Replace(this string subject, string search, string replaceWith)
-        {
-            return null;
-        }
-    }
+		[LinqExtensionMethod]
+		public static string Replace(this string subject, string search, string replaceWith)
+		{
+			throw new InvalidOperationException("To be translated to SQL only");
+		}
+
+		// NH4 default behavior
+		[LinqExtensionMethod("Replace", LinqExtensionPreEvaluation.AllowPreEvaluation)]
+		public static string ReplaceWithEvaluation(this string subject, string search, string replaceWith)
+		{
+			if (subject == null)
+				throw new ArgumentNullException(nameof(subject));
+			return subject.Replace(search, replaceWith) + " (done in-memory)";
+		}
+
+		// Just for checking weird combination
+		[LinqExtensionMethod("Replace", LinqExtensionPreEvaluation.AllowPreEvaluation), NoPreEvaluation]
+		public static string ReplaceWithEvaluation2(this string subject, string search, string replaceWith)
+		{
+			if (subject == null)
+				throw new ArgumentNullException(nameof(subject));
+			return subject.Replace(search, replaceWith) + " (done in-memory)";
+		}
+	}
 }

src/NHibernate.Test/Linq/FunctionTests.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Text.RegularExpressions;
 using NHibernate.DomainModel;
 using NHibernate.DomainModel.Northwind.Entities;
 using NHibernate.Linq;
@@ -114,17 +115,58 @@ where e.FirstName.Substring(0, 2) == "An"
 		[Test]
 		public void ReplaceFunction()
 		{
+			var suppliedName = "Anne";
 			var query = from e in db.Employees
 						where e.FirstName.StartsWith("An")
 						select new
-								{
-									Before = e.FirstName,
-									AfterMethod = e.FirstName.Replace("An", "Zan"),
-									AfterExtension = ExtensionMethods.Replace(e.FirstName, "An", "Zan"),
-									AfterExtension2 = e.FirstName.ReplaceExtension("An", "Zan")
-								};
-
-			var s = ObjectDumper.Write(query);
+							{
+								Before = e.FirstName,
+								// This one call the standard string.Replace, not the extension. The linq registry handles it.
+								AfterMethod = e.FirstName.Replace("An", "Zan"),
+								AfterExtension = ExtensionMethods.Replace(e.FirstName, "An", "Zan"),
+								AfterNamedExtension = e.FirstName.ReplaceExtension("An", "Zan"),
+								AfterEvaluableExtension = e.FirstName.ReplaceWithEvaluation("An", "Zan"),
+								AfterEvaluable2Extension = e.FirstName.ReplaceWithEvaluation2("An", "Zan"),
+							BeforeConst = suppliedName,
+								// This one call the standard string.Replace, not the extension. The linq registry handles it.
+								AfterMethodConst = suppliedName.Replace("An", "Zan"),
+								AfterExtensionConst = ExtensionMethods.Replace(suppliedName, "An", "Zan"),
+								AfterNamedExtensionConst = suppliedName.ReplaceExtension("An", "Zan"),
+								AfterEvaluableExtensionConst = suppliedName.ReplaceWithEvaluation("An", "Zan"),
+								AfterEvaluable2ExtensionConst = suppliedName.ReplaceWithEvaluation2("An", "Zan")
+						};
+			var results = query.ToList();
+			var s = ObjectDumper.Write(results);
+
+			foreach (var result in results)
+			{
+				var expectedDbResult = Regex.Replace(result.Before, "An", "Zan", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+				Assert.That(result.AfterMethod, Is.EqualTo(expectedDbResult), $"Wrong {nameof(result.AfterMethod)} value");
+				Assert.That(result.AfterExtension, Is.EqualTo(expectedDbResult), $"Wrong {nameof(result.AfterExtension)} value");
+				Assert.That(result.AfterNamedExtension, Is.EqualTo(expectedDbResult), $"Wrong {nameof(result.AfterNamedExtension)} value");
+				Assert.That(result.AfterEvaluableExtension, Is.EqualTo(expectedDbResult), $"Wrong {nameof(result.AfterEvaluableExtension)} value");
+				Assert.That(result.AfterEvaluable2Extension, Is.EqualTo(expectedDbResult), $"Wrong {nameof(result.AfterEvaluable2Extension)} value");
+
+				var expectedDbResultConst = Regex.Replace(result.BeforeConst, "An", "Zan", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+				var expectedInMemoryResultConst = result.BeforeConst.Replace("An", "Zan");
+				var expectedInMemoryExtensionResultConst = result.BeforeConst.ReplaceWithEvaluation("An", "Zan");
+				Assert.That(result.AfterMethodConst, Is.EqualTo(expectedInMemoryResultConst), $"Wrong {nameof(result.AfterMethodConst)} value");
+				Assert.That(result.AfterExtensionConst, Is.EqualTo(expectedDbResultConst), $"Wrong {nameof(result.AfterExtensionConst)} value");
+				Assert.That(result.AfterNamedExtensionConst, Is.EqualTo(expectedDbResultConst), $"Wrong {nameof(result.AfterNamedExtensionConst)} value");
+				Assert.That(result.AfterEvaluableExtensionConst, Is.EqualTo(expectedInMemoryExtensionResultConst), $"Wrong {nameof(result.AfterEvaluableExtensionConst)} value");
+				Assert.That(result.AfterEvaluable2ExtensionConst, Is.EqualTo(expectedInMemoryExtensionResultConst), $"Wrong {nameof(result.AfterEvaluable2ExtensionConst)} value");
+			}
+
+			// Should cause ReplaceWithEvaluation to fail
+			suppliedName = null;
+			var failingQuery = from e in db.Employees
+						where e.FirstName.StartsWith("An")
+						select new
+						{
+							Before = e.FirstName,
+							AfterEvaluableExtensionConst = suppliedName.ReplaceWithEvaluation("An", "Zan")
+						};
+			Assert.That(() => failingQuery.ToList(), Throws.InstanceOf<HibernateException>().And.InnerException.InstanceOf<ArgumentNullException>());
 		}
 
 		[Test]

src/NHibernate/Linq/LinqExtensionMethodAttribute.cs

@@ -2,17 +2,106 @@
 
 namespace NHibernate.Linq
 {
-	public class LinqExtensionMethodAttribute: Attribute
+	/// <summary>
+	/// Flag a method as being a SQL function call for the linq-to-nhibernate provider. Its
+	/// parameters will be used as the function call parameters.
+	/// </summary>
+	public class LinqExtensionMethodAttribute : LinqExtensionMethodAttributeBase
 	{
+		/// <summary>
+		/// Default constructor. The method call will be translated by the linq provider to
+		/// a function call having the same name than the method.
+		/// </summary>
 		public LinqExtensionMethodAttribute()
-		{
-		}
+			: base(LinqExtensionPreEvaluation.NoEvaluation) { }
 
+		/// <summary>
+		/// Constructor specifying a SQL function name.
+		/// </summary>
+		/// <param name="name">The name of the SQL function.</param>
 		public LinqExtensionMethodAttribute(string name)
+			: this(name, LinqExtensionPreEvaluation.NoEvaluation) { }
+
+		/// <summary>
+		/// Constructor allowing to specify a <see cref="LinqExtensionPreEvaluation"/> for the method.
+		/// </summary>
+		/// <param name="preEvaluation">Should the method call be pre-evaluated when not depending on
+		/// queried data? Default is <see cref="LinqExtensionPreEvaluation.NoEvaluation"/>.</param>
+		public LinqExtensionMethodAttribute(LinqExtensionPreEvaluation preEvaluation)
+			: base(preEvaluation) { }
+
+		/// <summary>
+		/// Constructor for specifying a SQL function name and a <see cref="LinqExtensionPreEvaluation"/>.
+		/// </summary>
+		/// <param name="name">The name of the SQL function.</param>
+		/// <param name="preEvaluation">Should the method call be pre-evaluated when not depending on
+		/// queried data? Default is <see cref="LinqExtensionPreEvaluation.NoEvaluation"/>.</param>
+		public LinqExtensionMethodAttribute(string name, LinqExtensionPreEvaluation preEvaluation)
+			: base(preEvaluation)
 		{
 			Name = name;
 		}
 
-		public string Name { get; private set; }
+		/// <summary>
+		/// The name of the SQL function.
+		/// </summary>
+		public string Name { get; }
+	}
+
+	/// <summary>
+	/// Can flag a method as not being callable by the runtime, when used in Linq queries.
+	/// If the method is supported by the linq-to-nhibernate provider, it will always be converted
+	/// to the corresponding SQL statement.
+	/// Otherwise the linq-to-nhibernate provider evaluates method calls when they do not depend on
+	/// the queried data.
+	/// </summary>
+	public class NoPreEvaluationAttribute : LinqExtensionMethodAttributeBase
+	{
+		/// <summary>
+		/// Default constructor.
+		/// </summary>
+		public NoPreEvaluationAttribute()
+			: base(LinqExtensionPreEvaluation.NoEvaluation) { }
+	}
+
+	/// <summary>
+	/// Base class for Linq extension attributes.
+	/// </summary>
+	public abstract class LinqExtensionMethodAttributeBase : Attribute
+	{
+		/// <summary>
+		/// Should the method call be pre-evaluated when not depending on queried data? If it can,
+		/// it would then be evaluated and replaced by the resulting (parameterized) constant expression 
+		/// in the resulting SQL query.
+		/// </summary>
+		public LinqExtensionPreEvaluation PreEvaluation { get; }
+
+		/// <summary>
+		/// Default constructor.
+		/// </summary>
+		/// <param name="preEvaluation">Should the method call be pre-evaluated when not depending on queried data?</param>
+		protected LinqExtensionMethodAttributeBase(LinqExtensionPreEvaluation preEvaluation)
+		{
+			PreEvaluation = preEvaluation;
+		}
+	}
+
+	/// <summary>
+	/// Possible method call behaviors when the linq to NHibernate provider pre-evaluates
+	/// expressions before translating them to SQL.
+	/// </summary>
+	public enum LinqExtensionPreEvaluation
+	{
+		/// <summary>
+		/// The method call will not be evaluated even if its arguments do not depend on queried data.
+		/// It will always be translated to the corresponding SQL statement.
+		/// </summary>
+		NoEvaluation,
+		/// <summary>
+		/// If the method call does not depend on queried data, the method call will be evaluated and replaced
+		/// by the resulting (parameterized) constant expression in the resulting SQL query. A throwing
+		/// method implementation will cause the query to throw.
+		/// </summary>
+		AllowPreEvaluation
 	}
 }

src/NHibernate/Linq/LinqExtensionMethods.cs

@@ -121,6 +121,15 @@ public static IQueryable<T> CacheRegion<T>(this IQueryable<T> query, string regi
 		[Obsolete("Please use SetOptions instead.")]
 		public static IQueryable<T> Timeout<T>(this IQueryable<T> query, int timeout)
 			=> query.SetOptions(o => o.SetTimeout(timeout));
+
+		/// <summary>
+		/// Allows to specify the parameter NHibernate type to use for a literal in a queryable expression.
+		/// </summary>
+		/// <typeparam name="T">The type of the literal.</typeparam>
+		/// <param name="parameter">The literal value.</param>
+		/// <param name="type">The NHibernate type, usually obtained from <c>NHibernateUtil</c> properties.</param>
+		/// <returns>The literal value.</returns>
+		[NoPreEvaluation]
 		public static T MappedAs<T>(this T parameter, IType type)
 		{
 			throw new InvalidOperationException("The method should be used inside Linq to indicate a type of a parameter");

src/NHibernate/Linq/SqlMethods.cs

@@ -11,6 +11,7 @@ public static class SqlMethods
 		/// will be translated.) This method can only be used in Linq2NHibernate expressions, and will throw
 		/// if called directly.
 		/// </summary>
+		[NoPreEvaluation]
 		public static bool Like(this string matchExpression, string sqlLikePattern)
 		{
 			throw new NotSupportedException(
@@ -24,6 +25,7 @@ public static bool Like(this string matchExpression, string sqlLikePattern)
 		/// will be translated.) This method can only be used in Linq2NHibernate expressions, and will throw
 		/// if called directly.
 		/// </summary>
+		[NoPreEvaluation]
 		public static bool Like(this string matchExpression, string sqlLikePattern, char escapeCharacter)
 		{
 			throw new NotSupportedException(

src/NHibernate/Linq/Visitors/NhPartialEvaluatingExpressionVisitor.cs

@@ -1,3 +1,5 @@
+using System;
+using System.Linq;
 using System.Linq.Expressions;
 using Remotion.Linq.Clauses.Expressions;
 using Remotion.Linq.Parsing;
@@ -21,17 +23,30 @@ protected override Expression VisitConstant(ConstantExpression expression)
 
 		public static Expression EvaluateIndependentSubtrees(Expression expression)
 		{
-			var evaluatedExpression = PartialEvaluatingExpressionVisitor.EvaluateIndependentSubtrees(expression, new NullEvaluatableExpressionFilter());
+			var evaluatedExpression = PartialEvaluatingExpressionVisitor.EvaluateIndependentSubtrees(expression, new NhEvaluatableExpressionFilter());
 			return new NhPartialEvaluatingExpressionVisitor().Visit(evaluatedExpression);
 		}
 
-		public Expression VisitPartialEvaluationException(PartialEvaluationExceptionExpression expression)
+		public Expression VisitPartialEvaluationException(PartialEvaluationExceptionExpression partialEvaluationExceptionExpression)
 		{
-			return Visit(expression.Reduce());
+			throw new HibernateException(
+				$"Evaluation failure on {partialEvaluationExceptionExpression.EvaluatedExpression}",
+				partialEvaluationExceptionExpression.Exception);
 		}
 	}
 
-	internal class NullEvaluatableExpressionFilter : EvaluatableExpressionFilterBase
+	internal class NhEvaluatableExpressionFilter : EvaluatableExpressionFilterBase
 	{
+		public override bool IsEvaluatableMethodCall(MethodCallExpression node)
+		{
+			if (node == null)
+				throw new ArgumentNullException(nameof(node));
+
+			var attributes = node.Method
+				.GetCustomAttributes(typeof(LinqExtensionMethodAttributeBase), false)
+				.Cast<LinqExtensionMethodAttributeBase>().ToArray();
+			return attributes.Length == 0 ||
+				attributes.Any(a => a.PreEvaluation == LinqExtensionPreEvaluation.AllowPreEvaluation);
+		}
 	}
 }