Implement SqlConnection.GetSchemaAsync #3005
Conversation
| cancellationToken.ThrowIfCancellationRequested(); | ||
| #if NET | ||
| DataTable schemaTable = isAsync ? await reader.GetSchemaTableAsync(cancellationToken).ConfigureAwait(false) : reader.GetSchemaTable(); | ||
| #else |
There was a problem hiding this comment.
Would it make sense to have the .NET Framework path also use an async API here? Unless I'm missing something, the exact same methods/code could be define for both, with the sole extension that in .NET Framework it wouldn't override?
There was a problem hiding this comment.
I can do that; I thought it might look a little clumsy for the .NET Framework version of Microsoft.Data.SqlClient to have a dedicated async method which isn't present on System.Data.SqlClient and which doesn't actually run asynchronously.
(GetSchemaTable just constructs a DataTable from the cached metadata - there's nothing which we can make async.)
| @@ -107,7 +109,7 @@ protected virtual void Dispose(bool disposing) | |||
| } | |||
| } | |||
|
|
|||
| private DataTable ExecuteCommand(DataRow requestedCollectionRow, string[] restrictions, DbConnection connection) | |||
| private async ValueTask<DataTable> ExecuteCommandAsync(DataRow requestedCollectionRow, string[] restrictions, DbConnection connection, bool isAsync, CancellationToken cancellationToken) | |||
There was a problem hiding this comment.
Really nice to be seeing proper async/await in SqlClient!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3005 +/- ##
=======================================
Coverage 72.48% 72.48%
=======================================
Files 288 288
Lines 59493 59508 +15
=======================================
+ Hits 43123 43135 +12
- Misses 16370 16373 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Contributes to #646, adding
SqlConnection.GetSchemaAsync. This is the only .NET 5.0+ method which would benefit from end-to-end async plumbing to remove the async-over-sync which is currently present. The other two schema methods on DbDataReader and IDbColumnSchemaGenerator already operate from the cached metadata; my implementation on DbDataReader wouldn't vary from the default one.I've added some basic documentation, but this effectively follows the
SqlCommand.[Something]Asyncpattern of "An asynchronous version of [Something], which <etc.>"Since .NET Framework doesn't have the base DbConnection.GetSchemaAsync methods, there's nothing to override. They'll be available anyway.
I've also moved the netfx GetSchema methods from SqlConnectionHelper.cs to SqlConnection.cs, to align that part of it with netcore. This contributes to the merge slightly.