Commit graph

89 commits

Author SHA1 Message Date
Tamir Duberstein
1091b50cc0 database/sql: remove duplicate validation
Since https://golang.org/cl/38533, this validation is performed in
driverArgs.

Change-Id: I13a3ca46a1aa3197370de1095fb46ab83ea4628c
Reviewed-on: https://go-review.googlesource.com/91115
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-13 21:25:13 +00:00
HaraldNordgren
8da7706bb4 database/sql: include SQL column name in Scan() error message
When 'convertAssign' gives an error, instead of giving just the index of
the failing column -- which is not always helpful, especially when there
are lots of columns in the query -- utilize 'rs.rowsi.Columns()' to
extract the underlying column name and include that in the error string:

    sql: Scan error on column index 0, name "some_column": ...

Fixes #23362

Change-Id: I0fe71ff3c25f4c0dd9fc6aa2c2da2360dd93e3e0
Reviewed-on: https://go-review.googlesource.com/86537
Reviewed-by: Harald Nordgren <haraldnordgren@gmail.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-02-13 21:24:53 +00:00
Daniel Theophanes
78583a125a database/sql: fix nil pointer use within withLock
During the refactor in 1126d1483f I
introduced a logical error within one withLock function that used
the result of the call before checking for the error. Change
the order so that the error is checked before the result is used.

None of the other withLock uses have similar issues.

Fixes #23208

Change-Id: I6c5dcf262e36bad4369c850f1e0131066360a82e
Reviewed-on: https://go-review.googlesource.com/85175
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-01-03 18:18:40 +00:00
kirk
bcf964de5e database/sql: fix transaction leak
When the user context which passed in (*DB)BeginTx is canceled or
timeout, the current implementation could cause db transaction leak
in some extreme scenario.

Goroutine 1:
        Call (*DB) BeginTx begins a transaction with a userContext.
        In (*DB)BeginTx, a new goroutine (*Tx)awaitDone
        which monitor context and rollback tx if needed will be created

Goroutine 2(awaitDone):
        block on tx.ctx.Done()

Goroutine 1:
        Execute some insert or update sqls on the database

Goroutine 1:
        Commit the transaction, (*Tx)Commit set
        the atomic variable tx.done to 1

Goroutine 3(maybe global timer):
        Cancel userContext which be passed in Tx

Goroutine 1:
        (*Tx)Commit checks tx.ctx.Done().
        Due to the context has been canceled, it will return
        context.Canceled or context.DeadlineExceeded error immediately
        and abort the real COMMIT operation of transaction

Goroutine 2:
        Release with tx.ctx.Done() signal, execute (*Tx)rollback.
        However the atomic variable tx.done is 1 currently,
        it will return ErrTxDone error immediately and
        abort the real ROLLBACK operation of transaction

Fixes #22976

Change-Id: I3bc23adf25db823861d91e33d3cca6189fb1171d
Reviewed-on: https://go-review.googlesource.com/81736
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2017-12-05 16:47:21 +00:00
Daniel Theophanes
262141a12a database/sql: check for arg counts after eliminating arguments
Check for the expected number of arguments in a SQL statement
after arguments are eliminated in the argument converter.

This situation was already tested for in TestNamedValueChecker.
However the test used Exec which didn't have any check for
NumInput on it at all, thus this issue was never caught.

In addition to moving the NumInput check on the Query
methods after the converter, add the NumInput check
to the Exec methods as well.

Fixes #22630

Change-Id: If45920c6e1cf70dca63822a0cedec2cdc5cc611c
Reviewed-on: https://go-review.googlesource.com/76732
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-11-18 01:52:49 +00:00
Daniel Theophanes
b44b2feb91 database/sql: allow OpenConnector in a driver.Driver interface
While driver.Connector was previously added to allow non-string
connection arguments and access to the context, most users of
the sql package will continue to rely on a string DSN.

Allow drivers to implement a string DSN to Connector interface
that both allows a single parsing of the string DSN and uses
the Connector interface which passes available context to
the driver dialer.

Fixes #22713

Change-Id: Ia0b862262f4c4670effe2538d0d6d43733fea18d
Reviewed-on: https://go-review.googlesource.com/77550
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
2017-11-16 22:36:07 +00:00
Russ Cox
442d15abc3 database/sql/driver: rename ResetSessioner to SessionResetter
Originally we tried the strict -er suffix as the rule in this case
but eventually we decided it was too awkward: io.WriteByter
became io.ByteWriter. By analogy, here the interface should be
named SessionResetter instead of the awkward ResetSessioner.

This change should not affect any drivers that have already
implemented the interface, because the method name is not changing.

(This was added during the Go 1.10 cycle and has not been
released yet, so we can change it.)

Change-Id: Ie50e4e090d3811f85965da9da37d966e9f45e79d
Reviewed-on: https://go-review.googlesource.com/78127
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2017-11-15 21:28:18 +00:00
Daniel Theophanes
f7df55d174 database/sql: do not leak the connectionResetter goroutine
Before terminating the connectionResetter goroutine the connection
pool processes all of the connections on the channel to unlock the
driverConn instances so everthing can shutdown cleanly. However
the channel was never closed so the goroutine hangs on the range.
Close the channel prior to ranging over it. Also prevent additional
connections from being sent to the resetter after the connection
pool has been closed.

Fixes #22699

Change-Id: I440d2b13cbedec2e04621557f5bd0b1526933dd7
Reviewed-on: https://go-review.googlesource.com/77390
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-14 00:25:42 +00:00
Daniel Theophanes
1126d1483f database/sql: ensure all driver interfaces are called under single lock
Russ pointed out in a previous CL golang.org/cl/65731 that not only
was the locking incomplete, previous changes did not correctly
lock driver calls in other sections. After inspecting
driverConn, driverStmt, driverResult, Tx, and Rows structs
where driver interfaces are stored, I discovered a few more places
that failed to lock driver calls. The largest of these
was the parameter type converter "driverArgs".

driverArgs was typically called right before another call to the
driver in a locked region, so I made the entire driverArgs expect
a locked driver mutex and combined the region. This should not
be a problem because the connection is pulled out of the connection
pool either way so there shouldn't be contention.

Fixes #21117

Change-Id: I88d46f74dca25fb11a30f0bf8e79785a73133d23
Reviewed-on: https://go-review.googlesource.com/71433
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-25 17:21:58 +00:00
Daniel Theophanes
6a223b82a4 database/sql: add driver.ResetSessioner and add pool support
A single database connection ususally maps to a single session.
A connection pool is logically also a session pool. Most
sessions have a way to reset the session state which is desirable
to prevent one bad query from poisoning another later query with
temp table name conflicts or other persistent session resources.

It also lets drivers provide users with better error messages from
queryies when the underlying transport or query method fails.
Internally the driver connection should now be marked as bad, but
return the actual connection. When ResetSession is called on the
connection it should return driver.ErrBadConn to remove it from
the connection pool. Previously drivers had to choose between
meaningful error messages or poisoning the connection pool.

Lastly update TestPoolExhaustOnCancel from relying on a
WAIT query fixing a flaky timeout issue exposed by this
change.

Fixes #22049
Fixes #20807

Change-Id: I2b5df6d954a38d0ad93bf1922ec16e74c827274c
Reviewed-on: https://go-review.googlesource.com/73033
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-24 21:37:46 +00:00
Russ Cox
3b9d947b2f Revert "database/sql: add driver.ResetSessioner and add pool support"
This reverts commit 2620ac3aea.

Reason for revert: broke all the builds.

Change-Id: I26fc09a13f5f80fa708de66c843442ff9d934694
Reviewed-on: https://go-review.googlesource.com/73050
Reviewed-by: Russ Cox <rsc@golang.org>
2017-10-24 18:43:18 +00:00
Daniel Theophanes
2620ac3aea database/sql: add driver.ResetSessioner and add pool support
A single database connection ususally maps to a single session.
A connection pool is logically also a session pool. Most
sessions have a way to reset the session state which is desirable
to prevent one bad query from poisoning another later query with
temp table name conflicts or other persistent session resources.

It also lets drivers provide users with better error messages from
queryies when the underlying transport or query method fails.
Internally the driver connection should now be marked as bad, but
return the actual connection. When ResetSession is called on the
connection it should return driver.ErrBadConn to remove it from
the connection pool. Previously drivers had to choose between
meaningful error messages or poisoning the connection pool.

Lastly update TestPoolExhaustOnCancel from relying on a
WAIT query fixing a flaky timeout issue exposed by this
change.

Fixes #22049
Fixes #20807

Change-Id: Idffa1a7ca9ccfe633257c4a3ae299b864f46c5b6
Reviewed-on: https://go-review.googlesource.com/67630
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-24 17:02:13 +00:00
Daniel Theophanes
532714829e database/sql: allow drivers to only implement Context variants
Drivers shouldn't need to implement both Queryer and QueryerContext,
they should just implement QueryerContext. Same with Execer and
ExecerContext. This CL tests for QueryContext and ExecerContext
first so drivers do not need to implement Queryer and Execer
with an empty definition.

Fixes #21663

Change-Id: Ifbaa71da669f4bc60f8da8c41a04a4afed699a9f
Reviewed-on: https://go-review.googlesource.com/65733
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-24 16:51:29 +00:00
Tim Cooper
a9afa4e933 database/sql: fix example usage of Out
Fixes #22292

Change-Id: I016e5f2b8624a6ebaf11a693a0f6d94d7b0d3f92
Reviewed-on: https://go-review.googlesource.com/71550
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2017-10-18 03:07:46 +00:00
Daniel Theophanes
292366e716 Revert "database/sql: prevent race in driver by locking dc in Next"
This reverts commit 897080d5cb.

Reason for revert: Fails to fix all the locking issues.

Updates #21117

Change-Id: I6fc9cb7897244d6e1af78c089a2bf383258ec049
Reviewed-on: https://go-review.googlesource.com/71450
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-17 23:02:14 +00:00
Daniel Theophanes
897080d5cb database/sql: prevent race in driver by locking dc in Next
Database drivers should be called from a single goroutine to ease
driver's design. If a driver chooses to handle context
cancels internally it may do so.

The sql package violated this agreement when calling Next or
NextResultSet. It was possible for a concurrent rollback
triggered from a context cancel to call a Tx.Rollback (which
takes a driver connection lock) while a Rows.Next is in progress
(which does not tack the driver connection lock).

The current internal design of the sql package is each call takes
roughly two locks: a closemu lock which prevents an disposing of
internal resources (assigning nil or removing from lists)
and a driver connection lock that prevents calling driver code from
multiple goroutines.

Fixes #21117

Change-Id: Ie340dc752a503089c27f57ffd43e191534829360
Reviewed-on: https://go-review.googlesource.com/65731
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-10-13 18:11:41 +00:00
Daniel Theophanes
14a1d934b6 database/sql: update minor sql docs
Replace the work "session" with "connection" in docs. Fix
The ErrConnDone documentation. Clarify what the context is used
for in StmtContext.

Change-Id: I2f07e58d0cd6321b386a73b038cf6070cb8e2572
Reviewed-on: https://go-review.googlesource.com/65732
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-09-24 13:58:38 +00:00
James Lawrence
e6358c798b 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 <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-09-23 20:47:06 +00:00
Davor Kapsa
e16dc7d5a9 database/sql: fix Conn's doc typo
Fixes #21798

Change-Id: Ided31a8b22c220acdeb3938cac41ce8db9a110c3
Reviewed-on: https://go-review.googlesource.com/62290
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2017-09-08 13:49:40 +00:00
Alexey Palazhchenko
e9b9dfe3f7 database/sql: fix wrong method name in description
Change-Id: Ie6a88b70d7c45c59995ee2f57fb28f9a3cbb404d
Reviewed-on: https://go-review.googlesource.com/49470
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-07-18 04:42:09 +00:00
Daniel Theophanes
cd24a8a550 database/sql: ensure a Stmt from a Conn executes on the same driver.Conn
Ensure a Stmt prepared on a Conn executes on the same driver.Conn.
This also removes another instance of duplicated prepare logic
as a side effect.

Fixes #20647

Change-Id: Ia00a19e4dd15e19e4d754105babdff5dc127728f
Reviewed-on: https://go-review.googlesource.com/45391
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-06-13 19:16:54 +00:00
Sarah Adams
3bcdbe57b6 database/sql: properly document QueryRow
Fixes golang/go#20163

Change-Id: I0caf95fc84aa502715848151c93b6e7bee003ea5
Reviewed-on: https://go-review.googlesource.com/44890
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2017-06-12 20:17:47 +00:00
Daniel Theophanes
b0d592c3c9 database/sql: prevent race on Rows close with Tx Rollback
In addition to adding a guard to the Rows close, add a var
in the fakeConn that gets read and written to on each
operation, simulating writing or reading from the server.

TestConcurrency/TxStmt* tests have been commented out
as they now fail after checking for races on the fakeConn.
See issue #20646 for more information.

Fixes #20622

Change-Id: I80b36ea33d776e5b4968be1683ff8c61728ee1ea
Reviewed-on: https://go-review.googlesource.com/45275
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-06-12 15:53:00 +00:00
Daniel Theophanes
3820191839 Revert "database/sql: Use Tx.ctx in Tx non-context methods"
This reverts commit ef0f7fb92b.

Reason for revert: Altered behavior of Queries prior to Tx commit. See #20631.

Change-Id: I2548507c2935a7c60b92aae377dcc8e9aca66331
Reviewed-on: https://go-review.googlesource.com/45231
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Bulat Gaifullin <gaifullinbf@gmail.com>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-06-12 15:50:16 +00:00
Bulat Gaifullin
ef0f7fb92b database/sql: Use Tx.ctx in Tx non-context methods
The Tx methods Query and Exec uses context.Background()
even Tx was created by context.
This patch enables using Tx.ctx in all Tx methods
which do not has context arg.
Backward compatibility:
- If Tx has created without context, nothing changes.
- If Tx has created with context and non-context method is called:
  - If context is expired, the execution fails,
    but it can fail on Commit or Rollback as well,
    so in terms of whole transaction - nothing changes.
  - If context is not expired, nothing changes too.

Fixes #20098
Change-Id: I9570a2deaace5875bb4c5dcf7b3a084a6bcd0d00
Reviewed-on: https://go-review.googlesource.com/44956
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-06-07 21:15:36 +00:00
Daniel Theophanes
729685c1d1 database/sql: ensure Rows is closed when Tx closes
Close any Rows queried within a Tx when the Tx is closed. This prevents
the Tx from blocking on rollback if a Rows query has not been closed yet.

Fixes #20575

Change-Id: I4efe9c4150e951d8a0f1c40d9d5e325964fdd608
Reviewed-on: https://go-review.googlesource.com/44812
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-06-05 19:48:49 +00:00
Daniel Theophanes
1865a56e8d database/sql: allow Stmt Query and Exec methods to open a new conn
Query and Exec functions on DB first attempt to get a cached
connection before requesting the connection pool to ignore
the cache and get a new connection. This change aligns Stmt to
that behavior as well.

Fixes #20433

Change-Id: Idda5f61927289d7ad0882effa3a50ffc9efd88e6
Reviewed-on: https://go-review.googlesource.com/43790
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-05-22 15:02:35 +00:00
Daniel Theophanes
a9bf3b2e19 database/sql: allow drivers to support custom arg types
Previously all arguments were passed through driver.IsValid.
This checked arguments against a few fundamental go types and
prevented others from being passed in as arguments.

The new interface driver.NamedValueChecker may be implemented
by both driver.Stmt and driver.Conn. This allows
this new interface to completely supersede the
driver.ColumnConverter interface as it can be used for
checking arguments known to a prepared statement and
arbitrary query arguments. The NamedValueChecker may be
skipped with driver.ErrSkip after all special cases are
exhausted to use the default argument converter.

In addition if driver.ErrRemoveArgument is returned
the argument will not be passed to the query at all,
useful for passing in driver specific per-query options.

Add a canonical Out argument wrapper to be passed
to OUTPUT parameters. This will unify checks that need to
be written in the NameValueChecker.

The statement number check is also moved to the argument
converter so the NamedValueChecker may remove arguments
passed to the query.

Fixes #13567
Fixes #18079
Updates #18417
Updates #17834
Updates #16235
Updates #13067
Updates #19797

Change-Id: I89088bd9cca4596a48bba37bfd20d987453ef237
Reviewed-on: https://go-review.googlesource.com/38533
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-05-18 22:22:31 +00:00
Daniel Theophanes
2133d63fa8 database/sql: ensure releaseConn is defined before a possible close
When running a Query on Stmt a dependency is added to the stmt and
rows. To do that it needs a reference to Rows, so the releaseConn
function is defined after the definition. However the
rows.initContextClose was set to run before the releaseConn was
set on rows, setting up a situation where the connection could
be canceled before the releaseConn was set and resulting in
a segfault.

Fixes #20160

Change-Id: I5592e7db2cf653dfc48d42cbc2b03ca20501b1a0
Reviewed-on: https://go-review.googlesource.com/42139
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-28 22:55:26 +00:00
Daniel Theophanes
d234f9a754 database/sql: allow using a single connection from the database
Databases have the following concepts: Statement, Batch, and Session.

A statement is often a single line like:
SELECT Amount from Account where ID = 50;

A batch is one or more statements submitted together for the query
to process. It may be a DELETE, INSERT, two UPDATES and a SELECT in
a single query text.

A session is usually represented by a single database connection.
This often is an issue when dealing with scopes in databases.
Temporary tables and variables can have batch, session, or global
scope depending on the syntax, database, and use.

Furthermore, some databases (sybase and derivatives in perticular)
that prevent certain statements from being in the same batch
and may necessitate being in the same session.

By allowing users to extract a Conn from the database they can manage
session on their own without hacking around it by making connection
pools of single connections (a real workaround presented in issue).
It is tempting to just use a transaction, but this isn't always
desirable or an option if running an interactive session or
alter script set that itself starts transactions.

Fixes #18081

Change-Id: I9bdf0796632c48d4bcaef3624c629641984ffaf2
Reviewed-on: https://go-review.googlesource.com/40694
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-24 17:03:05 +00:00
Daniel Theophanes
dec95d8fad database/sql: correctly guard the query Row preventing early release
When a Tx starts a query, prevent returning the connection to the pool
until after the query finishes.

Fixes #19058

Change-Id: I2c0480d9cca9eeb173b5b3441a5aeed6f527e0ac
Reviewed-on: https://go-review.googlesource.com/40400
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-04-12 17:36:26 +00:00
Daniel Theophanes
ab0e9019ea database/sql: de-duplicate various methods
Form a new method pattern where *driverConn and
release functions are passed into the method.
They are named DB.execDC, DB.queryDC, DB.beginDC. This
allows more code to be de-duplicated when starting
queries.

The Stmt creation and management code are untouched.

Change-Id: I24c853531e511d8a4bc1f53dd4dbdf968763b4e7
Reviewed-on: https://go-review.googlesource.com/39630
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-12 01:50:31 +00:00
Sarah Adams
82e1732f14 database/sql: proper prepared statement support in transactions
This change was originally written by Marko Tiikkaja <marko@joh.to>.
https://go-review.googlesource.com/#/c/2035/

Previously *Tx.Stmt always prepared a new statement, even if an
existing one was available on the connection the transaction was on.
Now we first see if the statement is already available on the
connection and only prepare if it isn't. Additionally, when we do
need to prepare one, we store it in the parent *Stmt to allow it to be
later reused by other calls to *Tx.Stmt on that statement or just
straight up by *Stmt.Exec et al.

To make sure that the statement doesn't disappear unexpectedly, we
record a dependency from the statement returned by *Tx.Stmt to the
*Stmt it came from and set a new field, parentStmt, to point to the
originating *Stmt. When the transaction's *Stmt is closed, we remove
the dependency. This way the "parent" *Stmt can be closed by the user
without her having to know whether any transactions are still using it
or not.

Fixes #15606

Change-Id: I41b5056847e117ac61130328b0239d1e000a4a08
Reviewed-on: https://go-review.googlesource.com/35476
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2017-03-08 17:32:12 +00:00
Daniel Theophanes
a335c344fa database/sql: ensure driverConns are closed if not returned to pool
Previously if a connection was requested but timed out during the
request and when acquiring the db.Lock the connection request
is fulfilled and the request is unable to be returned to the
connection pool, then then driver connection would not be closed.

No tests were added or modified because I was unable to determine
how to trigger this situation without something invasive.

Change-Id: I9d4dc680e3fdcf63d79d212174a5b8b313f363f1
Reviewed-on: https://go-review.googlesource.com/36641
Reviewed-by: Russ Cox <rsc@golang.org>
2017-02-10 13:28:32 +00:00
Daniel Theophanes
4f6d4bb3f4 database/sql: do not exhaust connection pool on conn request timeout
Previously if a context was canceled while it was waiting for a
connection request, that connection request would leak.

To prevent this remove the pending connection request if the
context is canceled and ensure no connection has been sent on the channel.
This requires a change to how the connection requests are represented in the DB.

Fixes #18995

Change-Id: I9a274b48b8f4f7ca46cdee166faa38f56d030852
Reviewed-on: https://go-review.googlesource.com/36563
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-02-09 01:12:19 +00:00
Kevin Burke
c57d91e34c database/sql: fix typo
Change-Id: I09fdcebb939417f18af09ed57f24460724cab64f
Reviewed-on: https://go-review.googlesource.com/36632
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-02-09 01:08:08 +00:00
Daniel Theophanes
c026845bd2 database/sql: record the context error in Rows if canceled
Previously it was intended that Rows.Scan would return
an error and Rows.Err would return nil. This was problematic
because drivers could not differentiate between a normal
Rows.Close or a context cancel close.

The alternative is to require drivers to return a Scan to return
an error if the driver is closed while there are still rows to be read.
This is currently not how several drivers currently work and may be
difficult to detect when there are additional rows.

At the same time guard the the Rows.lasterr and prevent a close
while a Rows operation is active.

For the drivers that do not have Context methods, do not check for
context cancelation after the operation, but before for any operation
that may modify the database state.

Fixes #18961

Change-Id: I49a25318ecd9f97a35d5b50540ecd850c01cfa5e
Reviewed-on: https://go-review.googlesource.com/36485
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-02-08 18:30:39 +00:00
Kevin Burke
ec4062f8ea database/sql: fix spelling mistake
Change-Id: I67db3b342929a7bd11f01bf3b9afb49f4da69a0a
Reviewed-on: https://go-review.googlesource.com/35841
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-02-01 21:06:35 +00:00
Daniel Theophanes
2b283cedef database/sql: fix race when canceling queries immediately
Previously the following could happen, though in practice it would
be rare.

Goroutine 1:
	(*Tx).QueryContext begins a query, passing in userContext

Goroutine 2:
	(*Tx).awaitDone starts to wait on the context derived from the passed in context

Goroutine 1:
	(*Tx).grabConn returns a valid (*driverConn)
	The (*driverConn) passes to (*DB).queryConn

Goroutine 3:
	userContext is canceled

Goroutine 2:
	(*Tx).awaitDone unblocks and calls (*Tx).rollback
	(*driverConn).finalClose obtains dc.Mutex
	(*driverConn).finalClose sets dc.ci = nil

Goroutine 1:
	(*DB).queryConn obtains dc.Mutex in withLock
	ctxDriverPrepare accepts dc.ci which is now nil
	ctxCriverPrepare panics on the nil ci

The fix for this is to guard the Tx methods with a RWLock
holding it exclusivly when closing the Tx and holding a read lock
when executing a query.

Fixes #18719

Change-Id: I37aa02c37083c9793dabd28f7f934a1c5cbc05ea
Reviewed-on: https://go-review.googlesource.com/35550
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-01-26 06:25:37 +00:00
shawnps
067bab00a8 all: fix misspellings
Change-Id: I429637ca91f7db4144f17621de851a548dc1ce76
Reviewed-on: https://go-review.googlesource.com/34923
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-01-07 16:53:25 +00:00
Daniel Theophanes
9def857072 database/sql: prevent Tx.rollback from racing Tx.close
Previously Tx.done was being set in close, but in a Tx
rollback and Commit are the real closing methods,
and Tx.close is just a helper common to both. Prior to this
change a multiple rollback statements could be called, one
would enter close and begin closing it while the other was
still in rollback breaking it. Fix that by setting done
in rollback and Commit, not in Tx.close.

Fixes #18429

Change-Id: Ie274f60c2aa6a4a5aa38e55109c05ea9d4fe0223
Reviewed-on: https://go-review.googlesource.com/34716
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-01-02 20:21:02 +00:00
Daniel Theophanes
d0501f1da9 database/sql: do not store Tx options in Context
Drivers which previously supported tip will need to update to this
revision before release.

Fixes #18284

Change-Id: I70b8e7afff1558a8b5348885ce9f50e067c72ee9
Reviewed-on: https://go-review.googlesource.com/34330
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-12-14 18:13:13 +00:00
Brad Fitzpatrick
454c234397 database/sql: use complete sentences in new docs
Change-Id: Icb842a80cab2b07b9ace1e8e14c4a19c48a92c43
Reviewed-on: https://go-review.googlesource.com/34247
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
2016-12-09 20:19:06 +00:00
Daniel Theophanes
e12ce1e400 database/sql: document that drivers may not return right after cancel
Fixes #18168

Change-Id: Idbfe3d4daedd93c7caf6f1770ecd988e9af39949
Reviewed-on: https://go-review.googlesource.com/34144
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-12-09 03:15:22 +00:00
Daniel Theophanes
ae9712c766 database/sql: ensure Commit and Rollback return ErrTxDone
Ensure documented behavior of returning ErrTxDone if the Tx has
already been committed or rolled back.

Fixes #18147

Change-Id: I07dc75bef4dbd4dd88dd252c96dc8ab99f28c00e
Reviewed-on: https://go-review.googlesource.com/33793
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2016-12-01 22:20:31 +00:00
Daniel Theophanes
e5e0562774 database/sql: document expectations for named parameters
Require parameter names to not begin with a symbol.

Change-Id: I5dfe9d4e181f0daf71dad2f395aca41c68678cbe
Reviewed-on: https://go-review.googlesource.com/33493
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-12-01 16:52:12 +00:00
Daniel Theophanes
2b1abf7594 database/sql: rename NamedParam to NamedArg and Param to Named
Be consistent with the argument names already provided. Also
parameter is the variable, argument is the value.

Fixes #18099

Change-Id: Idb3f4e9ffc214036c721ddb4f614ec6c95bb7778
Reviewed-on: https://go-review.googlesource.com/33660
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-11-30 16:42:58 +00:00
Daniel Theophanes
0d163ce1c9 database/sql: do not bypass the driver locks with Context methods
When context methods were initially added it was attempted to unify
behavior between drivers without Context methods and those with
Context methods to always return right away when the Context expired.
However in doing so the driver call could be executed outside of the
scope of the driver connection lock and thus bypassing thread safety.

The new behavior waits until the driver operation is complete. It then
checks to see if the context has expired and if so returns that error.

Change-Id: I4a5c7c3263420c57778f36a5ed6fa0ef8cb32b20
Reviewed-on: https://go-review.googlesource.com/32422
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-29 18:52:38 +00:00
Brad Fitzpatrick
f7b2f58cda database/sql: force users of NamedParam to name struct literals fields
Or they can use sql.Param instead.

Change-Id: Icf21dbcc87170635c3f5d3f49736429a37abe9da
Reviewed-on: https://go-review.googlesource.com/33576
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Minux Ma <minux@golang.org>
2016-11-24 23:32:46 +00:00
Daniel Theophanes
90b8a0ca2d database/sql: ensure all driver Stmt are closed once
Previously  driver.Stmt could could be closed multiple times in
edge cases that drivers may not test for initially. Make their
job easier by ensuring the driver is only closed a single time.

Fixes #16019

Change-Id: I1e4777ef70697a849602e6ef9da73054a8feb4cd
Reviewed-on: https://go-review.googlesource.com/33352
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-11-17 18:13:41 +00:00