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 adding FillAsync and async APIs on data adapters #22109

Open
spudcud opened this issue Jun 3, 2017 · 109 comments
Open

Consider adding FillAsync and async APIs on data adapters #22109

spudcud opened this issue Jun 3, 2017 · 109 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Data enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@spudcud
Copy link

spudcud commented Jun 3, 2017

Core 2.0 preview 1 now supports SqlDataAdapter and DataTable which is great and the best way to fill a DataSet from a stored procedure is by using SqlDataAdapter.Fill(DataSet). This works perfectly when not used in an async environment. There is currently no way in Core or full .NET to do a FillAsync. Without a FillAsync which would take a single line of code now requires 20 lines of much more complicated code including using SqlDataReader.GetSchemaTable() which is currently broken preview 1. Based on other posts I believe it will be working in Preview 2 but have not seen a definitive on that.

This is not something for 2.0 but is a clear deficiency in truly being able to support a fully async back-end and would be a much needed improvement moving forward.

@saurabh500
Copy link
Contributor

Moving to future because of lack of resources

@saurabh500
Copy link
Contributor

Alternatively if someone from the community would like to drive this, the effort would be welcome.

@danmoseley
Copy link
Member

This needs a formal API proposal written up. @spudcud do you want to do this - updating your top post is clearest?
See https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

@jonreis
Copy link

jonreis commented Jan 31, 2018

Not sure if this helps, but someone has implemented this.

https://github.com/voloda/AsyncDataAdapter/blob/master/AsyncDataAdapter/DataAdapter.cs

@Misiu
Copy link

Misiu commented Oct 16, 2018

@danmosemsft any chance this can be added to Core 3.0?
I've added package suggested by @jonreis and it looks like it works fine 😄 Ideally this should be added into Core.

@effyteva
Copy link

+1 on this,
Perhaps it about time to take care of it?

@Misiu
Copy link

Misiu commented Apr 11, 2019

@danmosemsft @saurabh500 any updates on this?

@brendanalexdr
Copy link

+1

@danmoseley
Copy link
Member

@saurabh500?

@danmoseley
Copy link
Member

@divega

@divega
Copy link
Contributor

divega commented Apr 14, 2019

Moving to out of System.Data.SqlClient and into System.Data because I believe the methods should be added to the base DbDataAdapter, before any provider implements it.

I am also keeping this issue in the Future milestone because we need a much better understanding of its impact before we decide to pick it up and implement it in a specific release. For example:

  1. How many modern applications that take advantage of async would start using async APIs on DataSet/DataAdapter if they were available? Why don't they use other more modern data access technologies that already support async? I appreciate that a few people have voted for this issue and have been asking for updates on it. This adds up to a bit of data, but we need more, especially given that there are many other things in the backlog that seem to have much more demand.

  2. Is the main goal of using async in this case mainly achieving better scalability on the server or more responsiveness on the client?

  3. What other API that perform I/O does Fill use, that would need to become async in order for FillAsync to avoid every blocking?

  4. What about the async Update story for DataSet/DataTable?

On the flip side, If someone from the community wanted to drive this, I believe the API design part, and possibly the implementation of FillAsync on SqlClient, would be relatively simple. The job is mostly to duplicate the code of the sync version into async methods, and also make sure that all potentially blocking calls in the implementation use async APIs as well.

For the SqlCient implementation of Update, currently batching is internal and synchronous. I suspect it would make sense to leverage the new public batching APIs from https://github.com/dotnet/corefx/issues/35135 once they are in place.

@divega
Copy link
Contributor

divega commented Apr 14, 2019

cc @roji @ajcvickers

@divega divega changed the title SqlDataAdapter needs FillAsync to fully support the async pattern. Consider adding FillAsync and async APIs on data adapters Apr 14, 2019
@roji
Copy link
Member

roji commented Apr 17, 2019

Similar to dotnet/corefx#35012, which adds missing async APIs elsewhere in ADO.NET.

Adding these APIs in the System.Data base classes is probably easy enough (with them delegating by default to the sync APIs, as usual with ADO.NET), allowing providers to implement later.

@effyteva
Copy link

Similar to dotnet/corefx#35012, which adds missing async APIs elsewhere in ADO.NET.

Adding these APIs in the System.Data base classes is probably easy enough (with them delegating by default to the sync APIs, as usual with ADO.NET), allowing providers to implement later.

Would it help if I publish a PR for that? It seems pretty straightforward.

Effy

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
maxle5 added a commit to maxle5/runtime that referenced this issue Mar 22, 2020
@uprightbass360
Copy link

Is this still under review?

@roji
Copy link
Member

roji commented May 20, 2020

@uprightbass360 this issue is currently in the backlog, which means we don't have immediate plans to implement it priority-wise. In general, investment in DbDataAdapter is mostly limited to maintenance at this point, but given enough user feedback we would definitely consider it. This could be a good issue for the community to look into.

@Misiu
Copy link

Misiu commented May 20, 2020

@maxle5 you added this feature to your fork. Any chance to create a PR?

@roji
Copy link
Member

roji commented May 20, 2020

One small note - I haven't reviewed @maxle5's commit, but we'd want to add async across all operations performed by DbDataAdapter, and not just for some (FillAsync). We don't want to end up with a type that only partially supports async.

@Misiu
Copy link

Misiu commented May 20, 2020

@roji
Copy link
Member

roji commented May 20, 2020

In that case a PR would be good to have!

@maxle5
Copy link

maxle5 commented May 20, 2020

@Misiu, @roji , I can definitely create a PR, I had just gotten side-tracked on a different project and never got around to it. I believe there is probably still some work required though; if someone could help point out what is missing, that would be great!

@stephentoub
Copy link
Member

In that case a PR would be good to have!

@roji, we don't accept PRs for new APIs until those APIs have been reviewed and approved. Have these? I don't see a formal API proposal here. Thanks.

@roji
Copy link
Member

roji commented May 20, 2020

@stephentoub this is only about adding async counterparts to already-existing sync APIs on DbDataAdapter. But you're right of course... @maxle5, what we'd need is an list of public APIs being added.

@maxle5
Copy link

maxle5 commented May 20, 2020

Full list of public APIs that were added (async counterparts to already-existing sync APIs):

namespace System.Data.Common
{
    public partial class DataAdapter
    {
        public virtual Task<int> FillAsync(DataSet dataSet);
        protected virtual Task<int> FillAsync(DataSet dataSet, string srcTable, IDataReader dataReader, int startRecord, int maxRecords);
        protected virtual Task<int> FillAsync(DataTable dataTable, IDataReader dataReader);
        protected virtual Task<int> FillAsync(DataTable[] dataTables, IDataReader dataReader, int startRecord, int maxRecords);
        public virtual Task<DataTable[]> FillSchemaAsync(DataSet dataSet, SchemaType schemaType);
        protected virtual Task<DataTable[]> FillSchemaAsync(DataSet dataSet, SchemaType schemaType, string srcTable, IDataReader dataReader);
        protected virtual Task<DataTable> FillSchemaAsync(DataTable dataTable, SchemaType schemaType, IDataReader dataReader);
    }
    public partial class DbDataAdapter
    {
        public override Task<int> FillAsync(DataSet dataSet);
        public Task<int> FillAsync(DataSet dataSet, int startRecord, int maxRecords, string srcTable);
        protected virtual Task<int> FillAsync(DataSet dataSet, int startRecord, int maxRecords, string srcTable, IDbCommand command, CommandBehavior behavior);
        public Task<int> FillAsync(DataSet dataSet, string srcTable);
        public Task<int> FillAsync(DataTable dataTable);
        protected virtual Task<int> FillAsync(DataTable dataTable, IDbCommand command, CommandBehavior behavior);
        protected virtual Task<int> FillAsync(DataTable[] dataTables, int startRecord, int maxRecords, IDbCommand command, CommandBehavior behavior);
        public Task<int> FillAsync(int startRecord, int maxRecords, params DataTable[] dataTables);
        public override Task<DataTable[]> FillSchemaAsync(DataSet dataSet, SchemaType schemaType);
        protected virtual Task<DataTable[]> FillSchemaAsync(DataSet dataSet, SchemaType schemaType, IDbCommand command, string srcTable, CommandBehavior behavior);
        public Task<DataTable[]> FillSchemaAsync(DataSet dataSet, SchemaType schemaType, string srcTable);
        public Task<DataTable> FillSchemaAsync(DataTable dataTable, SchemaType schemaType);
        protected virtual Task<DataTable> FillSchemaAsync(DataTable dataTable, SchemaType schemaType, IDbCommand command, CommandBehavior behavior);        
    }
    public partial interface IDbDataAdapter
    {
        Task PrepareAsync(CancellationToken cancellationToken);
        Task<int> ExecuteNonQueryAsync();
        Task<IDataReader> ExecuteReaderAsync();
        Task<IDataReader> ExecuteReaderAsync(CommandBehavior behavior);
        Task<object> ExecuteScalarAsync();
    }
    public partial interface IDbDataAdapter
    {
        Task CloseAsync();
        Task OpenAsync(CancellationToken cancellationToken);
    }
}

@elachlan
Copy link
Contributor

@roji Do you have an answer on using default interface implementation (DIM) in the runtime?

@roji
Copy link
Member

roji commented Nov 18, 2021

@elachlan not yet.

@pevans360

This comment has been minimized.

@roji

This comment has been minimized.

@pevans360

This comment has been minimized.

@pevans360

This comment has been minimized.

@elachlan

This comment has been minimized.

@roji

This comment has been minimized.

@pevans360

This comment has been minimized.

@roji

This comment has been minimized.

@Misiu
Copy link

Misiu commented Feb 11, 2022

@roji any updates?
.NET 7 Preview 1 is almost released (dotnet/core#7106), so is there a chance to have this issue added to .NET 7 milestone?

@daiplusplus
Copy link

Just bumping myself (sorry!) for awareness:

For those wanting something in the meanwhile, my AsyncDataAdapter is available on NuGet:

@coderb
Copy link

coderb commented Apr 6, 2022

+1 for async version of DataTable.Load()

@Welchen
Copy link

Welchen commented Dec 28, 2022

Any update on this?

@Misiu
Copy link

Misiu commented Dec 31, 2022

So... Maybe .NET8?

@pjenei
Copy link

pjenei commented May 22, 2023

@Jehoel @ryan-carbon (snipping last reply from you both, which I emphatically agree with)

It sounds like we all have similar use cases on this.

Also, in my use case, we only use fill/getschema methods. We aren't using and of the CUD type methods through DataAdapter.

So... what is the "recommended", NON-EF approach to

  • Get Schema
  • Fill a Data Set

Worst case, I'd hypothesize I could just

  • Open a reader

  • Loop the recordsets returned from the reader

    • Create a datatable per recordset manually (I need to confirm it works with zero rows, though from my recollection it does)

      • ** something like this may work**
        SqlDataReader reader = command.ExecuteReader();
        DataTable schemaTable = reader.GetSchemaTable();
    • Loop the records and .newrow them for each row

    • Add each datatable to the dataset

Any thoughts for those most basic needs....

@ChristineBoersen I've created a very-very basic solution for my problem, which is similar to yours: I just need to fill an empty DataSet with data from an already opened DataReader. You can create your own version based on this. The main differences from your requirements:

  • (You don't mention IDataReader but I have to make clear that my code depends on DbDataReader rather than IDataReader, because IDataReader does not have any async methods.)
  • No DbDataAdapter is involved, just a DataSet and a DbDataReader.
  • I use an ADO.NET provider that does not support multiple result sets, so I don't loop through result sets using NextResultAsync(). This also means I don't have to take care of adding multiple tables to the DataSet and their naming.
  • I don't use GetSchemaTableAsync() to get the detailed schema information. In my first version, I used it, but it was much slower than this one and I don't really need the detailed information. I can create the columns based on GetName() and GetFieldType().
    public static async Task FillAsync(this DataSet dataSet, DbDataReader dataReader, CancellationToken cancellationToken)
    {
      var dataTable = dataSet.Tables.Add("Table");
      for (int i = 0; i < dataReader.FieldCount; i++)
      {
        var column = new DataColumn(dataReader.GetName(i), dataReader.GetFieldType(i));
        dataTable.Columns.Add(column);
      }

      var values = new object[dataTable.Columns.Count];
      dataTable.BeginLoadData();
      while (await dataReader.ReadAsync(cancellationToken))
      {
        dataReader.GetValues(values);
        dataTable.LoadDataRow(values, true);
      }
      dataTable.EndLoadData();
    }

@RabbitFH
Copy link

+1 async

@elachlan
Copy link
Contributor

@roji You wrote:

Also, don't forget we still need an OK for using default interface implementation (DIM) - I'll try to push for a definite answer on this, but once again this won't happen in the next couple of months.

It is two years later. Could we please get an answer on the DIM?

@Rayzbam

This comment was marked as duplicate.

2 similar comments
@jamezamm

This comment was marked as duplicate.

@gumbarros

This comment was marked as duplicate.

@slang25
Copy link
Contributor

slang25 commented Aug 1, 2023

Stop

Stop people, you're plus one-ing all wrong 😄 Scroll to the top and hit the thumbs-up on the opening post, and then optionally hit subscribe to hear about new developments (and hopefully not people saying "+1 async")

@RabbitFH
Copy link

+1 async

@jgilbert2017
Copy link

.net9 pretty please.

@Tasin5541
Copy link

Definitely need this for cases where EF Core is not good enough for maintaining SLA and have to end up using Stored Procedure with multiple datasets.

@roji
Copy link
Member

roji commented Mar 7, 2024

@Tasin5541 and others, I recommend taking a look at Dapper.

@Gruski
Copy link

Gruski commented Apr 28, 2024

Why don't they use other more modern data access technologies?

Like what? Non ORM direct SQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Data enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.