Skip to content

Fixes #3654: small enhancement for polymorphic query evaluation #3655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 65 additions & 3 deletions src/NHibernate.Test/Async/NHSpecificTest/GH2614/Fixture.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
68 changes: 65 additions & 3 deletions src/NHibernate.Test/NHSpecificTest/GH2614/Fixture.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
36 changes: 26 additions & 10 deletions src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs
Original file line number Diff line number Diff line change
@@ -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)
36 changes: 26 additions & 10 deletions src/NHibernate/Engine/Query/HQLQueryPlan.cs
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions src/NHibernate/Hql/Ast/ANTLR/QueryTranslatorImpl.cs
Original file line number Diff line number Diff line change
@@ -341,6 +341,15 @@ public bool ContainsCollectionFetches
}
}

public bool ContainsOrderByClause
{
get
{
ErrorIfDML();
return ((QueryNode)_sqlAst).GetOrderByClause()?.ChildCount > 0;
}
}

public ISet<ICollectionPersister> UncacheableCollectionPersisters
{
get
2 changes: 2 additions & 0 deletions src/NHibernate/Hql/IQueryTranslator.cs
Original file line number Diff line number Diff line change
@@ -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.