Skip to content
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

Consider making DbDataReader implement IAsyncEnumerable #25298

Closed
roji opened this issue Mar 3, 2018 · 4 comments
Closed

Consider making DbDataReader implement IAsyncEnumerable #25298

roji opened this issue Mar 3, 2018 · 4 comments

Comments

@roji
Copy link
Member

roji commented Mar 3, 2018

DbDataReader is used to stream rows from a database. Although it does implement IEnumerable (of DbDataRecord), that API is old and rarely-used (it's non-generic, sync-only and quite inefficient).

Consider making DbDataReader implement IAsyncEnumerable, to provide a modern and efficient API for going through the rows. This would simply be a layer implemented on top of DbDataReader's existing async operations (e.g. ReadAsync()), so it wouldn't require any sort of change from providers.

@davidfowl
Copy link
Member

/cc @MadsTorgersen @stephentoub

@divega
Copy link
Contributor

divega commented Mar 3, 2018

System.Data Triage: Moving to future since we are not doing this in 2.1.

If the timing of the introduction of IAsyncEnumerable is right, this could be the sole way we leverage ValueTask/IValueTaskSource for streaming rows in DbDataReader. Otherwise we may want to still have a ValueTask<bool> alternative for ReadAsync.

We should also understand if there are inherent inefficiencies in implementing IAsyncEnumerable at this layer, e.g. it seems we would be allocating a DbDataRecord (or similar) for each row, which in the most commonly used pattern for DbDataReader isn't necessary.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jun 23, 2020
@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Dec 23, 2024
@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Jan 6, 2025
@julealgon
Copy link

@roji isn't this still desirable?

@dotnet-policy-service dotnet-policy-service bot removed no-recent-activity backlog-cleanup-candidate An inactive issue that has been marked for automated closure. labels Jan 7, 2025
@roji
Copy link
Member Author

roji commented Jan 7, 2025

@julealgon I'm not completely sure. It has been a very long time since I originally opened this, there has been almost no interest (5 upvotes), and the new API would be far from trivial (IAsyncEnumerable of what exactly? If of DbDataRecord, that ancient type probably needs to be extended to support new things; if we also allocate a new one per row, that's a lot of allocations that don't happen using DbDataReader today, etc. etc.).

So I think it's OK to leave this closed for now until new interest/needs come up around it.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants