From e6358c798b3001e98b6f7f4c3e2d906cf48533b2 Mon Sep 17 00:00:00 2001 From: James Lawrence Date: Sat, 5 Aug 2017 06:04:19 -0400 Subject: [PATCH] database/sql: add OpenDB to directly create a *DB without a DSN. The current Open method limits the ability for driver maintainers to expose options for their drivers by forcing all the configuration to pass through the DSN in order to create a *DB. This CL allows driver maintainers to write their own initialization functions that return a *DB making configuration of the underlying drivers easier. Fixes #20268 Change-Id: Ib10b794f36a201bbb92c23999c8351815d38eedb Reviewed-on: https://go-review.googlesource.com/53430 Reviewed-by: Daniel Theophanes Run-TryBot: Daniel Theophanes TryBot-Result: Gobot Gobot --- src/database/sql/driver/driver.go | 23 ++++++++++++ src/database/sql/sql.go | 62 ++++++++++++++++++++++++------- src/database/sql/sql_test.go | 19 +++++++--- 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/src/database/sql/driver/driver.go b/src/database/sql/driver/driver.go index 0262ca24ba..f5a2e7c16c 100644 --- a/src/database/sql/driver/driver.go +++ b/src/database/sql/driver/driver.go @@ -55,6 +55,29 @@ type Driver interface { Open(name string) (Conn, error) } +// Connector is an optional interface that drivers can implement. +// It allows drivers to provide more flexible methods to open +// database connections without requiring the use of a DSN string. +type Connector interface { + // Connect returns a connection to the database. + // Connect may return a cached connection (one previously + // closed), but doing so is unnecessary; the sql package + // maintains a pool of idle connections for efficient re-use. + // + // The provided context.Context is for dialing purposes only + // (see net.DialContext) and should not be stored or used for + // other purposes. + // + // The returned connection is only used by one goroutine at a + // time. + Connect(context.Context) (Conn, error) + + // Driver returns the underlying Driver of the Connector, + // mainly to maintain compatibility with the Driver method + // on sql.DB. + Driver() Driver +} + // ErrSkip may be returned by some optional interfaces' methods to // indicate at runtime that the fast path is unavailable and the sql // package should continue as if the optional interface was not diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 8d506385ff..89e5bf691e 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -317,8 +317,7 @@ var ErrNoRows = errors.New("sql: no rows in result set") // connection is returned to DB's idle connection pool. The pool size // can be controlled with SetMaxIdleConns. type DB struct { - driver driver.Driver - dsn string + connector driver.Connector // numClosed is an atomic counter which represents a total number of // closed connections. Stmt.openStmt checks it before cleaning closed // connections in Stmt.css. @@ -575,6 +574,48 @@ func (db *DB) removeDepLocked(x finalCloser, dep interface{}) func() error { // to block until the connectionOpener can satisfy the backlog of requests. var connectionRequestQueueSize = 1000000 +type dsnConnector struct { + dsn string + driver driver.Driver +} + +func (t dsnConnector) Connect(_ context.Context) (driver.Conn, error) { + return t.driver.Open(t.dsn) +} + +func (t dsnConnector) Driver() driver.Driver { + return t.driver +} + +// OpenDB opens a database using a Connector, allowing drivers to +// bypass a string based data source name. +// +// Most users will open a database via a driver-specific connection +// helper function that returns a *DB. No database drivers are included +// in the Go standard library. See https://golang.org/s/sqldrivers for +// a list of third-party drivers. +// +// OpenDB may just validate its arguments without creating a connection +// to the database. To verify that the data source name is valid, call +// Ping. +// +// The returned DB is safe for concurrent use by multiple goroutines +// and maintains its own pool of idle connections. Thus, the OpenDB +// function should be called just once. It is rarely necessary to +// close a DB. +func OpenDB(c driver.Connector) *DB { + db := &DB{ + connector: c, + openerCh: make(chan struct{}, connectionRequestQueueSize), + lastPut: make(map[*driverConn]string), + connRequests: make(map[uint64]chan connRequest), + } + + go db.connectionOpener() + + return db +} + // Open opens a database specified by its database driver name and a // driver-specific data source name, usually consisting of at least a // database name and connection information. @@ -599,15 +640,8 @@ func Open(driverName, dataSourceName string) (*DB, error) { if !ok { return nil, fmt.Errorf("sql: unknown driver %q (forgotten import?)", driverName) } - db := &DB{ - driver: driveri, - dsn: dataSourceName, - openerCh: make(chan struct{}, connectionRequestQueueSize), - lastPut: make(map[*driverConn]string), - connRequests: make(map[uint64]chan connRequest), - } - go db.connectionOpener() - return db, nil + + return OpenDB(dsnConnector{dsn: dataSourceName, driver: driveri}), nil } func (db *DB) pingDC(ctx context.Context, dc *driverConn, release func(error)) error { @@ -878,7 +912,7 @@ func (db *DB) openNewConnection() { // maybeOpenNewConnctions has already executed db.numOpen++ before it sent // on db.openerCh. This function must execute db.numOpen-- if the // connection fails or is closed before returning. - ci, err := db.driver.Open(db.dsn) + ci, err := db.connector.Connect(context.Background()) db.mu.Lock() defer db.mu.Unlock() if db.closed { @@ -996,7 +1030,7 @@ func (db *DB) conn(ctx context.Context, strategy connReuseStrategy) (*driverConn db.numOpen++ // optimistically db.mu.Unlock() - ci, err := db.driver.Open(db.dsn) + ci, err := db.connector.Connect(ctx) if err != nil { db.mu.Lock() db.numOpen-- // correct for earlier optimism @@ -1454,7 +1488,7 @@ func (db *DB) beginDC(ctx context.Context, dc *driverConn, release func(error), // Driver returns the database's underlying driver. func (db *DB) Driver() driver.Driver { - return db.driver + return db.connector.Driver() } // ErrConnDone is returned by any operation that is performed on a connection diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index bcf0887d0e..fe7c3278c7 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -81,6 +81,13 @@ func newTestDB(t testing.TB, name string) *DB { return db } +func TestOpenDB(t *testing.T) { + db := OpenDB(dsnConnector{dsn: fakeDBName, driver: fdriver}) + if db.Driver() != fdriver { + t.Fatalf("OpenDB should return the driver of the Connector") + } +} + func TestDriverPanic(t *testing.T) { // Test that if driver panics, database/sql does not deadlock. db, err := Open("test", fakeDBName) @@ -1672,7 +1679,7 @@ func TestIssue4902(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) - driver := db.driver.(*fakeDriver) + driver := db.Driver().(*fakeDriver) opens0 := driver.openCount var stmt *Stmt @@ -1765,7 +1772,7 @@ func TestMaxOpenConns(t *testing.T) { db := newTestDB(t, "magicquery") defer closeDB(t, db) - driver := db.driver.(*fakeDriver) + driver := db.Driver().(*fakeDriver) // Force the number of open connections to 0 so we can get an accurate // count for the test @@ -2057,7 +2064,7 @@ func TestConnMaxLifetime(t *testing.T) { db := newTestDB(t, "magicquery") defer closeDB(t, db) - driver := db.driver.(*fakeDriver) + driver := db.Driver().(*fakeDriver) // Force the number of open connections to 0 so we can get an accurate // count for the test @@ -2146,7 +2153,7 @@ func TestStmtCloseDeps(t *testing.T) { db := newTestDB(t, "magicquery") defer closeDB(t, db) - driver := db.driver.(*fakeDriver) + driver := db.Driver().(*fakeDriver) driver.mu.Lock() opens0 := driver.openCount @@ -3071,7 +3078,7 @@ func TestIssue6081(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) - drv := db.driver.(*fakeDriver) + drv := db.Driver().(*fakeDriver) drv.mu.Lock() opens0 := drv.openCount closes0 := drv.closeCount @@ -3326,7 +3333,7 @@ func TestConnectionLeak(t *testing.T) { // Now we have defaultMaxIdleConns busy connections. Open // a new one, but wait until the busy connections are released // before returning control to DB. - drv := db.driver.(*fakeDriver) + drv := db.Driver().(*fakeDriver) drv.waitCh = make(chan struct{}, 1) drv.waitingCh = make(chan struct{}, 1) var wg sync.WaitGroup