From 16ff0c33173836ff44852091570def376dcfb947 Mon Sep 17 00:00:00 2001 From: CSharper2010 Date: Wed, 26 Feb 2025 17:12:33 +0100 Subject: [PATCH 1/2] Fixes #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 --- .../NHSpecificTest/GH2614/Fixture.cs | 68 ++++++++++++++++++- src/NHibernate/Engine/Query/HQLQueryPlan.cs | 36 +++++++--- .../Hql/Ast/ANTLR/QueryTranslatorImpl.cs | 9 +++ src/NHibernate/Hql/IQueryTranslator.cs | 2 + 4 files changed, 102 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs index 82876011bae..2ffb1f99cda 100644 --- 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 index a3a83f1c9c4..b724e7ba58b 100644 --- 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(); var distinction = new HashSet(ReferenceComparer.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 index a318e9be5b5..27d5c351ba1 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs +++ b/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 UncacheableCollectionPersisters { get diff --git a/src/NHibernate/Hql/IQueryTranslator.cs b/src/NHibernate/Hql/IQueryTranslator.cs index 93fee2aaf80..6156b7d5fda 100644 --- a/src/NHibernate/Hql/IQueryTranslator.cs +++ b/src/NHibernate/Hql/IQueryTranslator.cs @@ -113,6 +113,8 @@ public partial interface IQueryTranslator /// True if the query does contain collection fetched; false otherwise. bool ContainsCollectionFetches { get; } + bool ContainsOrderByClause { get; } + bool IsManipulationStatement { get; } // 6.0 TODO : replace by ILoader QueryLoader. From 2d8ee368fa007cc2336f1bda39000ce6d14f9529 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 26 Feb 2025 16:19:26 +0000 Subject: [PATCH 2/2] Generate async files --- .../Async/NHSpecificTest/GH2614/Fixture.cs | 68 ++++++++++++++++++- .../Async/Engine/Query/HQLQueryPlan.cs | 36 +++++++--- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH2614/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH2614/Fixture.cs index 2b7c0006da0..34a2027dfc3 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/GH2614/Fixture.cs +++ b/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)); } } } diff --git a/src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs b/src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs index 134e4fa7e9d..2a035f085dc 100644 --- a/src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs +++ b/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(); var distinction = new HashSet(ReferenceComparer.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 PerformIterateAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)