Implement SqlConnection.GetSchemaAsync #3005

edwardneal · 2024-11-14T21:28:42Z

Contributes to #646, adding SqlConnection.GetSchemaAsync.

I can't add the IDbColumnSchemaGenerator method because the .NET Framework version of SqlDataReader doesn't yet implement that interface. #2967 tracks this. SqlDataReader.GetSchemaTableAsync requires some extra work with the state machine to guarantee that the column metadata is available, and I'll submit a separate PR for this.

I've added some basic documentation, but this effectively follows the SqlCommand.[Something]Async pattern of "An asynchronous version of [Something], which <etc.>"

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.

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbMetaDataFactory.cs

roji · 2024-11-14T22:20:22Z

src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbMetaDataFactory.cs

@@ -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)


Really nice to be seeing proper async/await in SqlClient!

codecov · 2024-11-14T23:01:04Z

Codecov Report

Attention: Patch coverage is 93.18182% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.02%. Comparing base (df35633) to head (eb164d6).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ient/src/Microsoft/Data/SqlClient/SqlDataReader.cs	50.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3005      +/-   ##
==========================================
- Coverage   63.30%   63.02%   -0.28%     
==========================================
  Files         280      274       -6     
  Lines       62386    62104     -282     
==========================================
- Hits        39493    39143     -350     
- Misses      22893    22961      +68

Flag	Coverage Δ
addons	`?`
netcore	`67.19% <91.66%> (+0.17%)`	⬆️
netfx	`62.29% <92.50%> (-3.87%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This introduces async-over-sync, removing such requires work with the state machine and separate review.

paulmedynski · 2025-07-02T14:07:58Z

@edwardneal - Can you resolve the conflicts, and then I will kick off another CI run?

edwardneal · 2025-07-02T21:15:12Z

Thanks @paulmedynski, I've just brought the branch up to date.

paulmedynski · 2025-07-03T11:56:18Z

/azp run

azure-pipelines · 2025-07-03T11:56:39Z

Azure Pipelines successfully started running 2 pipeline(s).

paulmedynski

A few inconsistencies and questions.

paulmedynski · 2025-07-03T19:07:44Z

doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml

+        The cancellation token.
+      </param>
+      <summary>
+        An asynchronous version of <see cref="M:Microsoft.Data.SqlClient.SqlConnection.GetSchema()" />, which returns schema information for the data source of this <see cref="T:Microsoft.Data.SqlClient.SqlConnection" />. For more information about scheme, see <see href="https://learn.microsoft.com/sql/connect/ado-net/sql-server-schema-collections">SQL Server Schema Collections</see>.


scheme -> schemas ?

Thanks - corrected in GetSchemaAsync and GetSchema.

paulmedynski · 2025-07-03T19:11:51Z

src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs

@@ -993,6 +993,21 @@ public event Microsoft.Data.SqlClient.SqlInfoMessageEventHandler InfoMessage { a
        public override System.Data.DataTable GetSchema(string collectionName) { throw null; }
        /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/GetSchemaCollectionNameRestrictionValues/*'/>
        public override System.Data.DataTable GetSchema(string collectionName, string[] restrictionValues) { throw null; }
+#if NET


This whole file is NET only, so is this #if necessary?

The file is also used to build the netstandard2.0 ref assemblies, which doesn't provide this member in the base class.

paulmedynski · 2025-07-04T10:12:35Z

src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs

+        public override System.Threading.Tasks.Task<System.Data.DataTable> GetSchemaAsync(string collectionName, System.Threading.CancellationToken cancellationToken = default) { throw null; }
+        /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/GetSchemaCollectionNameRestrictionValuesAsync/*'/>
+        public override System.Threading.Tasks.Task<System.Data.DataTable> GetSchemaAsync(string collectionName, string[] restrictionValues, System.Threading.CancellationToken cancellationToken = default) { throw null; }
+#else


Are we missing GetSchemaTableAsync ?

I've removed this method from the implementation and the ref assemblies. At the moment, we get the metadata for a result set by getting the SqlDataReader.MetaData property, which synchronously waits on the network. A future PR can perform the work needed to unpick the async-over-sync here and provide a fully async implementation.

paulmedynski · 2025-07-04T10:17:37Z

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlSchemaInfoTest/SqlSchemaInfoTest.cs

+
+                Assert.True(dataBases.Rows.Count > 0, "At least one database is expected");
+
+                DataTable metaDataCollections = await conn.GetSchemaAsync(DbMetaDataCollectionNames.MetaDataCollections);


Why are we only testing one of the GetSchemaAsync() methods within a transaction, but not the others?

It was an artifact of the existing synchronous tests. I've added that coverage to both the async and the sync tests.

paulmedynski · 2025-07-04T10:19:29Z

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs

@@ -1638,6 +1638,31 @@ public override DataTable GetSchemaTable()
            }
        }

+        /// <include file='../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetSchemaTableAsync/*' />
+#if NETFRAMEWORK
+        internal Task<DataTable> GetSchemaTableAsync(CancellationToken cancellationToken = default)


I think we're missing a test for this new method.

Remove SqlDataReader.GetSchemaTableAsync (and references to such.) Expand test coverage of both GetSchema and GetSchemaAsync. Correct typo in documentation for GetSchema and GetSchemaAsync.

edwardneal added 2 commits November 14, 2024 20:36

Implemented asynchronous schema APIs

7313a6f

Remove erroneously-added reference to shared csproj

63dae61

roji reviewed Nov 14, 2024

View reviewed changes

Implemented SqlDataReader.GetSchemaTableAsync

40d5f66

edwardneal changed the title ~~Implement SqlConnection.GetSchemaAsync~~ Implement SqlConnection.GetSchemaAsync and SqlDataReader.GetSchemaTableAsync Nov 15, 2024

edwardneal added 2 commits November 15, 2024 12:13

ref project cleanup

a58b0a4

Add default parameter value to ref projects

b60c6a6

edwardneal marked this pull request as draft November 15, 2024 22:38

edwardneal added 2 commits November 16, 2024 08:39

Merge branch 'main' into issue-646

fdc11cd

Revert addition of SqlDataReader API

6bbd433

This introduces async-over-sync, removing such requires work with the state machine and separate review.

edwardneal marked this pull request as ready for review November 16, 2024 08:56

edwardneal changed the title ~~Implement SqlConnection.GetSchemaAsync and SqlDataReader.GetSchemaTableAsync~~ Implement SqlConnection.GetSchemaAsync Nov 16, 2024

mdaigle added Enhancement 💡 Issues that are feature requests for the drivers we maintain. Public API 🆕 Issues/PRs that introduce new APIs to the driver. labels Nov 25, 2024

edwardneal added 2 commits July 2, 2025 22:01

Merge main

2d3a93d

Account for SqlDataReader merge and .NET Standard support

eb164d6

edwardneal requested a review from a team as a code owner July 2, 2025 21:14

paulmedynski requested changes Jul 4, 2025

View reviewed changes

paulmedynski requested a review from a team July 4, 2025 10:20

Response to code review

5f05e45

Remove SqlDataReader.GetSchemaTableAsync (and references to such.) Expand test coverage of both GetSchema and GetSchemaAsync. Correct typo in documentation for GetSchema and GetSchemaAsync.


		Assert.True(dataBases.Rows.Count > 0, "At least one database is expected");

		DataTable metaDataCollections = await conn.GetSchemaAsync(DbMetaDataCollectionNames.MetaDataCollections);

Implement SqlConnection.GetSchemaAsync #3005

Are you sure you want to change the base?

Implement SqlConnection.GetSchemaAsync #3005

Uh oh!

Conversation

edwardneal commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

paulmedynski commented Jul 2, 2025

Uh oh!

edwardneal commented Jul 2, 2025

Uh oh!

paulmedynski commented Jul 3, 2025

Uh oh!

azure-pipelines bot commented Jul 3, 2025

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edwardneal commented Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading