Have IFutureEnumerable.GetEnumerable executing immediatly the query

[fredericDelaporte]

As raised in #1663, IFutureEnumerable.GetEnumerable does not actually execute the query, it is still a deferred IEnumerable. This contradicts its purpose and xml comment:

Synchronously triggers the future query and all other pending future if the query was not already resolved, then returns a non-deferred enumerable of the query resulting items.

As this member has been introduced in 5.0, this PR targets the 5.0.x branch.

Update: the async file has been generated with the 0.8.2.1 AsyncGenerator instead of the 0.6.0, which cannot run with a recent MSBuild.

[hazzik]

Are you going to release 5.0.5?

[fredericDelaporte]

I am not decided yet. 5.0.4 has just been shipped, of course if I had known I would have delayed it slightly for putting this into it. What do you think? Do I add a release commit here and proceed with release?

[hazzik]

@fredericDelaporte you have closed #1663. Do you still intend to release this fix? Is it required?

Do I add a release commit here

Yes,

and proceed with release?

As you wish - depends on your decision.

[fredericDelaporte]

@fredericDelaporte you have closed #1663. Do you still intend to release this fix? Is it required?

I was forgetting this question. #1663 was about another subject on Future, for which I was telling to solve it on user side by calling GetEnumerable. But as GetEnumerable was not working as expected, it was not enough. The user came back about that point, which is now fixed with its own PR (not a "t: fix").

[fredericDelaporte]

From a #1718 comment:

@bahusoid:

delayed IEnumerable interface for IFutureEnumerable is obsoleted and suggested replacement is to use "non delayed" GetEnumerable call

This change has been made when implementing async. Implementing directly IEnumerable with an iterator triggering IO bound operations does not allow for an async counterpart. That is why the IO bound operations have been moved to the GetEnumerable method. Having it not delayed is required if willing to trigger the query for fetching entities in the session first level cache without directly using the query output (see #1663).

IMHO from this description it's not clear that suggested refactoring changes behavior

GetEnumerable() description is quite clear about what it does, although it what not doing it initially due to being still delayed (see #1665).

So I think that we still should provide a way for retrieving lazy enumerable.

This would need to be a new additional method.

[bahusoid]

Posting here my original concern to keep all details in one place:
I've just noticed that delayed IEnumerable interface for IFutureEnumerable is obsoleted and suggested replacement is to use "non delayed" GetEnumerable call:

nhibernate-core/src/NHibernate/IFutureEnumerable.cs

Lines 10 to 12 in dc71c7d

	/// <para>This interface is directly usable as a <see cref="IEnumerable{T}"/> for backward compatibility, but this will
	/// be dropped in a later version. Please get the <see cref="IEnumerable{T}"/> from <see cref="IFutureEnumerable{T}.GetEnumerable"/>
	/// or <see cref="IFutureEnumerable{T}.GetEnumerableAsync(CancellationToken)"/>.</para>

IMHO from this description it's not clear that suggested refactoring changes behavior:

public IEnumerable<TBase> GetLazyList()
{
return session.Query<SubParent1>().Future<TBase>().Concat(session.Query<SubParent2>().Future())
}

So lazy method above become non lazy after suggested refactoring. And the query above can't be re-written in lazy way without wrapping this code in some custom user lazy enumerator.
So I think that we still should provide a way for retrieving lazy enumerable.

[bahusoid]

GetEnumerable() description is quite clear about what it does, although it what not doing it initially due to being still delayed

But I'm not referring to this description.

Please get the <see cref="IEnumerable{T}"/> from <see cref="IFutureEnumerable{T}.GetEnumerable"/>

Pretty straightforward suggestion. I would not expect that suggestion to use something else also changes initial behavior (so it's unlikely I will double check comments on GetEnumerable())

[fredericDelaporte]

Then propose a PR for adding an additional deferred method and changing the obsolete message.