Improve IQuery.Enumerable

[bahusoid]

Do not open multiple readers when query has multiple translators (see more details here)

It fixes MySql Query test under .NET Core (as there is a bug with handling multiple readers)
Also added proper dispose logic to Query test (so this part better review with whitespace changes disabled)

Possible breaking changes:

1. IQuery.Enumerable call no longer opens reader until is actually enumerated.
2. In case polymorphic queries. Old behavior: all readers are opened at once. New behavior: next reader is opened when previous is processed and closed.

[hazzik]

MySQL fails because of bugs in the provider.

[bahusoid]

MySQL fails because of bugs in the provider.

But it exposed an issue in our Enumerable implementation. QueryLoader.GetEnumerable opens a reader when called:

nhibernate-core/src/NHibernate/Loader/Hql/QueryLoader.cs

Line 441 in 109f085

var rs = GetResultSet(cmd, queryParameters, session, null);

And opening readers for all translators doesn't look like a good idea to me:

nhibernate-core/src/NHibernate/Engine/Query/HQLQueryPlan.cs

Lines 169 to 174 in 109f085

	for (int i = 0; i < Translators.Length; i++)
	{
	var result = Translators[i].GetEnumerable(queryParameters, session);
	results[i] = result;
	}
	return new JoinedEnumerable(results);

So my latest commit fixes it and MySql I believe won't fail for Query test anymore.

[hazzik]

Why?

[bahusoid]

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.

[fredericDelaporte]

Just for reference, the MySql bug causing those Enumerable tests to fail is here.

This MySQL bug causes unexpected readers to get closed by MySQL with the current implementation of Enumerable. This PR proposal avoids being exposed to that MySQL bug.

Its main purpose is to reduce the number of cases where multiple readers are opened simultaneously on the same connection. For databases not supporting it, avoids having them copied into memory by NHibernate.
But I wonder if this may have some averse effects due to delaying the time where subsequent readers are opened: they may see data changes done after the call to Enumerable but before fully enumerating it, while I guess that with most databases, opened readers yield some kind of snapshot of the data at the time they were opened.

About the async generation change, I do not think it should be done as part of this PR.

[bahusoid]

For databases not supporting it, avoids having them copied into memory by NHibernate.

I would say It makes Enumerable method usable for such cases. As the main purpose for separate Enumerable method is to avoid full query loading at once.

About the async generation change, I do not think it should be done as part of this PR.

This PR didn't change async generation. I've added rule for the newly added method.

But I wonder if this may have some adverse effects due to delaying the time where subsequent readers are opened

Yeah. That might be true. But as I understand the main purpose of this method is to process huge result sets - it's unlikely it's called under strict isolation levels. But that's just my speculations...

[bahusoid]

Closed in favor of #2274