Skip to content

Introduce DbDataSource abstraction to System.Data #64812

Closed

Description

@roji

Summary

This proposes introducing a new ADO.NET abstraction called DbDataSource, which represents a data source from which (1) connections can be retrieved, and against which (2) commands can be executed directly. tl;dr:

// Provider-specific data source construction:
var dataSourceBuilder = new NpgsqlDataSourceBuilder
{
    ConnectionString = "<CONNECTION_STRING>",
    // Set authentication callbacks, auth tokens, ILoggerFactory, and any other options
};
using var dataSource = dataSourceBuilder.Build();

// Use the data source as a connection factory:
await using (var connection = await dataSource.OpenConnectionAsync())
{
    // Use the connection as usual
}

// Provider-agnostic data source construction:
using var dataSource = dbProviderFactory.CreateDataSource("<CONNECTION_STRING>");

// Get a command that executes directly against the data source, without explicitly referring to the connection
await using (var command = dataSource.CreateCommand("SELECT * FROM Blogs"))
await using (var reader = command.ExecuteReaderAsync())
{
    // Process results as usual
}

Summary of benefits

  • DbDataSource encapsulates all information and configuration needed to provide an open database connection (auth information, logging setup, type mapping config, etc.), ready for executing commands.
  • This makes it suitable for passing around and registering in DI as a factory for connections, without needing to pass around any additional information.
  • It's intended for DbDataSources to correspond to connection pools managed internally in the database driver.
    • With the current API, drivers need to look up the internal pool each time a connection is opened, by the connection string. Since DbDataSource allows user code to directly reference an (abstracted) internal pool, it eliminates that lookup and helps perf.
    • DbDataSource isn't an abstraction for a connection pool (see #24856); that would require introducing some pooling-specific APIs, is complicated, and might not be a good idea. However, it could evolve into one in the future, with external pooling implementations which can be used across ADO.NET providers.
  • It's also possible to get and execute a DbCommand directly on DbDataSource, without needing to deal with DbConnection at all. In scenarios where no connection state is required (e.g. transactions), it's not necessary to burden users with managing the connection, so this can be done under the hood. In addition, this opens up command execution models which aren't tied to a specific connection (e.g. multiplexing on Npgsql).
  • The proposal includes a shim to make the new abstraction immediately available on all ADO.NET drivers, without them needing to implement anything (unless they wish to customize behavior).

Connection instantiation and dependency injection

In ADO.NET, DbConnection implementations are typically instantiated directly, passing the connection string to the constructor:

using var connection = new NpgsqlConnection("<CONNECTION_STRING>");

This requires knowing the connection string at every site where a connection is instantiated. In addition, further authentication details are frequently needed beyond the connection string (SSL callbacks, auth tokens...), which also need to be set on the connection before it's opened. In practice, this means that user applications typically need to create some sort of factory for creating connections, to centralize this information.

Using ADO.NET with dependency injection

For example, when using ADO.NET (or Dapper) with DI, it's possible to configure a scoped connection as follows:

builder.Services.AddScoped(_ => new NpgsqlConnection(
    builder.Configuration.GetConnectionString("BloggingContext")));

However, if multiple connections are needed for any reason, then this becomes more complicated, and some sort of factory is necessary. Rather than forcing every application to build this, DbDataSource would allow the following:

builder.Services.AddSingleton(sp => NpgsqlDataSource.Create(
    builder.Configuration.GetConnectionString("BloggingContext")));

Provider-specific sugar APIs could make this nicer:

builder.Services.AddNpgsqlDataSource(
    builder.Configuration.GetConnectionString("BloggingContext"), o => o
      .UseEncryption(...)
      .SomeOtherConfig(...));

Applications can then get injected with the DbDataSource, and open connections through it. Alternatively, they can configure a scoped connection from the DbDataSource to be injected directly.

API point for connection configuration

The fact that connections are currently directly instantiated makes it difficult to add any sort of configuration APIs; the main way to tweak ADO.NET behavior is currently via connection strings, but that's not appropriate in many cases.

For example, let's suppose an ADO.NET provider wishes to add support Microsoft.Extensions.Logging. It's not possible to pass ILoggerFactory via the connection string, and while it's possible to add a DbConnection constructor overload which accepts ILoggerFactory, that would mean re-instantiating all the required loggers each time a connection is created - which isn't acceptable for perf reasons. With DbDataSource, the logging factory could be assigned directly on the provider's DbDataSource implementation, and all connection retrieved from that DbDataSource would use it.

In a similar vein, Npgsql has an extensible plugin system for modifying type mappings. This allows users to read PostgreSQL date/time values as NodaTime types, instead of as built-in .NET types such as DateTime. Similarly, database spatial types can be read as types from the NetTopologySuite library. At this point, Npgsql plugins can be enabled either globally (affecting all connections in the program), or for a specific NpgsqlConnection instance (generally inappropriate for perf reasons). It would be more appropriate to enable such plugins at the DbDataSource level, allowing multiple type mapping schemes to exist in the same applications without any perf overhead, etc.

Other possible features enabled by DbDataSource could be centrally specifying SQL statements to be prepared on all physical connections, SQL to execute on every physical connection when it's first opened (e.g. to set some state variable), etc.

It's suggested that ADO.NET drivers expose builders - e.g. NpgsqlDataSourceBuilder - which can configure all provider-specific aspects of the data source. Once a data source has been built, it should generally be immutable with respect to configuration options since connections may already have been handed out. An exception to this would probably be the rotation of authentication tokens.

DbProviderFactory

ADO.NET already contains an abstraction called DbProviderFactory, which can be used to create connections and other ADO.NET objects. However, this abstraction only serves for writing database-agnostic code that isn't specific to a certain driver:

using var connection = dbProviderFactory.CreateConnection();

The resulting DbConnection has no connection string or any other information on it; DbProviderFactory is nothing more than an abstraction for instantiating the provider-specific DbConnection implementation. It is generally a singleton type which represents the driver, as opposed to a database, a connection pool or similar.

We could theoretically retrofit DbProviderFactory with the above new APIs instead of introducing a new DbDataSource type. However, mixing the two concerns could be confusing, aswe'd then have two types of DbProviderFactory instances:

  1. Those which were created with connection information/configuration (new kind), correspond to a database and can produce open, ready-to-use connections
  2. Those which weren't (existing kind), and correspond to the driver as a whole. They can only produce blank connections which require configuration before they can be opened

As a result, I'm proposing to keep DbProviderFactory and DbDataSource separate, and to add a new CreateDataSource method on the DbProviderFactory abstraction, to allow DbDataSource to be created and used in database-agnostic code.

Executing commands directly on DbDataSource

Beyond being just a factory for connections, we can allow executing commands directly on DbDataSource:

await using var command = dataSource.CreateCommand("SELECT ...");
command.Parameters.Add(...);
var result = await command.ExecuteScalarAsync();

The main advantage here is API usability: in many (most?) scenarios, there's no need for users to concern themselves with connections - they simply want to execute a query against a database. Any scenarios which involve connection state (e.g. transaction) do require explicitly handling connections, but in the common case where no connection state is involved, we can simplify things. In typical DI scenarios, it's expected that a DbDataSource get injected, and that most commands would be executed directly against it.

Note that this isn't a Dapper-like improvement of ADO.NET: users still deal with the standard DbCommand API, add parameters and read results as usual. In fact, Dapper could add extension methods for better usability over DbDataSource just as it does over DbConnection.

A second advantage is to open up execution models which bypass the concept of a connection - this is what Npgsql does with multiplexing. In this model, commands are provided to the driver for execution on any arbitrary connection; the driver is free to coalesce unrelated commands together for better efficiency, and to continue sending commands on connections even when they're already busy, allowing better utilization of pooled connections. Executing a command directly on a DbDataSource is the right API for this kind of execution model.

To make this available on all ADO.NET providers without requiring changes, the default implementation of DbDataSource.CreateCommand would return a DbCommand wrapper over the native DbCommand returned by the driver's DbProviderFactory.CreateCommand. DbCommand.ExecuteReader (and other execution methods) would be overridden to open the connection (retrieved from DbDataSource.GetConnection) before execution, and to close it afterwards (e.g. with CommandBehavior.CloseConnection). Since the connection is owned by the command in this scenario, accessing DbCommand.Connection and Transaction should raise an exception.

Additional notes

  • When instantiating a DbConnection directly and when pooling is enabled, the ADO.NET driver must locate the internal connection pool based on the connection string. This typically involves an internal dictionary lookup on the connection string, but if any additional information can affect pooling, the situation can become more complicated. DbDataSource eliminates this perf overhead by doing any setup once at startup, and from that point the user directly references the internal pool through the abstraction.
  • DbDataSource is disposable, so that disposing may close all connections and resources associated with them, similar to the semi-standard ClearPool API that exists in SqlClient, Npgsql, Microsoft.Data.SQLite, etc.
    • A disposed DbDataSource is expected to throw ObjectDisposedException from GetConnection. However, connections which have already been handed out should continue working, and can themselves be disposed.
  • Since the provider's regular DbConnection instances are handed out by DbDataSource, users may call Close and Open on them. While this is not the recommended usage pattern, this would work and have the same effect as if the connection were instantiated directly. That is, if Close is called on a DbConnection that was acquired from a DbDataSource that represents a connection pool, the connection would be returned to the pool as usual.
  • When users instantiate two pooling DbDataSource instances with the same connection strings, those DbDataSources would typically (but not necessarily) represent two distinct pools. This isn't the case when using the shim implementation, where all connections with the same connection string are managed via the same pool.
  • Naming-wise:
    • There's a DataSource property on DbConnection, which contains an arbitrary provider-specific string representing the database of the connection. Introducing DbDataSource seems reasonably consistent with that.
    • There's also DbDataSourceEnumerator which returns a DataTable with various information on visible databases. Logically it's the same concept of a DataSource, though, so that makes sense.
  • Unfortunately we can't use covariant return types on DbDataSource, since we have async APIs returning ValueTask<T> which isn't covariant. So we use the old ADO.NET "covariance" pattern (protected virtual plus public non-virtual that gets replaced in implementations).

API proposal

public abstract class DbDataSource : IDisposable, IAsyncDisposable
{
    public abstract string ConnectionString { get; }

    protected abstract DbConnection GetDbConnection();

    protected virtual DbConnection OpenDbConnection()
    {
        var connection = GetDbConnection();
        connection.Open();

        return connection;
    }

    protected virtual async ValueTask<DbConnection> OpenDbConnectionAsync(CancellationToken cancellationToken = default)
    {
        var connection = GetDbConnection();
        await connection.OpenAsync(cancellationToken);

        return connection;
    }

    protected virtual DbCommand CreateDbCommand(string? commandText = null)
    {
        var connection = GetDbConnection();
        var command = connection.CreateCommand();

        // Return a DbCommand wrapper over connection and command, which takes care of opening/closing the connection around executions.
    }

    protected virtual DbBatch CreateDbBatch()
    {
        // Similar to CreateDbCommand...
    }

    public DbConnection GetConnection() => GetDbConnection();
    public DbConnection OpenConnection() => OpenDbConnection();
    public ValueTask<DbConnection> OpenConnectionAsync() => OpenDbConnectionAsync();
    public DbCommand CreateCommand(string? commandText = null) => CreateDbCommand();
    public DbBatch CreateBatch() => CreateDbBatch();

    public virtual void Dispose()
    {
    }

    public virtual ValueTask DisposeAsync()
        => default;
}

For provider-agnostic code, we'd add the following on System.Data.Common.DbProviderFactory:

public class DbProviderFactory
{
    public virtual DbDataSource CreateDataSource(string connectionString)
        => new DefaultDataSource(this, connectionString);
}

To make this work on existing providers, the default implementation of CreateDataSource would return an internal "shim" implementation implemented which simply gets connections from the DbProviderFactory that created it:

internal sealed class DefaultDataSource : DbDataSource
{
    private readonly DbProviderFactory _dbProviderFactory;
    private readonly string _connectionString;

    internal DefaultDataSource(DbProviderFactory dbProviderFactory, string connectionString)
        => (_dbProviderFactory, _connectionString) = (dbProviderFactory, connectionString);

    public override string ConnectionString => _connectionString;

    protected override DbConnection GetDbConnection()
    {
        var connection = _dbProviderFactory.CreateConnection();
        if (connection is null)
        {
            throw new InvalidOperationException("DbProviderFactory returned a null connection");
        }

        connection.ConnectionString = _connectionString;

        return connection;
    }

    // TODO: Command execution
}

Edit history

Date Modification
2022-02-04 Initial proposal
2022-02-07 Add note on multiple DbDatasources, connection strings and pools. Clarified that DbDataSource.ConnectionString should be non-nullable,
2022-05-05 Made DbDataSource.Dispose (and async) virtual no-op instead of abstract. Added missing public non-virtual DbDataSource.CreateCommand() and CreateBatch(). Added optional commandText parameter to CreateCommand for usability.

Activity

  1. added this to the 7.0.0 milestone on Feb 4, 2022
  2. self-assigned this
    on Feb 4, 2022
  3. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Feb 4, 2022
  4. removed
    untriagedNew issue has not been triaged by the area owner
    on Feb 4, 2022
  5. pinkfloydx33 commented on Feb 5, 2022

    @pinkfloydx33

    This would be great. I am getting tired of implementing factories for producing connections... though I often need to create a connection that varies based on something variable (known at runtime) other than the actual connection string. Almost something like a named data source that could be pre-registered and requested ala httpclient.

    To explain, we have a single application, but multiple tenant databases. My factories often are built around named retrieval and the factory is able to build a connection using the correct connection string for the customer I'm working with. It would be great if this could somehow be supported natively, especially with DI.

  6. roji commented on Feb 5, 2022

    @roji
    MemberAuthor

    @pinkfloydx33 thanks for the feedback. As a connection factory abstraction, a single DbDataSource isn't meant to handle multiple databases - that requires you to specify which database (or tenant) you want when you request a connection (and the connection objects themselves can't be pooled together if they're connected to different databases).

    Of course, you can have a tenant manager service in front of multiple DbDataSource instances, each corresponding to a single connection string (and your tenant manager service would be responsible for instantiating a DbDataSource when a connection to a new tenant is requested). In other words, I'd expect multi-tenant multi-database applications to have a DbDataSource per tenant.

  7. bgrainger commented on Feb 5, 2022

    @bgrainger
    Contributor

    I like it; I think a lot of existing code has to build their own concept of a factory for DI, even if it's as simple as .AddScoped(_ => new NpgsqlConnection(...)) example you gave.

    // Provider-agnostic data source construction:
    using var dataSource = dbProviderFactory.CreateDataSource("<CONNECTION_STRING>");

    I'm not yet convinced that this is a good API. For symmetry with other DbProviderFactory methods, I would expect just CreateDataSource(). However, that implies that the DbDataSource.ConnectionString property would be mutable, which would be a bad idea; keeping DbDataSource immutable is good. Would it be better to standardise the DbDataSourceBuilder abstract base class, and have DbProviderFactory.CreateDataSourceBuilder()?

    Secondly, the fact that a connection string is the only way to customise the DbDataSource (through this API) seems to imply that most/all useful properties can be set through the connection string, which this proposal already admits is a weakness of the current ADO.NET design.

    Finally, it's hard to imagine a scenario where providerFactory.CreateDataSource(connectionString) provides any concrete benefit over var connection = providerFactory.CreateConnection(); connection.ConnectionString = connectionString;. (And the latter code would end up being written anyway, since it wouldn't require .NET 7.0.) Am I missing something?

    public abstract string ConnectionString { get; } // TODO: Nullable?

    This should be non-null. Even if a driver can be used with all defaulted values in the connection string, that could be expressed as “”. And 99.9% of the time, there will at least be a remote server hostname (non-SQLite) or local file name (SQLite).

    It's intended for DbDataSources to correspond to connection pools managed internally in the database driver.

    Connection pools are usually implicit by connection string (“This typically involves an internal dictionary lookup on the connection string”). If DbDataSource is 1-1 with a connection pool (which seems like a typical, simple implementation) it may be worth explicitly clarifying that there is no requirement that new ExampleConnection(connectionString) and new ExampleDataSource(connectionString) share the same physical pool. (Even two different DbDataSources with the exact same connection string might have separate pools.) The ADO.NET shim would imply an equivalence (because it has to use the underlying provider's logic, which will usually create a shared pool for equal connection strings), but a custom implementation in a provider might find it simpler to keep them separate. The documentation should call this out and warn users not to rely on connection pools being separate per DbDataSource or being shared, unless provider-specific documentation explicitly states otherwise. (Or that a connection pool even exists; some providers may just not support pooling at all.)

    Naming-wise, there's a DataSource property on DbConnection, which contains an arbitrary provider-specific string representing the database of the connection. Introducing DbDataSource seems reasonably consistent with that.

    This actually seems a little confusing to me; coming to the API with fresh eyes, I would expect the DataSource property to be a way to get back to the DbDataSource that created the connection, i.e., Assert.Equal(dataSource, dataSource.GetConnection().DataSource). (Obviously the existing property can't be changed.)

    Should getting back to the DbDataSource from a connection be possible? One can imagine DB-independent code that, given a DbConnection, would like to create a second, “identical” connection. AFAIK, this is typically done by casting the DbConnection to ICloneable, for libraries that support that interface.

  8. Wraith2 commented on Feb 5, 2022

    @Wraith2
    Contributor

    keeping DbDataSource immutable is good. Would it be better to standardise the DbDataSourceBuilder abstract base class, and have DbProviderFactory.CreateDataSourceBuilder()

    If DbDataSource is immutable it's really just a type that encapsulates the two properties on a connection string config setting, the value and the provider type, isn't it? I think I've mentioned before that I'm not a fan of passing connection strings around because they're missing the really important information about the provider. It also encourages people to manipulate them as strings when i think using the ConnectionStringBuilder class is a better idea. Is it worth considering getting a correctly types connection string builder from the DbDataSource?

    It might be valuable to provide extensions to Microsoft.Extensions configuration readers so that DbDataSources can be created by users directly from config files rather than making them resolve the types themselves. So provide a DbDataSource dataSource = config.GetDbDataSource("name"); and then users can pass that around.

  9. bgrainger commented on Feb 5, 2022

    @bgrainger
    Contributor

    If DbDataSource is immutable it's really just a type that encapsulates the two properties on a connection string config setting, the value and the provider type, isn't it?

    On that abstract base class, perhaps yes. But it's highly likely that derived types will add properties for LoggerFactory and callbacks for client certificates or rotating passwords (e.g., Amazon IAM).

    It also encourages people to manipulate them as strings when I think using the ConnectionStringBuilder class is a better idea. Is it worth considering getting a correctly typed connection string builder from the DbDataSource.

    FWIW, DbConnectionStringBuilder had some performance issues last time I profiled it (albeit a few years ago): mysql-net/MySqlConnector#238. I'd personally prefer not to have it part of the DbDataSource API.

  10. FransBouma commented on Feb 5, 2022

    @FransBouma
    Contributor

    Am I right in that the main purpose of this class is solely dependency injection support? As to me every data access library under the sun has implemented code which supports what this class covers, so not sure what core problems it is solving, besides DI related ones?

  11. bgrainger commented on Feb 6, 2022

    @bgrainger
    Contributor

    A simple implementation (for discussion purposes) is in MySqlConnector:

  12. 25 remaining items

  13. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on May 31, 2022
  14. added a commit that references this issue on Jun 1, 2022
    a29d9e0
  15. roji commented on Jun 1, 2022

    @roji
    MemberAuthor

    Everyone, a PR implementing DbDataSource is out: #70006. I'd appreciate a thorough review from anyone who's interested.

    With regards to the API review comments (#64812 (comment)):

    Maybe GetConnection() should be called CreateConnection() because it returns a new instance

    • After some thought and discussion, the PR does change the naming from GetConnection to CreateConnection, but leaves OpenConnection as-is. I don't think it's perfect and am open to discussion.
    • GetConnection could confuse some people, making them think there's only one connection (though there are many counter-examples, such as IEnumearble.GetEnumerator). CreateConnection avoids that, and it feels like OpenConnection is less susceptible to create that confusion.
    • Create is the prefix used in other places in ADO.NET (DbConnection.CreateCommand, DbProviderFactory), so we're consistent with that.
    • The API will indeed typically return new instances of DbConnection; but in pooling scenarios DbConnection is a façade which does not represent a physical pool (it's a lightweight object that can be closed and re-opened). So I think it's important to avoid naming such as OpenNewConnection which would be too heavy in the pooling context (which is the vast majority of cases).
    • Consuming users are expected to almost always use OpenConnection, and should only rarely need to get an unopened connection (i.e. when they need to do something to the connection object before opening it). So it's important to keep the OpenConnection API concise and easy - it's the primary API point for this new abstraction.

    Consider introducing a term that represents connection who logically don't have a connection and rename CreateCommand()

    • The issue is that the instance returned from CreateCommand doesn't allow accessing the Connection and Transaction properties (since it owns the connection and manages it internally).
    • After thinking about it, I currently think leaving the name CreateCommand is the right thing to do:
      • Manipulating these two properties is only a small part of the command API surface - execution is the main function, and that behaves just like a normal command. This command doesn't behave fundamentally differently from a regular command - it just has two disabled properties.
      • There are already command states in which e.g. mutating Connection/Transaction shouldn't work, e.g. changing them while the command is in progress and a reader is open.
      • Introducing a new special term would add more confusion than help; an informative NotSupportedException from these properties should be sufficient.
      • Once again, CreateCommand is a main user-facing API on DbDataSource, so it's good to keep it concise and not overload it because of a restriction that won't matter in most cases.

    Note: a WIP implementation in Npgsql can be found here.

  16. Wraith2 commented on Jun 1, 2022

    @Wraith2
    Contributor

    I read through the changes last night and had nothing to add or ask. Looks good to me.

  17. ithline commented on Jun 1, 2022

    @ithline

    If the only difference between OpenConnection and CreateConnection is that the former returns already open connection, couldn't both methods be merged together with optional parameter specifying if the connection should return already open?

  18. terrajobst commented on Jun 1, 2022

    @terrajobst
    Contributor
    • Manipulating these two properties is only a small part of the command API surface - execution is the main function, and that behaves just like a normal command. This command doesn't behave fundamentally differently from a regular command - it just has two disabled properties.

    If you believe that's the right thing, I can accept that. From my own experience I'd say I have rarely (if ever) used the Connection property on a Command object; having that throw would largely be unobserved. Using transactions with commands is very common though, but the way ADO.NET works is that one usually initiates the transaction before creating the command and then passes in the transaction into the command. So given that these APIs don't accept one, that's probably not causing any confusion either.

    it just has two disabled properties

    Looking at the API it seems both Connection and Transaction are nullable propertieso on DbCommand. So if you say disabled I'd think it would mean that they return null and would only throw when someone tries to assign to them, is that correct?

    FWIW, I think you still want to come up with a name for these kind of commands, not for the API itself, but I think this will be a very useful thing for the documentation in order to describe what these kind of commands can go. Having a concept also allows you to refer to them when throwing exceptions, e.g. "You cannot set a connection on connection-less commands". People can then search for "connection-less commands" to learn what these are and how they work. Otherwise you have to refer to them in a convoluted way, like "You cannot set a connection on commands created by DbDataSource.CreateCommand()", which might also be confusing when people get these command objects from a different place, e.g. SqlDataSource.CreateCommand() or MyBackendThingie.Command.

  19. roji commented on Jun 2, 2022

    @roji
    MemberAuthor

    @Wraith2 thanks for taking the time to review!

    @ithline collapsing OpenConnection and Createconnection into a single method is possible, however:

    • Providers will generally only need to override CreateConnection: OpenConnection simply calls that and adds the open. So if we collapse, all providers would need to reimplement the opening as well.
    • Users are expected to only use OpenConnection in the vast majority of cases; the only reason why they'd need an un-opened connection is if they need to tweak the connection in some way before opening it (this generally shouldn't happen much). So in their interaction with the main API, I'd rather they didn't have a bool parameter they'd need to understand and think about - just a simple OpenConnection seems best.
  20. roji commented on Jun 2, 2022

    @roji
    MemberAuthor

    Looking at the API it seems both Connection and Transaction are nullable propertieso on DbCommand. So if you say disabled I'd think it would mean that they return null and would only throw when someone tries to assign to them, is that correct?

    That's a good point - we can indeed make them just always return null when read (the getter currently also throws in the PR); this may unblock certain scenarios where code inspects these properties for some reason. For Connection specifically that may not be a good idea, since in the normal case a command with no connection isn't executable, but in this case it is; so code making standard assumptions would fail here, and a fail-fast exception from the getter may be better (for transaction it seems more OK). What do you think?

    FWIW, I think you still want to come up with a name for these kind of commands, not for the API itself, but I think this will be a very useful thing for the documentation in order to describe what these kind of commands can go.

    You're right - in my head I've been calling these "data source commands", though "connection-less commands" is probably better. Will keep this in mind for the docs.

  21. ithline commented on Jun 2, 2022

    @ithline

    @roji my thought process was that OpenConnection() and CreateConnection() should mimick CreateCommand(string? commandText = null).

    If the public signature would be public DbConnection CreateConnection(bool createOpen = true), then users could call just dataSource.CreateConnection() and when they would need closed connection, then they would pass false value.

    From provider implementation it is a bit more work, but it would result in a cleaner public API, mainly because OpenConnection does not communicate very well (at least to me) that the method returns a new instance.

  22. roji commented on Jun 2, 2022

    @roji
    MemberAuthor

    The thing is that almost nobody needs to get an un-opened connection, but everyone would be "burdened" with understanding what the optional bool parameter is about (e.g. Intellisense). This is very different from the situation with DbCommand and commandText, since every command requires a commandText in order to work. I basically want to separate/hide away the unopen scenario because very few users need it.

    [...] mainly because OpenConnection does not communicate very well (at least to me) that the method returns a new instance.

    I get that, though I personally don't feel that OpenConnection conveys that either way (it can be read as "open the connection" or "open a connection"); FWIW I felt the same way about GetConnection - after all, we have IEnumerable.GetEnumerator in .NET which returns a new instance.

  23. added a commit that references this issue on Jun 6, 2022
    be4d292
  24. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Jun 6, 2022
  25. ghost locked as resolved and limited conversation to collaborators on Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions