Runtime: Async System.Data resultset and database schema APIs

Created on 17 Jun 2020  路  6Comments  路  Source: dotnet/runtime

Provider tracking issues

This new API has been merged, following are tracking issues for implementation in the different providers:

Background and Motivation

This proposal introduces async counterparts to existing sync APIs for fetching schema information for resultset and database schema. The original proposal was developed by @manandre and @bgrainger in https://github.com/npgsql/npgsql/issues/2976.

Proposed API

```c#
public class DbConnection
{
public virtual Task GetSchemaAsync(string collectionName = null, string[] restrictions = null, CancellationToken cancellationToken = default);
}

public class DbDataReader
{
public virtual Task GetSchemaTableAsync(CancellationToken cancellationToken = default);
}

public interface IDbColumnSchemaGenerator
{
Task> GetColumnSchemaAsync(CancellationToken cancellationToken = default);
}

public static class DbDataReaderExtensions
{
public static Task> GetColumnSchemaAsync(this DbDataReader reader, CancellationToken cancellationToken = default);
}
```

Notes

  • The default implementations of the above will call the appropriate sync counterpart, as is standard in ADO.NET.
  • As these APIs aren't perf-sensitive, they return Task instead of ValueTask. Since they typically return a considerable amount of data, the extra allocation is unlikely to matter in any case. They also mostly return asynchronously.

/cc @David-Engel @cheenamalhotra @bgrainger @manandre @bricelam @ajcvickers @stephentoub @terrajobst @mgravell @FransBouma

api-approved area-System.Data

Most helpful comment

The reason for DbDataReaderExtensions.GetColumnSchema() and IDbColumnSchemaGenerator is that it falls back to DbDataReader.GetColumnSchema() and an adapter when the reader doesn't implement the interface.

https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReaderExtensions.cs#L54-L65

The reason that it wasn't implemented directly on DbDataReader is because it was added over .NET Standard 1.2 where the DbDataReader type is forwarded to .NET Framework's System.Data assembly (where we couldn't add API) while the interface and the extension method live in the .NET Standard System.Data.Common assembly (where we could add API).

In other words, I think it's perfectly fine to add DbDataReader.GetColumnSchemaAsync() directly in .NET 5 given that we don't need to maintain compatibility with .NET Framework.

All 6 comments

Video

  • Instead of adding an interface member, we'd prefer exposing a new virtual on DbDataReader directly

    • @ajcvickers will follow up to see why we had the extension method calling through the interface instead.

  • We avoid having more than two optional parameters b/c we found in UX studies that optional parameters are confusing

    • We should mirror the sync version which has had three overloads

    • Sadly all three were virtual and no overload accepts null (thus can't be chained)

    • For consistency, we should follow this pattern.

```C#
namespace System.Data.Common
{
public partial class DbConnection
{
public virtual Task GetSchemaAsync(CancellationToken cancellationToken = default);
public virtual Task GetSchemaAsync(string collectionName, CancellationToken cancellationToken = default);
public virtual Task GetSchemaAsync(string collectionName, string[] restrictions, CancellationToken cancellationToken = default);
}

public partial class DbDataReader
{
    public virtual Task<DataTable> GetSchemaTableAsync(CancellationToken cancellationToken = default);
    public virtual Task<ReadOnlyCollection<DbColumn>> GetColumnSchemaAsync(CancellationToken cancellationToken = default);
}

}
```

Should nullable annotations be discussed in this PR, or does that depend on whether #689 is merged before or afterwards?

The only change I would make is string?[] restrictions in public virtual Task<DataTable> DbConnection.GetSchemaAsync(string collectionName, string?[] restrictions, CancellationToken cancellationToken = default);
`

We should, yes, thanks, @bgrainger. The nullable annotations on these should match those on the sync equivalents, and I see that the sync variant does have string?[] restrictions, so sounds good to me.

@ajcvickers will follow up to see why we had the extension method calling through the interface instead.

From https://github.com/dotnet/runtime/issues/16305

The right way of exposing the API would be to add an API in the DbDataReader class called
ReadOnlyCollection<DbColumn> DbDataReader.GetColumnSchema()

This is however not possible in .Net Core current version as it would hinder portability of apps created on .Net core with .Net framework.

CC @saurabh500 in case there's anything to add here.

The reason for DbDataReaderExtensions.GetColumnSchema() and IDbColumnSchemaGenerator is that it falls back to DbDataReader.GetColumnSchema() and an adapter when the reader doesn't implement the interface.

https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReaderExtensions.cs#L54-L65

The reason that it wasn't implemented directly on DbDataReader is because it was added over .NET Standard 1.2 where the DbDataReader type is forwarded to .NET Framework's System.Data assembly (where we couldn't add API) while the interface and the extension method live in the .NET Standard System.Data.Common assembly (where we could add API).

In other words, I think it's perfectly fine to add DbDataReader.GetColumnSchemaAsync() directly in .NET 5 given that we don't need to maintain compatibility with .NET Framework.

Thanks for looking into this @bgrainger and @bricelam

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nalywa picture nalywa  路  3Comments

yahorsi picture yahorsi  路  3Comments

jchannon picture jchannon  路  3Comments

GitAntoinee picture GitAntoinee  路  3Comments

btecu picture btecu  路  3Comments