Skip to content

Improve IQuery.Enumerable #2017

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

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions src/AsyncGenerator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
- conversion: Ignore
name: ProcessResults
containingTypeName: IQueryBatchItem
- conversion: Ignore
name: GetEnumerable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because proper async enumeration is not supported by Async Generator (and I guess won't be supported until C#8 with its IAsyncEnumerable). Generated code yields all results to List which kills the whole point of Enumerable call:

private async Task<IEnumerable<object>> GetEnumerableAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)
{
	cancellationToken.ThrowIfCancellationRequested();
	var yields = new List<object>();
	foreach (var t in Translators)
	{
		foreach (object obj in await (t.GetEnumerableAsync(queryParameters, session, cancellationToken)).ConfigureAwait(false))
		{
	yields.Add(obj);
		}
	}
	return yields;
}

Besides current EnumerableAsync methods are not async at all - the only async part is opening reader in query loader but iteration is synchronous.

containingTypeName: HQLQueryPlan
- conversion: Ignore
name: PostProcessInsert
containingTypeName: HqlSqlWalker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public async Task SerializationFailsOnAfterStatementAggressiveReleaseWithOpenRes
// to resources, which should make aggresive-Release not Release
// the connection (and thus cause serialization to fail)
IEnumerable en = await (s.CreateQuery("from Silly").EnumerableAsync());
en.GetEnumerator().MoveNext();

try
{
Expand Down
756 changes: 379 additions & 377 deletions src/NHibernate.Test/Async/Legacy/FooBarTest.cs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/NHibernate.Test/Async/Stats/StatsFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ public async Task QueryStatGatheringAsync()
Assert.AreEqual( continents, stats.QueryExecutionMaxTimeQueryString );

IEnumerable itr = await (s.CreateQuery(continents).EnumerableAsync());
itr.GetEnumerator().MoveNext();
// Enumerable() should increment the execution count
Assert.AreEqual(2, continentStats.ExecutionCount, "unexpected execution count");
// but should not effect the cumulative row count
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public void SerializationFailsOnAfterStatementAggressiveReleaseWithOpenResources
// to resources, which should make aggresive-Release not Release
// the connection (and thus cause serialization to fail)
IEnumerable en = s.CreateQuery("from Silly").Enumerable();
en.GetEnumerator().MoveNext();

try
{
Expand Down
756 changes: 379 additions & 377 deletions src/NHibernate.Test/Legacy/FooBarTest.cs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/NHibernate.Test/Stats/StatsFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public void QueryStatGathering()
Assert.AreEqual( continents, stats.QueryExecutionMaxTimeQueryString );

IEnumerable itr = s.CreateQuery(continents).Enumerable();
itr.GetEnumerator().MoveNext();
// Enumerable() should increment the execution count
Assert.AreEqual(2, continentStats.ExecutionCount, "unexpected execution count");
// but should not effect the cumulative row count
Expand Down
213 changes: 0 additions & 213 deletions src/NHibernate.Test/UtilityTest/JoinedEnumerableFixture.cs

This file was deleted.

37 changes: 15 additions & 22 deletions src/NHibernate/Async/Engine/Query/HQLQueryPlan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
using System;
using System.Collections;
using System.Collections.Generic;

using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using NHibernate.Event;
using NHibernate.Hql;
using NHibernate.Linq;
Expand All @@ -20,14 +22,10 @@

namespace NHibernate.Engine.Query
{
using System.Threading.Tasks;
using System.Threading;
public partial interface IQueryPlan
{
Task PerformListAsync(QueryParameters queryParameters, ISessionImplementor statelessSessionImpl, IList results, CancellationToken cancellationToken);
Task<int> PerformExecuteUpdateAsync(QueryParameters queryParameters, ISessionImplementor statelessSessionImpl, CancellationToken cancellationToken);
Task<IEnumerable<T>> PerformIterateAsync<T>(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken);
Task<IEnumerable> PerformIterateAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken);
}
public partial class HQLQueryPlan : IQueryPlan
{
Expand Down Expand Up @@ -99,35 +97,30 @@ public async Task PerformListAsync(QueryParameters queryParameters, ISessionImpl
ArrayHelper.AddAll(combinedResults, tmp);
}
}

public async Task<IEnumerable> PerformIterateAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)
// Since v5.3
[Obsolete("This method has no more usages and will be removed in a future version")]
public Task<IEnumerable> PerformIterateAsync(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
if (Log.IsDebugEnabled())
if (cancellationToken.IsCancellationRequested)
{
Log.Debug("enumerable: {0}", _sourceQuery);
queryParameters.LogParameters(session.Factory);
}
if (Translators.Length == 0)
{
return CollectionHelper.EmptyEnumerable;
return Task.FromCanceled<IEnumerable>(cancellationToken);
}
if (Translators.Length == 1)
try
{
return await (Translators[0].GetEnumerableAsync(queryParameters, session, cancellationToken)).ConfigureAwait(false);
return Task.FromResult<IEnumerable>(PerformIterate(queryParameters, session));
}
var results = new IEnumerable[Translators.Length];
for (int i = 0; i < Translators.Length; i++)
catch (Exception ex)
{
var result = await (Translators[i].GetEnumerableAsync(queryParameters, session, cancellationToken)).ConfigureAwait(false);
results[i] = result;
return Task.FromException<IEnumerable>(ex);
}
return new JoinedEnumerable(results);
}

// Since v5.3
[Obsolete("This method has no more usages and will be removed in a future version")]
public async Task<IEnumerable<T>> PerformIterateAsync<T>(QueryParameters queryParameters, IEventSource session, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
//TODO 6.0: Get rid of SafetyEnumerable and use Cast
return new SafetyEnumerable<T>(await (PerformIterateAsync(queryParameters, session, cancellationToken)).ConfigureAwait(false));
}

Expand Down
8 changes: 4 additions & 4 deletions src/NHibernate/Async/Impl/SessionImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public override async Task<IEnumerable<T>> EnumerableAsync<T>(IQueryExpression q

using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called
{
return await (plan.PerformIterateAsync<T>(queryParameters, this, cancellationToken)).ConfigureAwait(false);
return plan.PerformIterate<T>(queryParameters, this);
}
}
}
Expand All @@ -326,7 +326,7 @@ public override async Task<IEnumerable> EnumerableAsync(IQueryExpression queryEx

using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called
{
return await (plan.PerformIterateAsync(queryParameters, this, cancellationToken)).ConfigureAwait(false);
return plan.PerformIterate(queryParameters, this);
}
}
}
Expand Down Expand Up @@ -1107,7 +1107,7 @@ public override async Task<IEnumerable> EnumerableFilterAsync(object collection,
using (BeginProcess())
{
var plan = await (GetFilterQueryPlanAsync(collection, filter, queryParameters, true, cancellationToken)).ConfigureAwait(false);
return await (plan.PerformIterateAsync(queryParameters, this, cancellationToken)).ConfigureAwait(false);
return plan.PerformIterate(queryParameters, this);
}
}

Expand All @@ -1117,7 +1117,7 @@ public override async Task<IEnumerable<T>> EnumerableFilterAsync<T>(object colle
using (BeginProcess())
{
var plan = await (GetFilterQueryPlanAsync(collection, filter, queryParameters, true, cancellationToken)).ConfigureAwait(false);
return await (plan.PerformIterateAsync<T>(queryParameters, this, cancellationToken)).ConfigureAwait(false);
return plan.PerformIterate<T>(queryParameters, this);
}
}

Expand Down
1 change: 0 additions & 1 deletion src/NHibernate/Async/NHibernateUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using System;
using System.Collections;
using NHibernate.Collection;
using NHibernate.Impl;
using NHibernate.Intercept;
using NHibernate.Proxy;
using NHibernate.Type;
Expand Down
Loading