Remove connection options inheritance by mdaigle · Pull Request #4237 · dotnet/SqlClient

mdaigle · 2026-04-24T19:27:55Z

This is prep work for respecting connection timeout values in the new connection pool.

Following in the steps of #4235 , this PR removes another unnecessary inheritance relationship. This also had its roots in ADO.NET, but is now vestigial. Standardizing on SqlConnectionOptions to match the variable naming throughout the codebase. The original SqlConnectionString name implied that it was just a string, but it's really a parsed version with additional functionality.

This pull request refactors the codebase to consistently use the SqlConnectionOptions type in place of the older DbConnectionOptions and SqlConnectionString types throughout the SQL client connection management code. This change improves type safety, clarity, and maintainability by standardizing connection option handling across all relevant classes and methods.

Refactoring for Consistent Connection Options Handling:

Replaced all usages of DbConnectionOptions and SqlConnectionString with SqlConnectionOptions in method parameters, constructors, and internal logic across connection-related classes such as DbConnectionInternal, DbConnectionClosed, SqlConnectionInternal, and ServerInfo. [1] [2] [3] [4] [5]
Updated method calls and internal logic to use SqlConnectionOptions methods and properties, including static helper methods like GetKeyValuePair and VerifyLocalHostAndFixup. [1] [2]

Code Cleanup and Type Alignment:

Removed unnecessary using directives for obsolete namespaces related to connection strings, and updated type hints and field declarations to use SqlConnectionOptions. [1] [2] [3]
Updated interface and method signatures in connection pool classes to accept SqlConnectionOptions instead of the previous types, ensuring consistency across pooling logic. [1] [2]

Internal Logic and Enum Usage:

Updated references to enums and nested types, such as TransactionBindingEnum, to use the new SqlConnectionOptions context.
Adjusted constructor and method implementations to align with the new type, including changes in overloads and private helper methods. [1] [2] [3] [4]

Overall, this refactoring modernizes the codebase and reduces ambiguity in how connection options are managed and passed between components.

Copilot

Pull request overview

Refactors SqlClient’s internal connection-string/option plumbing to standardize on SqlConnectionOptions, removing the legacy DbConnectionOptions/SqlConnectionString inheritance and propagating the new type through pooling, connection creation, and related helpers.

Changes:

Inlines former DbConnectionOptions parsing/state into SqlConnectionOptions and removes DbConnectionOptions.cs.
Updates connection/pooling code paths (factory, pool interfaces/implementations, internal connection types) to accept/use SqlConnectionOptions.
Updates unit/functional tests and a handful of call sites to use SqlConnectionOptions.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStringTest.cs	Switches tests from `SqlConnectionString` to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs	Updates pool test scaffolding to pass `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs	Updates mock pool signatures to match `SqlConnectionOptions`-based interface.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs	Updates channel pool tests to pass `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlExceptionTest.cs	Updates JSON stack trace fixture for new signatures (partially).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs	Switches protocol mapping helpers to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	Updates connect signature to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs	Updates debug parsing/casts to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs	Updates `TypeSystem` enum references to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs	Updates network library converter mapping to `SqlConnectionOptions.NETLIB` (partially).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionOptions.cs	Converts former `SqlConnectionString : DbConnectionOptions` into standalone `SqlConnectionOptions` and inlines parsing logic.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionOptions.Debug.cs	Moves debug-only parsing validation helpers onto `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs	Updates factory APIs and internal wiring to use `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs	Updates cached connection options fields/properties and casts to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs	Updates auth parameter creation to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs	Updates pending request/userOptions types and pool APIs to `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/SqlConnectionPoolGroupProviderInfo.cs	Updates constructor/FailoverCheck signatures to `SqlConnectionOptions` (partially).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs	Changes pool interface to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs	Stores/exposes `SqlConnectionOptions` instead of `DbConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Updates pool API signatures to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Updates internal connection APIs to accept/store `SqlConnectionOptions` (partially).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/ServerInfo.cs	Updates server info construction to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs	Updates open/replace signatures to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs	Updates open/replace signatures to accept `SqlConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionString.netfx.cs	Updates to wrap `SqlConnectionOptions` rather than `DbConnectionOptions`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.cs	Removes obsolete base parser/option type.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs	Updates exception helper signature to accept `SqlConnectionOptions`.

Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs:3912

SqlConnectionString is still referenced later in this file (e.g., ShouldDisableTnir(SqlConnectionString connectionOptions)), but the refactor renames/replaces it with SqlConnectionOptions. This will break NETFRAMEWORK builds; update the remaining signatures/usages to SqlConnectionOptions.

        private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup, SqlConnectionOptions options)
        {
            // @TODO: Invert to save on indentation
            if (serverInfo.ExtendedServerName == null)
            {

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionOptions.cs:25

This PR removes DbConnectionOptions/SqlConnectionString by folding the parsing logic into SqlConnectionOptions, but there are still in-repo references to the old SqlConnectionString type (e.g., in SqlDependency.cs, SqlClientPermission.netfx.cs, and some netfx-only helper methods). Those need to be updated to SqlConnectionOptions (and SqlConnectionOptions.KeywordMap where applicable) or the build will fail.

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionOptions.cs:16

SqlConnectionOptions now lives outside System.Security.Permissions for non-.NET Framework builds, but this file unconditionally imports System.Security.Permissions. Since the PermissionSet APIs are only used under #if NETFRAMEWORK, the using System.Security.Permissions; should be guarded (or removed for non-NETFRAMEWORK) to avoid compilation failures on net8+/net9+ targets that don’t reference that assembly/package.

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

mdaigle · 2026-04-27T22:40:34Z

@@ -53,20 +55,20 @@ internal DbConnectionString(
            string restrictions,
            KeyRestrictionBehavior behavior,
            IReadOnlyDictionary<string, string> synonyms)


We don't need to have a constructor that accepts synonyms. The only value that is ever passed is SqlConnectionOptions.KeywordMap. I opted to leave this constructor as-is to reduce the disturbance to anyone who uses this via reflection. Modifying these CAS helper-classes isn't the focus of this PR. They support a deprecated feature and I just need them to compile

mdaigle · 2026-04-27T22:44:45Z

                //  Need to call setter on Credential property rather than setting _credential directly as pool groups need to be checked
-                SqlConnectionString connectionOptions = (SqlConnectionString)ConnectionOptions;
-                if (UsesClearUserIdOrPassword(connectionOptions))
+                if (UsesClearUserIdOrPassword(ConnectionOptions))


A bunch of these casts are now redundant.

mdaigle · 2026-04-27T22:46:15Z

+        internal SqlConnectionOptions ConnectionOptions => PoolGroup?.ConnectionOptions;

-        internal DbConnectionOptions UserConnectionOptions => _userConnectionOptions;
+        internal SqlConnectionOptions UserConnectionOptions => _userConnectionOptions;


ConnectionOptions and UserConnectionOptions look the same but are subtly different (one is the expanded form). They'll be consolidated in the next PR.

mdaigle · 2026-04-27T22:47:45Z

-                SqlConnectionString constr = (SqlConnectionString)ConnectionOptions;

-                return constr?.WorkstationId
+                return ConnectionOptions?.WorkstationId


No comment on the nullability of ConnectionOptions. Not the focus of this PR, so I just maintained existing behavior.

mdaigle · 2026-04-27T22:50:10Z

 namespace Microsoft.Data.SqlClient
 {
-    internal sealed class SqlConnectionString : DbConnectionOptions
+    internal sealed partial class SqlConnectionOptions


DbConnectionOptions was merged down into this class. It's marked partial because DbConnectionOptions had a debug partial for printing out connection string options. I've maintained that pattern here to reduce changes.

mdaigle · 2026-04-27T22:53:00Z

            else
            {
-                return base.Expand();
+                return _usersConnectionString;


DbConnectionOptions.Expand() directly returned _usersConnectionString

mdaigle · 2026-04-27T22:53:18Z


        #endregion
+
+        #region DbConnectionOptions methods


Methods carried over from DbConnectionOptions

mdaigle · 2026-04-27T22:55:56Z

                // which we have already demanded upon.  
-                SqlConnectionString connStringObj = (SqlConnectionString)_con.ConnectionOptions;
-#if NETFRAMEWORK
+                SqlConnectionOptions connStringObj = _con.ConnectionOptions;


The ifdef moved up because we only need the options object if we're creating a permission set.

mdaigle · 2026-04-27T22:57:15Z

                        + @"""InnerException"":null,"
                        + @"""HelpURL"":null,"
-                        + @"""StackTraceString"":""   at Microsoft.Data.SqlClient.SqlInternalConnectionTds..ctor(DbConnectionPoolIdentity identity, SqlConnectionString connectionOptions, SqlCredential credential, Object providerInfo, String newPassword, SecureString newSecurePassword, Boolean redirectedUserInstance, SqlConnectionString userConnectionOptions, SessionData reconnectSessionData, Boolean applyTransientFaultHandling, String accessToken)\\n   at Microsoft.Data.SqlClient.SqlConnectionFactory.CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, Object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, DbConnectionOptions userOptions)\\n   at System.Data.ProviderBase.DbConnectionFactory.CreatePooledConnection(DbConnectionPool pool, DbConnection owningObject, DbConnectionOptions options, DbConnectionPoolKey poolKey, DbConnectionOptions userOptions)\\n   at System.Data.ProviderBase.DbConnectionPool.CreateObject(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection)\\n   at System.Data.ProviderBase.DbConnectionPool.UserCreateRequest(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection)\\n   at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, UInt32 waitForMultipleObjectsTimeout, Boolean allowCreate, Boolean onlyOneCheckConnection, DbConnectionOptions userOptions, DbConnectionInternal& connection)\\n   at System.Data.ProviderBase.DbConnectionPool.WaitForPendingOpen()\\n"","
+                        + @"""StackTraceString"":""   at Microsoft.Data.SqlClient.SqlInternalConnectionTds..ctor(DbConnectionPoolIdentity identity, SqlConnectionOptions connectionOptions, SqlCredential credential, Object providerInfo, String newPassword, SecureString newSecurePassword, Boolean redirectedUserInstance, SqlConnectionOptions userConnectionOptions, SessionData reconnectSessionData, Boolean applyTransientFaultHandling, String accessToken)\\n   at Microsoft.Data.SqlClient.SqlConnectionFactory.CreateConnection(SqlConnectionOptions options, DbConnectionPoolKey poolKey, Object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, SqlConnectionOptions userOptions)\\n   at System.Data.ProviderBase.DbConnectionFactory.CreatePooledConnection(DbConnectionPool pool, DbConnection owningObject, SqlConnectionOptions options, DbConnectionPoolKey poolKey, SqlConnectionOptions userOptions)\\n   at System.Data.ProviderBase.DbConnectionPool.CreateObject(DbConnection owningObject, SqlConnectionOptions userOptions, DbConnectionInternal oldConnection)\\n   at System.Data.ProviderBase.DbConnectionPool.UserCreateRequest(DbConnection owningObject, SqlConnectionOptions userOptions, DbConnectionInternal oldConnection)\\n   at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, UInt32 waitForMultipleObjectsTimeout, Boolean allowCreate, Boolean onlyOneCheckConnection, SqlConnectionOptions userOptions, DbConnectionInternal& connection)\\n   at System.Data.ProviderBase.DbConnectionPool.WaitForPendingOpen()\\n"","


The text of this exception doesn't really matter. The test covers message deserialization, so we just need valid formatting. Updated for completeness.

mdaigle · 2026-04-27T22:58:03Z

            );
            dbConnectionPoolGroup ??= new DbConnectionPoolGroup(
-                new DbConnectionOptions("DataSource=localhost;", null),
+                new SqlConnectionOptions("Data Source=localhost;"),


A side effect of this change is that it's now impossible to construct connection options from an invalid connection string, even in tests!

saurabh500

Looks a lot better now.

mdaigle added 2 commits April 24, 2026 12:00

Rename SqlConnectionString to SqlConnectionOptions

5d26307

Remove the DbConnectionOptions parent class.

cadd40a

Copilot AI review requested due to automatic review settings April 24, 2026 19:27

github-project-automation Bot added this to SqlClient Board Apr 24, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Apr 24, 2026

Copilot started reviewing on behalf of mdaigle April 24, 2026 19:28 View session

mdaigle added this to the 7.1.0-preview2 milestone Apr 24, 2026

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mdaigle added 2 commits April 24, 2026 14:15

Fix netfx compilation issues.

9ef831b

Add comments and missing using.

4ff134c

Copilot AI review requested due to automatic review settings April 24, 2026 21:28

Copilot started reviewing on behalf of mdaigle April 24, 2026 21:29 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mdaigle added 2 commits April 24, 2026 14:39

Fix compilation issues.

6209356

Fix compilation issues.

341f181

Copilot AI review requested due to automatic review settings April 24, 2026 21:48

Copilot started reviewing on behalf of mdaigle April 24, 2026 21:48 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread ...Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStringTest.cs Outdated

Comment thread ...icrosoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionOptionsTest.cs

mdaigle added 3 commits April 24, 2026 15:10

Remove stale file.

439c6ef

Clean up redundant casts.

bf5931d

Fix unit tests.

1133675

Copilot AI review requested due to automatic review settings April 24, 2026 22:41

Copilot started reviewing on behalf of mdaigle April 24, 2026 22:41 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread ...t.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs Outdated

mdaigle added 2 commits April 24, 2026 15:48

Fix compilation issue.

6d0afa0

Fix test connection string.

c55f6fb

Copilot AI review requested due to automatic review settings April 27, 2026 21:02

Copilot AI reviewed Apr 27, 2026

View reviewed changes

mdaigle commented Apr 27, 2026

View reviewed changes

mdaigle marked this pull request as ready for review April 27, 2026 23:02

mdaigle requested a review from a team as a code owner April 27, 2026 23:02

mdaigle changed the title ~~Dev/mdaigle/connection options~~ Remove connection options inheritance Apr 27, 2026

mdaigle assigned mdaigle, paulmedynski and priyankatiwari08 and unassigned mdaigle Apr 27, 2026

David-Engel approved these changes Apr 27, 2026

View reviewed changes

saurabh500 approved these changes Apr 27, 2026

View reviewed changes

Conversation

mdaigle commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Choose a reason for hiding this comment

Uh oh!

saurabh500 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mdaigle commented Apr 24, 2026 •

edited

Loading