Skip to content

Commit 16ff0c3

Browse files
committed
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
1 parent f5a7a1f commit 16ff0c3

File tree

4 files changed

+102
-13
lines changed

4 files changed

+102
-13
lines changed

Diff for: src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs

+65-3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@ protected override void OnSetUp()
1010
using (var s = OpenSession())
1111
using (var t = s.BeginTransaction())
1212
{
13-
s.Save(new ConcreteClass1 {Name = "C1"});
14-
s.Save(new ConcreteClass2 {Name = "C2"});
13+
s.Save(new ConcreteClass1 {Name = "C11"});
14+
s.Save(new ConcreteClass1 {Name = "C12"});
15+
s.Save(new ConcreteClass2 {Name = "C21"});
16+
s.Save(new ConcreteClass2 {Name = "C22"});
17+
s.Save(new ConcreteClass2 {Name = "C23"});
18+
s.Save(new ConcreteClass2 {Name = "C24"});
1519
t.Commit();
1620
}
1721
}
@@ -34,9 +38,67 @@ public void PolymorphicListReturnsCorrectResults()
3438
{
3539
var query = s.CreateQuery(
3640
@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
41+
query.SetMaxResults(10);
42+
var list = query.List();
43+
Assert.That(list.Count, Is.EqualTo(6));
44+
}
45+
}
46+
47+
[Test]
48+
public void PolymorphicListWithSmallMaxResultsReturnsCorrectResults()
49+
{
50+
using (var s = OpenSession())
51+
using (s.BeginTransaction())
52+
{
53+
var query = s.CreateQuery(
54+
@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
55+
query.SetMaxResults(1);
56+
var list = query.List();
57+
Assert.That(list.Count, Is.EqualTo(1));
58+
}
59+
}
60+
61+
[Test]
62+
public void PolymorphicListWithSkipReturnsCorrectResults()
63+
{
64+
using (var s = OpenSession())
65+
using (s.BeginTransaction())
66+
{
67+
var query = s.CreateQuery(
68+
@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
69+
query.SetFirstResult(5);
3770
query.SetMaxResults(5);
3871
var list = query.List();
39-
Assert.That(list.Count, Is.EqualTo(2));
72+
Assert.That(list.Count, Is.EqualTo(1));
73+
}
74+
}
75+
76+
[Test]
77+
public void PolymorphicListWithSkipManyReturnsCorrectResults()
78+
{
79+
using (var s = OpenSession())
80+
using (s.BeginTransaction())
81+
{
82+
var query = s.CreateQuery(
83+
@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT");
84+
query.SetFirstResult(6);
85+
query.SetMaxResults(5);
86+
var list = query.List();
87+
Assert.That(list.Count, Is.EqualTo(0));
88+
}
89+
}
90+
91+
[Test]
92+
public void PolymorphicListWithOrderByStillShowsWarning()
93+
{
94+
using (var s = OpenSession())
95+
using (s.BeginTransaction())
96+
{
97+
var query = s.CreateQuery(
98+
@"SELECT Name FROM NHibernate.Test.NHSpecificTest.GH2614.BaseClass ROOT ORDER BY ROOT.Name");
99+
query.SetMaxResults(3);
100+
var list = query.List();
101+
Assert.That(list.Count, Is.EqualTo(3));
40102
}
41103
}
42104
}

Diff for: src/NHibernate/Engine/Query/HQLQueryPlan.cs

+26-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
using System;
22
using System.Collections;
33
using System.Collections.Generic;
4-
4+
using System.Linq;
55
using NHibernate.Event;
66
using NHibernate.Hql;
7-
using NHibernate.Linq;
87
using NHibernate.Type;
98
using NHibernate.Util;
109

@@ -96,8 +95,15 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
9695
QueryParameters queryParametersToUse;
9796
if (needsLimit)
9897
{
99-
Log.Warn("firstResult/maxResults specified on polymorphic query; applying in memory!");
98+
if (Translators.Any(t => t.ContainsOrderByClause))
99+
// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
100+
Log.Warn("firstResult/maxResults specified on polymorphic query with order by; applying in memory!");
101+
else if (queryParameters.RowSelection.FirstRow > 0)
102+
// in memory evaluation is only problematic if items are skipped or if there is an order by clause thus correctness is not ensured
103+
Log.Warn("firstResult specified on polymorphic query; applying in memory!");
104+
100105
RowSelection selection = new RowSelection();
106+
UpdateRowSelection(selection, alreadyIncluded: 0);
101107
selection.FetchSize = queryParameters.RowSelection.FetchSize;
102108
selection.Timeout = queryParameters.RowSelection.Timeout;
103109
queryParametersToUse = queryParameters.CreateCopyUsing(selection);
@@ -109,7 +115,7 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
109115

110116
IList combinedResults = results ?? new List<object>();
111117
var distinction = new HashSet<object>(ReferenceComparer<object>.Instance);
112-
int includedCount = -1;
118+
int includedCount = 0;
113119
for (int i = 0; i < Translators.Length; i++)
114120
{
115121
IList tmp = Translators[i].List(session, queryParametersToUse);
@@ -120,9 +126,7 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
120126
? 0
121127
: queryParameters.RowSelection.FirstRow;
122128

123-
int max = queryParameters.RowSelection.MaxRows == RowSelection.NoValue
124-
? RowSelection.NoValue
125-
: queryParameters.RowSelection.MaxRows;
129+
int max = queryParametersToUse.RowSelection.MaxRows;
126130

127131
int size = tmp.Count;
128132
for (int x = 0; x < size; x++)
@@ -132,22 +136,34 @@ public void PerformList(QueryParameters queryParameters, ISessionImplementor ses
132136
{
133137
continue;
134138
}
135-
includedCount++;
136-
if (includedCount < first)
139+
if (includedCount++ < first)
137140
{
138141
continue;
139142
}
140143
combinedResults.Add(result);
141-
if (max >= 0 && includedCount > max)
144+
if (max != RowSelection.NoValue && includedCount >= max)
142145
{
143146
// break the outer loop !!!
144147
return;
145148
}
146149
}
150+
151+
UpdateRowSelection(queryParametersToUse.RowSelection, includedCount);
147152
}
148153
else
149154
ArrayHelper.AddAll(combinedResults, tmp);
150155
}
156+
157+
void UpdateRowSelection(RowSelection selection, int alreadyIncluded)
158+
{
159+
if (queryParameters.RowSelection.MaxRows != RowSelection.NoValue)
160+
{
161+
if (queryParameters.RowSelection.FirstRow > 0)
162+
selection.MaxRows = Math.Max(0, queryParameters.RowSelection.FirstRow + queryParameters.RowSelection.MaxRows - alreadyIncluded);
163+
else
164+
selection.MaxRows = Math.Max(0, queryParameters.RowSelection.MaxRows - alreadyIncluded);
165+
}
166+
}
151167
}
152168

153169
public IEnumerable PerformIterate(QueryParameters queryParameters, IEventSource session)

Diff for: src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs

+9
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,15 @@ public bool ContainsCollectionFetches
341341
}
342342
}
343343

344+
public bool ContainsOrderByClause
345+
{
346+
get
347+
{
348+
ErrorIfDML();
349+
return ((QueryNode)_sqlAst).GetOrderByClause()?.ChildCount > 0;
350+
}
351+
}
352+
344353
public ISet<ICollectionPersister> UncacheableCollectionPersisters
345354
{
346355
get

Diff for: src/NHibernate/Hql/IQueryTranslator.cs

+2
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ public partial interface IQueryTranslator
113113
/// <returns>True if the query does contain collection fetched; false otherwise.</returns>
114114
bool ContainsCollectionFetches { get; }
115115

116+
bool ContainsOrderByClause { get; }
117+
116118
bool IsManipulationStatement { get; }
117119

118120
// 6.0 TODO : replace by ILoader QueryLoader.

0 commit comments

Comments
 (0)