Fixes nhibernate#3654:

HQLQueryPlan for polymorphic query: * Calculate a row selection in with the remaining number of rows as MaxRows * Warn only in cases where correctness or performance may suffer * Make includedCount zero-based
csharper2010 · Feb 26, 2025 · 39d6f57 · 39d6f57
1 parent 7075b02
commit 39d6f57
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 13 deletions.
diff --git a/src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs b/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));
 			}
 		}
 	}

diff --git a/src/NHibernate/Engine/Query/HQLQueryPlan.cs b/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)

diff --git a/src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs b/src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs
@@ -315,6 +315,15 @@ public bool ContainsCollectionFetches
 			}
 		}
 
+		public bool ContainsOrderByClause
+		{
+			get
+			{
+				ErrorIfDML();
+				return ((QueryNode)_sqlAst).GetOrderByClause()?.ChildCount > 0;
+			}
+		}
+
 		public ISet<ICollectionPersister> UncacheableCollectionPersisters
 		{
 			get

diff --git a/src/NHibernate/Hql/IQueryTranslator.cs b/src/NHibernate/Hql/IQueryTranslator.cs
@@ -111,6 +111,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; }
 
 		Loader.Loader Loader { get; }