nhibernate / nhibernate-core

NHibernate Object Relational Mapper
https://nhibernate.info
GNU Lesser General Public License v2.1
2.13k stars 927 forks source link

Add support for IAsyncEnumerable #2267

Open cremor opened 4 years ago

cremor commented 4 years ago

C# 8.0 and .NET Standard 2.1 brought us asynchronous streams (horrible name, I'll call them "async enumerables" from now on). It would be nice if NHibernate could add new methods that return an IAsyncEnumerable<T>. This would allow us to iterate over the results with await foreach.

I know this won't be a simple task and will require a few preparation steps, like adding .NET Standard 2.1 (and maybe .NET Core 3.0) targets to the build process. (~Maybe this would even be the first time that NHibernate has a different API across target frameworks?~ edit: .NET Standard 2.0 and .NET Framework support IAsyncEnumerable with Microsoft.Bcl.AsyncInterfaces.) But I thought that this issue could at least be a tracking issue for the overall progress.

fredericDelaporte commented 4 years ago

I think it would have to be done for NHibernate 6.0, which may simply drop .Net Framework v4. (So the plan would then be to wait for .Net 5 and target this one for NHibernate 6.)

Otherwise we may end-up with a lot of conditional compilation directives in code, because there is also Span<T> and other features which will be tempting to use with 2.1.

cremor commented 4 years ago

Waiting for NHibernate 6.0 to add IAsyncEnumerable methods sounds reasonable.

OT: I'm not so sure about dropping both the .NET Framework and .NET Standard 2.0 targets, that would mean loosing many projects that use NHibernate today. In my opinion, at least .NET Standard 2.0 should stay a target framework, so that projects targeting a recent version of .NET Framework can still use NHibernate.

fredericDelaporte commented 4 years ago

Maybe there is a misunderstanding here: I do not see this as dropping .Net Framework, since we would target .Net 5, which is meant to be the successor of .Net Framework v4.8 (and of .Net Core 3 altogether). So that would be a change a bit like dropping support for .Net Framework v2/v3.5 in favor of v4. At some point we will have to do it anyway.

hazzik commented 4 years ago

We can start using C# 8 now and add support of the interface in the current version. The interfaces are available in the compatibility package.

To be able to do we need to deprecate ToEnumerableAsync (which is broken-ish anyway: it mixes async and sync access to the data provider which could lead to blocking) and make a new method ToAsyncEnumerable which will return IAsyncEnumerable interface.

hazzik commented 4 years ago

The only problem I see now: we need a support from @maca88’s AsyncGenerator so we can start using the new C#8 syntactic sugar

cremor commented 4 years ago

@hazzik Good point, I forgot about the compatability package (Microsoft.Bcl.AsyncInterfaces). Thanks for mentioning it! With the help of this package IAsyncEnumerable methods could indeed be supported for .NET Standard 2.0 and .NET Framework down to 4.6.1.

@fredericDelaporte Dropping .NET Framework 2.0/3.5 was not a huge deal because updating a solution from 2.0/3.5 to 4.0 was as easy as changing the target framework most of the time. In contrast, switching from .NET Framework to .NET 5 will be at least as much work as it is now to switch to .NET Standard/.NET Core.

I know that .NET Framework support has to be dropped sometime. But I wouldn't do it in the foreseeable future.

maca88 commented 4 years ago

we need a support from @maca88’s AsyncGenerator so we can start using the new C#8 syntactic sugar

By adding Microsoft.Bcl.AsyncInterfaces we do not gain the ability to use C#8 syntactic sugars like await foreach, so the only thing that the generator could do is to modify the return type. By modifying the return type, the generator would need to transform the creation of the IEnumerable that is returned and all its usages, which is not that easy as there are many possible ways of creating and using an IEnumerable. Here is my attempt to add IAsyncEnumerable without modifying the generator.

hazzik commented 4 years ago

By adding Microsoft.Bcl.AsyncInterfaces we do not gain the ability to use C#8 syntactic sugars like await foreach

Obviously not. What I mean is that we need AsyncGenerator support to start using C#8 regardless of async enumerables feature.

maca88 commented 4 years ago

What I mean is that we need AsyncGenerator support to start using C#8 regardless of async enumerables feature.

I agreee, my current plan is to start working on it after 5.3 release.

cremor commented 3 years ago

I understand that this might still be far away and not finally decided, but is this feature still planned for NHibernate 6.0? If yes, could someone please assign it to the "next major" milestone?

maca88 commented 3 years ago

is this feature still planned for NHibernate 6.0?

It all depends on scroll feature from Hibernate that has to be ported for #2274 to work properly. I will try to port the scroll feature in the following weeks so that this could be included in the next minor version instead.

aokellermann commented 2 years ago

what is the status of this feature?

kalimuthuswamy commented 1 year ago

what is the status of this feature?