Fixes #3654: small enhancement for polymorphic query evaluation

src/NHibernate.Test/Async/NHSpecificTest/GH2614/Fixture.cs

@@ -21,8 +21,12 @@ protected override void OnSetUp()
 			using (var s = OpenSession())
 			using (var t = s.BeginTransaction())
 			{
-				s.Save(new ConcreteClass1 {Name = "C1"});
-				s.Save(new ConcreteClass2 {Name = "C2"});
+				s.Save(new ConcreteClass1 {Name = "C11"});
+				s.Save(new ConcreteClass1 {Name = "C12"});
+				s.Save(new ConcreteClass2 {Name = "C21"});
+				s.Save(new ConcreteClass2 {Name = "C22"});
+				s.Save(new ConcreteClass2 {Name = "C23"});
+				s.Save(new ConcreteClass2 {Name = "C24"});
 				t.Commit();
 			}
 		}
@@ -45,9 +49,67 @@ public async Task PolymorphicListReturnsCorrectResultsAsync()
 			{
 				var query = s.CreateQuery(
 					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetMaxResults(10);
+				var list = await (query.ListAsync());
+				Assert.That(list.Count, Is.EqualTo(6));
+			}
+		}
+
+		[Test]
+		public async Task PolymorphicListWithSmallMaxResultsReturnsCorrectResultsAsync()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetMaxResults(1);
+				var list = await (query.ListAsync());
+				Assert.That(list.Count, Is.EqualTo(1));
+			}
+		}
+
+		[Test]
+		public async Task PolymorphicListWithSkipReturnsCorrectResultsAsync()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetFirstResult(5);
 				query.SetMaxResults(5);
 				var list = await (query.ListAsync());
-				Assert.That(list.Count, Is.EqualTo(2));
+				Assert.That(list.Count, Is.EqualTo(1));
+			}
+		}
+
+		[Test]
+		public async Task PolymorphicListWithSkipManyReturnsCorrectResultsAsync()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetFirstResult(6);
+				query.SetMaxResults(5);
+				var list = await (query.ListAsync());
+				Assert.That(list.Count, Is.EqualTo(0));
+			}
+		}
+
+		[Test]
+		public async Task PolymorphicListWithOrderByStillShowsWarningAsync()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT ORDER BY ROOT.Name");
+				query.SetMaxResults(3);
+				var list = await (query.ListAsync());
+				Assert.That(list.Count, Is.EqualTo(3));
 			}
 		}
 	}

src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs

@@ -10,8 +10,12 @@ protected override void OnSetUp()
 			using (var s = OpenSession())
 			using (var t = s.BeginTransaction())
 			{
-				s.Save(new ConcreteClass1 {Name = "C1"});
-				s.Save(new ConcreteClass2 {Name = "C2"});
+				s.Save(new ConcreteClass1 {Name = "C11"});
+				s.Save(new ConcreteClass1 {Name = "C12"});
+				s.Save(new ConcreteClass2 {Name = "C21"});
+				s.Save(new ConcreteClass2 {Name = "C22"});
+				s.Save(new ConcreteClass2 {Name = "C23"});
+				s.Save(new ConcreteClass2 {Name = "C24"});
 				t.Commit();
 			}
 		}
@@ -34,9 +38,67 @@ public void PolymorphicListReturnsCorrectResults()
 			{
 				var query = s.CreateQuery(
 					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetMaxResults(10);
+				var list = query.List();
+				Assert.That(list.Count, Is.EqualTo(6));
+			}
+		}
+
+		[Test]
+		public void PolymorphicListWithSmallMaxResultsReturnsCorrectResults()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetMaxResults(1);
+				var list = query.List();
+				Assert.That(list.Count, Is.EqualTo(1));
+			}
+		}
+
+		[Test]
+		public void PolymorphicListWithSkipReturnsCorrectResults()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetFirstResult(5);
 				query.SetMaxResults(5);
 				var list = query.List();
-				Assert.That(list.Count, Is.EqualTo(2));
+				Assert.That(list.Count, Is.EqualTo(1));
+			}
+		}
+
+		[Test]
+		public void PolymorphicListWithSkipManyReturnsCorrectResults()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
+				query.SetFirstResult(6);
+				query.SetMaxResults(5);
+				var list = query.List();
+				Assert.That(list.Count, Is.EqualTo(0));
+			}
+		}
+
+		[Test]
+		public void PolymorphicListWithOrderByStillShowsWarning()
+		{
+			using (var s = OpenSession())
+			using (s.BeginTransaction())
+			{
+				var query = s.CreateQuery(
+					@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT ORDER BY ROOT.Name");
+				query.SetMaxResults(3);
+				var list = query.List();
+				Assert.That(list.Count, Is.EqualTo(3));
 			}
 		}
 	}

src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs

@@ -11,10 +11,9 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-
+using System.Linq;
 using NHibernate.Event;
 using NHibernate.Hql;
-using NHibernate.Linq;
 using NHibernate.Type;
 using NHibernate.Util;
 
@@ -46,8 +45,15 @@ public async Task PerformListAsync(QueryParameters queryParameters, ISessionImpl
 			QueryParameters queryParametersToUse;
 			if (needsLimit)
 			{
-				Log.Warn("firstResult/maxResults specified on polymorphic query; applying in memory!");
+				if (Translators.Any(t => t.ContainsOrderByClause))
+					// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
+					Log.Warn("firstResult/maxResults specified on polymorphic query with order by; applying in memory!");
+				else if (queryParameters.RowSelection.FirstRow > 0)
+					// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
+					Log.Warn("firstResult specified on polymorphic query; applying in memory!");
+
 				RowSelection selection = new RowSelection();
+				UpdateRowSelection(selection, alreadyIncluded: 0);
 				selection.FetchSize = queryParameters.RowSelection.FetchSize;
 				selection.Timeout = queryParameters.RowSelection.Timeout;
 				queryParametersToUse = queryParameters.CreateCopyUsing(selection);
@@ -59,7 +65,7 @@ public async Task PerformListAsync(QueryParameters queryParameters, ISessionImpl
 
 			IList combinedResults = results ?? new List<object>();
 			var distinction = new HashSet<object>(ReferenceComparer<object>.Instance);
-			int includedCount = -1;
+			int includedCount = 0;
 			for (int i = 0; i < Translators.Length; i++)
 			{
 				IList tmp = await (Translators[i].ListAsync(session, queryParametersToUse, cancellationToken)).ConfigureAwait(false);
@@ -70,9 +76,7 @@ public async Task PerformListAsync(QueryParameters queryParameters, ISessionImpl
 												? 0
 												: queryParameters.RowSelection.FirstRow;
 
-					int max = queryParameters.RowSelection.MaxRows == RowSelection.NoValue
-											? RowSelection.NoValue
-											: queryParameters.RowSelection.MaxRows;
+					int max = queryParametersToUse.RowSelection.MaxRows;
 
 					int size = tmp.Count;
 					for (int x = 0; x < size; x++)
@@ -82,22 +86,34 @@ public async Task PerformListAsync(QueryParameters queryParameters, ISessionImpl
 						{
 							continue;
 						}
-						includedCount++;
-						if (includedCount < first)
+						if (includedCount++ < first)
 						{
 							continue;
 						}
 						combinedResults.Add(result);
-						if (max >= 0 && includedCount > max)
+						if (max != RowSelection.NoValue && includedCount >= max)
 						{
 							// break the outer loop !!!
 							return;
 						}
 					}
+
+					UpdateRowSelection(queryParametersToUse.RowSelection, includedCount);
 				}
 				else
 					ArrayHelper.AddAll(combinedResults, tmp);
 			}
+
+			void UpdateRowSelection(RowSelection selection, int alreadyIncluded)
+			{
+				if (queryParameters.RowSelection.MaxRows != RowSelection.NoValue)
+				{
+					if (queryParameters.RowSelection.FirstRow > 0)
+						selection.MaxRows = Math.Max(0, queryParameters.RowSelection.FirstRow + queryParameters.RowSelection.MaxRows - alreadyIncluded);
+					else
+						selection.MaxRows = Math.Max(0, queryParameters.RowSelection.MaxRows - alreadyIncluded);
+				}
+			}
 		}
 
 		public async Task<IEnumerable> PerformIterateAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)

src/NHibernate/Engine/Query/HQLQueryPlan.cs

@@ -1,10 +1,9 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-
+using System.Linq;
 using NHibernate.Event;
 using NHibernate.Hql;
-using NHibernate.Linq;
 using NHibernate.Type;
 using NHibernate.Util;
 
@@ -96,8 +95,15 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
 			QueryParameters queryParametersToUse;
 			if (needsLimit)
 			{
-				Log.Warn("firstResult/maxResults specified on polymorphic query; applying in memory!");
+				if (Translators.Any(t => t.ContainsOrderByClause))
+					// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
+					Log.Warn("firstResult/maxResults specified on polymorphic query with order by; applying in memory!");
+				else if (queryParameters.RowSelection.FirstRow > 0)
+					// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
+					Log.Warn("firstResult specified on polymorphic query; applying in memory!");
+
 				RowSelection selection = new RowSelection();
+				UpdateRowSelection(selection, alreadyIncluded: 0);
 				selection.FetchSize = queryParameters.RowSelection.FetchSize;
 				selection.Timeout = queryParameters.RowSelection.Timeout;
 				queryParametersToUse = queryParameters.CreateCopyUsing(selection);
@@ -109,7 +115,7 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
 
 			IList combinedResults = results ?? new List<object>();
 			var distinction = new HashSet<object>(ReferenceComparer<object>.Instance);
-			int includedCount = -1;
+			int includedCount = 0;
 			for (int i = 0; i < Translators.Length; i++)
 			{
 				IList tmp = Translators[i].List(session, queryParametersToUse);
@@ -120,9 +126,7 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
 												? 0
 												: queryParameters.RowSelection.FirstRow;
 
-					int max = queryParameters.RowSelection.MaxRows == RowSelection.NoValue
-											? RowSelection.NoValue
-											: queryParameters.RowSelection.MaxRows;
+					int max = queryParametersToUse.RowSelection.MaxRows;
 
 					int size = tmp.Count;
 					for (int x = 0; x < size; x++)
@@ -132,22 +136,34 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
 						{
 							continue;
 						}
-						includedCount++;
-						if (includedCount < first)
+						if (includedCount++ < first)
 						{
 							continue;
 						}
 						combinedResults.Add(result);
-						if (max >= 0 && includedCount > max)
+						if (max != RowSelection.NoValue && includedCount >= max)
 						{
 							// break the outer loop !!!
 							return;
 						}
 					}
+
+					UpdateRowSelection(queryParametersToUse.RowSelection, includedCount);
 				}
 				else
 					ArrayHelper.AddAll(combinedResults, tmp);
 			}
+
+			void UpdateRowSelection(RowSelection selection, int alreadyIncluded)
+			{
+				if (queryParameters.RowSelection.MaxRows != RowSelection.NoValue)
+				{
+					if (queryParameters.RowSelection.FirstRow > 0)
+						selection.MaxRows = Math.Max(0, queryParameters.RowSelection.FirstRow + queryParameters.RowSelection.MaxRows - alreadyIncluded);
+					else
+						selection.MaxRows = Math.Max(0, queryParameters.RowSelection.MaxRows - alreadyIncluded);
+				}
+			}
 		}
 
 		public IEnumerable PerformIterate(QueryParameters queryParameters, IEventSource session)

src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs

@@ -341,6 +341,15 @@ public bool ContainsCollectionFetches
 			}
 		}
 
+		public bool ContainsOrderByClause
+		{
+			get
+			{
+				ErrorIfDML();
+				return ((QueryNode)_sqlAst).GetOrderByClause()?.ChildCount > 0;
+			}
+		}
+
 		public ISet<ICollectionPersister> UncacheableCollectionPersisters
 		{
 			get

src/NHibernate/Hql/IQueryTranslator.cs

@@ -113,6 +113,8 @@ public partial interface IQueryTranslator
 		/// <returns>True if the query does contain collection fetched; false otherwise.</returns>
 		bool ContainsCollectionFetches { get; }
 
+		bool ContainsOrderByClause { get; }
+
 		bool IsManipulationStatement { get; }
 
 		// 6.0 TODO : replace by ILoader QueryLoader.