code review 6845094: database/sql: Alter *DB.begin to return driver.ErrB...

database/sql: Alter *DB.begin to return driver.ErrBadConn when driver.Conn.Begin returns driver.ErrBadConn
Fixes issue 4433

[chalfant, 2012-12-13 00:26:25]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2012-12-13 05:58:13]

LGTM

I'd prefer a test, but this is "obviously correct" from local inspection
and all the existing tests pass.

Bonus points if you come back with a test that would've failed before this
and now passes.

On Wed, Dec 12, 2012 at 4:26 PM, <james.chalfant@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
>     database/sql: Alter *DB.begin to return driver.ErrBadConn when
> driver.Conn.Begin returns driver.ErrBadConn
> Fixes issue 4433
>
> Please review this at
https://codereview.appspot.**com/6845094/<https://codereview.appspot.com/6845...
>
> Affected files:
>   M src/pkg/database/sql/sql.go
>
>
> Index: src/pkg/database/sql/sql.go
> ==============================**==============================**=======
> --- a/src/pkg/database/sql/sql.go
> +++ b/src/pkg/database/sql/sql.go
> @@ -426,7 +426,7 @@
>         txi, err := ci.Begin()
>         if err != nil {
>                 db.putConn(ci, err)
> -               return nil, fmt.Errorf("sql: failed to Begin transaction:
> %v", err)
> +               return nil, err
>         }
>         return &Tx{
>                 db:  db,
>
>
>

[bradfitz, 2012-12-13 06:05:03]

*** Submitted as https://code.google.com/p/go/source/detail?r=22be5b0e2f1e ***

    database/sql: Alter *DB.begin to return driver.ErrBadConn when
driver.Conn.Begin returns driver.ErrBadConn
Fixes issue 4433

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/6845094

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[chalfant, 2012-12-14 08:18:59]

As requested, here's a change with a test that now passes which would have
failed.

https://codereview.appspot.com/6942050

Thanks,
-James

On Dec 12, 2012, at 9:58 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> LGTM
> 
> I'd prefer a test, but this is "obviously correct" from local inspection and
all the existing tests pass.
> 
> Bonus points if you come back with a test that would've failed before this and
now passes.
> 
> On Wed, Dec 12, 2012 at 4:26 PM, <james.chalfant@gmail.com> wrote:
> Reviewers: golang-dev_googlegroups.com,
> 
> Message:
> Hello golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go
> 
> 
> Description:
>     database/sql: Alter *DB.begin to return driver.ErrBadConn when
> driver.Conn.Begin returns driver.ErrBadConn
> Fixes issue 4433
> 
> Please review this at https://codereview.appspot.com/6845094/
> 
> Affected files:
>   M src/pkg/database/sql/sql.go
> 
> 
> Index: src/pkg/database/sql/sql.go
> ===================================================================
> --- a/src/pkg/database/sql/sql.go
> +++ b/src/pkg/database/sql/sql.go
> @@ -426,7 +426,7 @@
>         txi, err := ci.Begin()
>         if err != nil {
>                 db.putConn(ci, err)
> -               return nil, fmt.Errorf("sql: failed to Begin transaction: %v",
err)
> +               return nil, err
>         }
>         return &Tx{
>                 db:  db,
> 
> 
>