Async System.Data resultset and database schema APIs

[roji]

Provider tracking issues

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

• SqlClient: .NET 5.0 | Add async System.Data resultset and database schema APIs  SqlClient#646
• Npgsql: Introduce async schema APIs on connection and reader npgsql/npgsql#2976
• MySqlConnector: Async resultset and database schema APIs mysql-net/MySqlConnector#835

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 npgsql/npgsql#2976.

Proposed API

public class DbConnection
{
    public virtual Task<DataTable> GetSchemaAsync(string collectionName = null, string[] restrictions = null, CancellationToken cancellationToken = default);
}

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

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

public static class DbDataReaderExtensions
{
    public static Task<ReadOnlyCollection<DbColumn>> 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

[terrajobst]

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.

namespace System.Data.Common
{
    public partial class DbConnection
    {
        public virtual Task<DataTable> GetSchemaAsync(CancellationToken cancellationToken = default);
        public virtual Task<DataTable> GetSchemaAsync(string collectionName, CancellationToken cancellationToken = default);
        public virtual Task<DataTable> 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);
    }
}

[bgrainger]

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);
`

[stephentoub]

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.

[bgrainger]

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

From #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.

[bricelam]

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.

runtime/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReaderExtensions.cs

Lines 54 to 65 in 4f9ae42

	public static ReadOnlyCollection<DbColumn> GetColumnSchema(this DbDataReader reader)
	{
	if (reader is null)
	{
	throw new ArgumentNullException(nameof(reader));
	}
	if (reader is IDbColumnSchemaGenerator schemaGenerator)
	{
	return schemaGenerator.GetColumnSchema();
	}
	return GetColumnSchemaCompatibility(reader);
	}

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.

[ajcvickers]

Thanks for looking into this @bgrainger and @bricelam