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>

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index e11a9dadd012f84eb3756a5793ab0d62b4ee963e..3d957e1450cf40c88a1ba4554d19cdd7c125c64c 100644 (file)
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -1449,14 +1449,14 @@ func (tx *Tx) closePrepared() {
 
 // Commit commits the transaction.
 func (tx *Tx) Commit() error {
+       if tx.isDone() {
+               return ErrTxDone
+       }
        select {
        default:
        case <-tx.ctx.Done():
                return tx.ctx.Err()
        }
-       if tx.isDone() {
-               return ErrTxDone
-       }
        var err error
        withLock(tx.dc, func() {
                err = tx.txi.Commit()

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 02746a2e30070c03d7489d1426cf842b70c974cc..1ec62178587d81ea445a3b92a24be620b38fb968 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -683,6 +683,37 @@ func TestQueryRow(t *testing.T) {
        }
 }
 
+func TestTxRollbackCommitErr(t *testing.T) {
+       db := newTestDB(t, "people")
+       defer closeDB(t, db)
+
+       tx, err := db.Begin()
+       if err != nil {
+               t.Fatal(err)
+       }
+       err = tx.Rollback()
+       if err != nil {
+               t.Errorf("expected nil error from Rollback; got %v", err)
+       }
+       err = tx.Commit()
+       if err != ErrTxDone {
+               t.Errorf("expected %q from Commit; got %q", ErrTxDone, err)
+       }
+
+       tx, err = db.Begin()
+       if err != nil {
+               t.Fatal(err)
+       }
+       err = tx.Commit()
+       if err != nil {
+               t.Errorf("expected nil error from Commit; got %v", err)
+       }
+       err = tx.Rollback()
+       if err != ErrTxDone {
+               t.Errorf("expected %q from Rollback; got %q", ErrTxDone, err)
+       }
+}
+
 func TestStatementErrorAfterClose(t *testing.T) {
        db := newTestDB(t, "people")
        defer closeDB(t, db)