Go: database/sql: nested transaction or save point support

[cznic]

It might be useful to consider supporting nested transactions when a particular
driver/DB combo is able to support that.

#Go 1.4+

[ianlancetaylor]

Comment 1:

_Labels changed: added repo-main, release-go1.4._

[rsc]

Comment 2:

_Labels changed: added release-go1.5, removed release-go1.4._

_Status changed to Accepted._

[amoghe]

Github reports this as targetting 1.5, but the 1.5 beta release has no relevant changes to the database/sql package.

Any updates on whether this will make it or be pushed out once again?

[ianlancetaylor]

I think it's getting pushed out again. Sorry.

[amoghe]

:'(

Datapoint: This (lack of) caused me much pain when migrating an older project from ruby(rails) to go.

[ianlancetaylor]

I completely agree that this is desirable, but somebody has to step up and do the work. Perhaps you could tackle it, for 1.6?

[amoghe]

I'll take a stab at it.

[amoghe]

In order to support nested transactions, I see two obvious alternatives.

TL;DR - Option 2 is better.

Option 1

Make the driver.Tx interface include an additional function

type Tx interface {
        Commit() error
        Rollback() error
        Begin() (*Tx, error)
}

The Begin() function can return an error indicating that transactions cannot be nested (any further). This would allow the sql.Tx object to expose a Begin(), which directly calls the underlying Begin function on the driver.Tx (which it wraps). E.g.

// - - - - - - - - - - - - - - - - - 
// In package sql
//

func (tx *Tx) Begin() (*Tx, error) {
      return tx.txi.Begin()
}

// - - - - - - - - - - - - - - - - - 
// In the driver implementation package
//

func (t *MyTx) Begin() (*Tx, error) {
      // error sanity checks
      _, err  := t.conn.Exec(...) // execute a SAVEPOINT maybe?
      if err != nil {
            return nil, err
      }
      newTx := t.clone()
      // some bookkeeping for newTx
      return &newTx, nil
}

The immediate drawback of this approach is that the next release will mean existing DB drivers will immediately stop compiling since their implementations of transaction object will no longer satisfy the driver.Tx interface.

However, the benefit is that having the transaction struct (that implements driver.Tx) implement the function that begins the nested transaction feels more natural. It is likely* that the transaction already holds a reference to the driver.Conn that it is tied to, so all the state it needs is already present in the transaction struct itself (* = my reading of 2 driver implementations)

Option 2

Have driver.Conn optionally implement an additional interface (driver.NestedBeginner?) which indicates that it can begin nested transactions

type NestedBeginner interface {
  NestedBegin(origTx *Tx) Tx
}

And expose this on the sql.Tx object as a Begin() function, which calls the NestedBegin() on the conn object, passing it the current transaction. E.g.

// - - - - - - - - - - - - - - - - - 
// In package sql
//

var ErrNestedTxUnsupported = errors.New("sql: Driver does not support nested transactions")

func (tx *Tx) Begin() (*Tx, error) {
      if tx.done {
            return ErrTxDone
      }
      if beginner, ok := tx.dc.ci.(driver.NestedBeginner); ok {
            tx.dc.Lock()
            nestedTx, err := beginner.NestedBegin(tx.txi) 
            if err != nil {
                  tx.dc.Unlock()
                  return nil, err
            }
            tx.dc.Unlock()
            return &Tx{db:  tx.db, dc:  dc, txi: nestedTx}, nil
      }
      return nil, ErrNestedTxUnsupported
}

// - - - - - - - - - - - - - - - - - 
// In the driver implementation package
//

// This function makes the driver connection object satisfy the NestedBeginner interface
func (c *MyConn) NestedBegin(origTxn driver.Tx) (driver.Tx, error) {
      return origTxn.NestedBegin() 
}

func (t *MyTxn) NestedBegin() {
      // implementation (similar to proposal 1) goes here
}

The benefit of this approach is that nothing changes for existing driver implementations (they are deemed to not support nested transactions until the pkg maintainers make the Conn satisfy the new interface). The sql.Tx.Begin() returns an error if the underlying driver.Conn doesn't implement the function.

However, this means the driver.Conn has to implement the NestedBeginner interface. This, in turn, means that in order for a nested transaction to begin, the struct that implements the driver.Conn acts as a proxy to the actual function that likely needs to be invoked on the existing transaction, to start a new transaction. This could end up feeling slightly clunkier than the first option, although that is not as important as maintaining the interface contract for backwards compatibility.

Hence I believe that option 2 is the more desirable one. I'd appreciate thoughts/feedback on this.

Note

The semantics of the nested transaction (and how Commits/Rollback might cause interactions between the inner/outer transactions) are to be implemented by the underlying driver. The sql package simply propagates them.

Its not clear to me if these discussions are to be had on the golang-dev mailing list or on the bug itself (the instructions on golang.org didn't call it out explicitly). It seems that both places would be able to persist design discussions for posterity but I'm happy to post this on the mailing list if needed.

[amoghe]

ping.

Its not clear to me if these discussions are to be had on the golang-dev mailing list or on the bug itself (the instructions on golang.org didn't call it out explicitly)

Happy to post this wherever necessary so that this makes it into 1.6 (as tagged)

[bradfitz]

If there's a patch ready, send it via the normal channels and use the normal "Update #7898" or "Fixes #7898" in the commit message and the bot will ping this thread about it.

[kostya-sh]

@amoghe, what db with nested transactions support have you used? I beleive savepoints are more widely supported by db engines than nested transactions.

[amoghe]

@kostya-sh You are correct. This (proposed) interface would allow db drivers the ability to offer nested transactions in whatever way is most suitable for their particular database. It is likely that most of them will use savepoints to do this.

The alternative (IIUC, you are proposing?) is to offer savepoints in the stdlib api itself and leave the handling of nesting them to the user.

[kostya-sh]

@amoghe, you can use savepoints without any changes to database/sql. E.g. with postgresql:

tx, err := db.Begin()
tx.Exec("insert into t1 values ('go1')")
tx.Exec("savepoint s1")
tx.Exec("insert into t1 values ('go2')")
tx.Exec("rollback to savepoint s1")
tx.Commit()

[rsc]

We (or at least I) don't understand the semantics of what is being proposed, and there seems to be no agreement about what they should be. There is also no code, nor anyone signed up to write the code. Therefore this will not be in Go 1.6.

Moving this to the proposal process, since it seems like that more clearly reflect the current status. The next step would be to write a design doc outlining the approach to take. See golang.org/s/proposal for details. Thanks.

[adg]

There seems to be agreement that something should be done here, but we need someone to push it forward with a formal proposal.

[kardianos]

@bradfitz @amoghe @cznic
I propose we add to database/sql

func (*Tx) RollbackTo(name string) error
func (*Tx) SavePoint(name string) error

And add to database/sql/driver

type TxSavePoint interface {
  RollbackTo(ctx context.Context, name string) error
  SavePoint(ctx context.Context, name string) error
}

As @kostya-sh noted, nested transactions have interesting semantics and are generally not encouraged at this time. RDMBS ask you to use save points to support such needs.

Depends on #15123

[bradfitz]

@kardianos, you want to see this happen for Go 1.8? Which 2+ popular databases support this? Could you implement support for at least one popular database for Go 1.8 if we do this?

[kardianos]

MySql, Postgresql, MS SQL Server, Oracle, and sqlite all support this.

Once the context gets generally added, I'll work on this.

[amoghe]

@kardianos I can take a stab at the sqlite3 implementation.

However, I'm not sure thats the interface that'll work best for sqlite3. They recommend emulating nested transactions using savepoints. From their documentation:

SAVEPOINTs are a method of creating transactions, similar to BEGIN and COMMIT, except that the SAVEPOINT and RELEASE commands are named and may be nested

So exposing "SavePoint" on the Tx struct would mean that the user has already begun the txn using a non-savepoint mechanism. This is why my initial proposal was to modify the Txn interface itself. This is not to say that this interface won't work for sqlite3 , but I can imagine there exists some SQL database which needs you to use either transactions (BEGIN...COMMIT) or savepoints exclusively (without intermingling the two).

Thoughts?

[kardianos]

@amoghe wait to implement quite yet. Here is what I would suggest: One of the parameters to start a transaction (coming in DB.Being{Context | Level} can take a name. When the name is provided you start a savepoint (named tx). If you attempt a savepoint through a normal tx, then it will give you an error. Thoughts?

[gopherbot]

CL https://golang.org/cl/30165 mentions this issue.

[cznic]

CL 30165 adds support for save points. I don't see how this enables nested transaction support, which is what this issue is about.

Additionally, even though a particular database supports nested transactions, but not savepoints, like for example QL, the new API in driver.go seems to be not compatible with such databases capability. I'd be glad to be wrong, perhaps explain the mechanism to me, TIA.

I'm sorry to comment this late on the issue, but the subscription to notifications for issues filled before moving to Github were lost in the transition (known issue).

[kardianos]

@cznic Fair points. This CL does NOT support nested transactions. The databases I'm aware of (please correct me if I'm wrong) recommend not using true tested transactions, but switch to using savepoint / rollback to savepoint method.

There are a number of changes to the database/sql package, some are CL / being written, others committed. Until 1.8 comes I want as much feedback from other database gophers as possible.

[kardianos]

@cznic More specifically, I think you can use nested transactions on top of savepoint, rollback.

What if you maintained a savepoint to nest level mapping in your driver and then translated the name into the appropriate tx to rollback. After a savepoint, you would need to (under the hood) start using the new Tx in the driver. I think it is doable. Thoughts?

[cznic]

What if you maintained a savepoint to nest level mapping in your driver and then translated the name into the appropriate tx to rollback.

@kardianos That might be the way. Rollback to a savepoint which is not the innermost one would effectively erase all savepoints "bellow" it, but otherwise SGTM, IINM :+1:

[amoghe]

Apologies if I sound like I'm repeating myself. I'm trying to figure out of this discussion is tracking what the user facing API offered by the stdlib should be, or the underlying implementation, or something else completely? (like the viability of even supporting this?).

Assuming we're still discussing what uses might want from the sql pkg API ...

What if you maintained a savepoint to nest level mapping in your driver and then translated the name into the appropriate tx to rollback.

What option 1 proposes (see beginning of thread) are the API level changes we might need to support nested transactions. Its up to the driver to either emulate nested txns using savepoints, or maybe use true nested txns if supported by the database. I'm assuming the user facing API for this should allow for this kind of usage:

tx1 := db.Begin()
// do some stuff
tx2 := tx1.Begin()
// more stuff
tx2.Commit()
tx1.Commit()

This API would allow for the txn object (or maybe the driver) to track any names it may have generated for the savepoints and the appropriate rollback logic.

Thoughts?

[kardianos]

@amoghe Did you see the proposed CL? https://golang.org/cl/30165

[amoghe]

@kardianos I'm sorry I completely missed that. Thank you for bringing it to my attention.

After reading the CL, it seems like the API presented by the database/sql pkg would need me to begin a transaction before I can use savepoints. This means that when the txn is created, a BEGIN is issued (because at that point the driver must begin a transaction on the underlying db), and then subsequently multiple SAVEPOINTS may be issued.
A couple of reservations about this...

• the user knows that he/she is only interested in using SAVEPOINTS, so why is he/she asked to begin a transaction? (this is what the original proposal tried to avoid by leaving the implementation to the driver, and only leaving the user to have to deal with a single concept - "transactions").
• I'm not familiar with many databases, but I can imagine there might exist one that suggests that transactions and savepoints be used in a mutually exclusive manner? (This would make the driver implementors life pretty difficult, I think)

Thoughts?

[amoghe]

PS - I'm leaving these comments here because they're about the general approach to extending API offered by database/sql, and not the specific implementation proposed in the CL.

If you think that the discussion should be continued on the CL, LMK and I'll repost them there.

[kardianos]

@amoghe What databases and database protocols are you familiar with? All the systems I'm aware of require a savepoint after a tran has started. With exception of sqlite, but that's sqlite.

[amoghe]

I'm only familiar with SQLite3 and MySQL. The difference in behaviors of just these two databases suggests (to me) that there might exist other behaviors that don't follow the transaction-followed-by-savepoint pattern.

[kardianos]

@amoghe I'm aware that sqlite is slightly different and there will be a work around. Every other rdmbs I'm aware of has savepoint rollback to after tran begin.

[wkhere]

2cents,

I just found this discussion when googling for any references on nested transactions in Go.

I believe a valid and obvious case for nested transactions is testing. When you test the code which already uses transactions (because operations being tested need transaction semantics), you want to wrap such code in a bigger transaction which will rollback after running test(s). You also want the whole machinery to be transparent - ie. to not introduce any additional concepts like savepoints just because for some cases you want your transactions to be part of a bigger one.

I was using this approach over years in testing with Postgres (8.x, 9.x) and application-level drivers (SQLAlchemy, recently Ecto, IIRC it was also possible with ActiveRecord as long ago as in its 2.x times). Looks like the effort of implementing nested transactions with savepoints was done in these drivers.

So the approach @amoghe proposes is used "in the wild" and I believe there's a value in it.

[kardianos]

@herenowcoder I've never used or heard of your use case. That certainly doesn't mean it is invalid; it sounds interesting.

Every time I've looked into this I've found that databases don't support nested transactions, they support savepoints. MySql and Postgresql only support savepoints. MS SQL Server kinda supports nested transactions, but not really (the outer transaction rules the inner one, the inner one doesn't even count), but again it does fully support save points.

My takeaway is that savepoints are what databases are doing, not nested transactions.

• Question 1: Does your understanding match with the above?
• Question 2: Are you saying regardless of the above, you would still want to emulate nested transactions?
• Question 3: From my point of view, SQLAlchemy, Ecto, and ActiveRecord are more sql frameworks, similar to what other go packages provide on top of database/sql. Is this also your understanding?

[wkhere]

@kardianos

• A1: dunno about MySql and MS SQL, Postgres is supporting savepoints. That's what I said, impl. of nested transactions with savepoints is done in the drivers (with a clarification in A3 below)
• A2: Yes, I want to emulate nested transactions, because they are composable, or a better word: embeddable. They enable modularity of your code. if you have only savepoints, you need to know up front that your transaction wlll be inside another transaction. The thing is - you don't know it up front. If you have nested transactions API you just code transactions. One can surround another or not.
  This is useful regardless of whether RDMBS implementor decides of support nested transactions directly or not.
• A3: Yes I did kind of shortcut - SQLAlchemy, Ecto And ActiveRecord are more less ORM-like libraries over low-level drivers, namely Psycopg, Postgrex, and whatever Ruby folks are using for Postgres. As the consumer of these APIs I haven't checked where exactly nested transactions are emulated - in the lib or the driver. I would bet it varies.

I also believe pointing to a distinction between sql driver and sql "framework" (a word lacking a proper definition btw) is not so much applicable here. In Golang ecosystem ORMs don't seem so popular and there must be a reason for this. I already have cases where I would want nested transaction without ORM "framework" thing.

[bradfitz]

I think we're going to kick this until Go 1.9. I'd rather not rush this in.

There are tons of database changes in Go 1.8 to keep at least some people happy.

[rubensayshi]

I think it would be really nice if https://github.com/golang/go/issues/14468 was done before this, this would allow downstream code to take a Queryable (or DBI or w/e name you'd give it) and not have to care if it's a DB, Tx or NestedTx when running queries.

[kardianos]

@rubensayshi I understand this sentiment. For simple queries you are 100% correct, that would be nice.

Here is what's currently stopping me from making a Queryable interface that can perform nested transactions: SQL doesn't work that way.

Let's assume a Queryable interface that allowed nested transactions in the API. Your function takes a func(ctx context.Context, q Queryable) error right? All good. Now let's take a look at some SQL text we might want to run in this queryable.

select * from Account;
begin tran;
insert into Account(ID)
select 42;
commit tran;

This is a perfectly good SQL text if you aren't already in a transaction. But if you ARE in a transaction, it will blow up. However, if you make it a policy to never start transactions within SQL text, this would work just fine.

...

In my own code bases, I do have a Queryable interface, but I use it only for executing queries in a single location with some extra handling around it. When I define SQL text, I have the type information that shows me if I'm in a Tx or not.

All that being said, I'm open to be persuaded otherwise. What I think I would need is the correct wording for the documentation and general advice when and when not to use these functions (nested tran, queryable).

[timbunce]

It seems to me that this 3 year old proposal has stumbled because it tried to cover both savepoints and nested transactions. (And nested transactions vs emulated nested transactions, and something like a Queryable interface that would work for DB, Tx, or Conn.)

In my experience, savepoints are valuable when truly needed but are relatively rarely needed. On the other hand, simple _emulated_ nested transactions can be _very_ useful for the reasons @wkhere pointed out.

Generally transactions should be defined at the outermost level of the code. If that code then gets used by some other code, such as when testing or by some another package, then that new outermost code should be able to control the transactions on that database connection.

func original_func() {
    tx, err := db.Begin()
    ....
    tx.Commit()  // tx.Commit (and tx.Rollback) do nothing when called via other_func()
}

func other_func() {
    tx, err := db.Begin()
    original_func()
    tx.Rollback()
}

That's very simple to understand and requires no external API changes.

Good support for savepoints, on the other hand, is tricky and deserves a separate Issue so as not to hinder discussion of support for simple _emulated_ nested transactions here. (Or perhaps close this and create two separate issues?)

p.s. I'm very new to Go so please forgive me if I'm misunderstanding something here, and for not trying to develop a patch myself.

[kardianos]

@timbunce Savepoint/Rollback support could be easily added; the reason the linked CL for savepoints was abandoned was that I was still unsure if we should support anything, or if we should support something different.

I haven't pushed a specific proposal yet because if people just want savepoints, they can run tx.ExecContext(ctx, "savepoint dancing_gophers;"). It would also be fairly easy to write a wrapper to create a POC for nested Tx using simple exec statements. So there isn't anything intrinsically blocking creating this. Right now I've been focusing on blocking issues; things that require API changes / additions to enable functionality.

Anything that can be proved out in a third party repo first, should be at this point. I have some experiments living at: https://github.com/golang-sql/sqlexp . If I have time I'll create a wrapper obj that implements the nested Tx.

[timbunce]

If I have time I'll create a wrapper obj that implements the nested Tx.

That would be wonderful. Thanks @kardianos!

[kardianos]

First attempt at https://godoc.org/github.com/golang-sql/sqlexp/nest . Feedback?

[amoghe]

@kardianos At first blush I'm quite excited by this. Thanks for taking the effort of whipping this up. Exposing Begin on an existing txn is what was originally discussed in 2015, however back then I noted that (repasting here to save folks the effort of scrolling back):

The immediate drawback of this approach is that ... existing DB drivers will immediately stop compiling since their implementations of transaction object will no longer satisfy the driver.Tx interface.

While I'm all for this approach (having used it successfully as described by @wkhere in ruby/rails) I'm not sure how we would avoid existing driver pkgs from breaking without introducing a new EnhancedTx or TxPlus or fixing as many drivers as we can before/after the introduction of such a change. Thoughts?

[kardianos]

Optional opt in interfaces. Lots sql/driver.

On Wed, May 17, 2017, 18:15 Akshay Moghe [e-mail] wrote:

@kardianos https://github.com/kardianos At first blush I'm quite
excited by this. Thanks for taking the effort of whipping this up. Exposing
Begin on an existing txn is what was originally discussed in 2015,
however back then I noted that (repasting here to save folks the effort of
scrolling back):

The immediate drawback of this approach is that ... existing DB drivers
will immediately stop compiling since their implementations of transaction
object will no longer satisfy the driver.Tx interface.

While I'm all for this approach (having used it successfully as described
by @wkhere https://github.com/wkhere in ruby/rails) I'm not sure how we
would avoid existing driver pkgs from breaking without introducing a new
EnhancedTx or TxPlus or fixing as many drivers as we can before/after the
introduction of such a change. Thoughts?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/7898#issuecomment-302273409, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAuFsUfZMFnWI3KSvuS8tCC_a6Zs9HqDks5r65uagaJpZM4FWWOn
.

[wkhere]

@kardianos in short - looks cool!

[JimNasbyGE]

Something to consider for savepoints; some databases (Oracle and Postgres at least) support RELEASE SAVEPOINT. Making use of that can become beneficial as you get several levels of nesting. If explicit support for savepoints is added, RELEASE should be included. It would be nice if @kardianos's nest package used them as well.

[robcapo]

I agree with @timbunce that in general, the savepoint API is not usually needed. I think the most common usecase is to allow composable functions to execute more than one SQL statement and guarantee that they finish with all-or-none. I think emulated nested transactions can solve this problem. Here's a really quickly thrown together example, which uses savepoints for its implementation:

type NestableTx struct {
    sql.Tx

    savePoint int
    next *NestableTx
    resolved bool
}

func (tx *NestableTx) Begin() (*NestableTx, error) {
    tx.next = &NestableTx{
        Tx:        tx.Tx,
        savePoint: tx.savePoint + 1,
    }

    _, err := tx.Exec("BEGIN SAVEPOINT SP" + strconv.Itoa(tx.next.savePoint))

    if err != nil {
        return nil, err
    }

    return tx.next, nil
}

func (tx *NestableTx) Rollback() error {
    tx.resolved = true

    if tx.savePoint > 0 {
        _, err := tx.Exec("ROLLBACK TO SAVEPOINT SP" + strconv.Itoa(tx.savePoint))
        return err
    }

    return tx.Tx.Rollback()
}

func (tx *NestableTx) Commit() error {
    if tx.next != nil && !tx.next.resolved {
        err := tx.next.Commit()
        if err != nil {
            return err
        }
    }

    tx.resolved = true

    if tx.savePoint > 0 {
        _, err := tx.Exec("RELEASE SAVEPOINT SP" + strconv.Itoa(tx.savePoint))

        return err
    }

    return tx.Tx.Commit()
}

I'm sure I'm missing some edge cases here, but my point is that a relatively simple implementation that supports emulated nested transactions can provide large value. Until Go has native support, I will likely just add an implementation like the above to my projects as needed.

[dhui]

Here's my stab at the problem: github.com/dhui/satomic

satomic is based on sqlexp and implements "nested" transactions, using savepoints for "nesting".
It's designed for ease of use and will automatically rollback to the previous savepoint/transaction on errors/panics!
The test coverage is pretty good and most popular RDBMSs (postgres, mysql, mssql, and sqlite) are supported.

Keep in mind that the API isn't stable yet, so the interfaces may change.
Any feedback is most welcome!

[jonbodner]

Is there still interest in adding this to Go 1.14 or 1.15? I've just written up a quick library for internal usage at my company, but it would be nicer to have nested transactions as part of the standard library.

I am happy to write up a simple spec and provide an implementation. I think it involves adding one new method to sql.Tx (Begin), updating the Commit and Rollback methods on sql.Tx to handle nested cases, and adding an interface to the driver package.

The rest of the work is really up to the db driver providers. I can see if I can release my internal implementation using savepoints; it's not a lot of code.

[wkhere]

@sammy007 have you digested the case described in this thread, with testing the code which uses transactions?

[kardianos]

@jonbodner This proposal has been accepted, and I would welcome an implementation, esp if the code doesn't complicate the existing implementation. This would be API support for nested transactions; the benefit of being able to create a consistent Queryer interface over both DB and Tx is too large to implement the more "raw" savepoint interface.

I've thought about the benefit/cost working through various line of business applications. I agree, it would be good to have this API.

[amoghe]

@kardianos - is the accepted proposal the same one in https://godoc.org/github.com/golang-sql/sqlexp/nest ?

I'm a little confused, I can't tell where along the way the conversation put forth a proposal and got accepted by the go maintainers? Did it happen on golang-nuts?

[jonbodner]

@kardianos I can put in some work on this. Should I work from tip in the database/sql package?

[kardianos]

@jonbodner Yes, you will want to work from tip.

I'm a little worried about complicating the Tx struct and embedded stmt handling too much. Also running a transaction on the non-active Tx should return an error.

[jonbodner]

@kardianos I have a very simple first pass. Maybe this is too simple?

```
Index: src/database/sql/sql.go

<+>UTF-8

--- src/database/sql/sql.go (revision 07957b794c7b99034f41976bbab560ff4615bbc4)
+++ src/database/sql/sql.go (date 1580359014508)
@@ -28,6 +28,8 @@
"sync"
"sync/atomic"
"time"
+

• "math/rand"
  )

var (
@@ -1974,6 +1976,10 @@
// any held driverConn back to the pool.
releaseConn func(error)

• // savePointNames are the names assigned to the savepoints used for nested
• // transactions. It is set to nil when there are no nested transactions.
• savePointNames []string
  +
  // done transitions from 0 to 1 exactly once, on Commit
  // or Rollback. once done, all operations fail with
  // ErrTxDone.
  @@ -1994,6 +2000,25 @@
  ctx context.Context
  }

+func makeName() string {

• out := make([]byte,10)
• for i := range out {
• out[i] = 65 + byte(rand.Intn(26))
• }
• return string(out)
  +}
  +
  +// Begin starts a nested transaction using a savepoint with a random name.
  +func (tx Tx) Begin() (Tx, error) {
• savePointName := makeName()
• _, err := tx.ExecContext(tx.ctx, "SAVEPOINT "+ savePointName)
• if err != nil {
• return tx, err
• }
• tx.savePointNames = append(tx.savePointNames, savePointName)
• return tx, nil
  +}
  +
  // awaitDone blocks until the context in Tx is canceled and rolls back
  // the transaction if it's not already done.
  func (tx *Tx) awaitDone() {
  @@ -2074,8 +2099,15 @@
  }
  }

-// Commit commits the transaction.
+// Commit commits the transaction. If there is a nested transaction active, the
+// nested transaction savepoint is released.
func (tx *Tx) Commit() error {

• if len(tx.savePointNames) != 0 {
• savePointName := tx.savePointNames[len(tx.savePointNames)-1]
• tx.savePointNames = tx.savePointNames[:len(tx.savePointNames)-1]
• _, err := tx.ExecContext(tx.ctx, "RELEASE SAVEPOINT " + savePointName)
• return err
• }
  // Check context first to avoid transaction leak.
  // If put it behind tx.done CompareAndSwap statement, we can't ensure
  // the consistency between tx.done and the real COMMIT operation.
  @@ -2121,8 +2153,15 @@
  return err
  }

-// Rollback aborts the transaction.
+// Rollback aborts the transaction. If there is a nested transaction active, the
+// nested transaction savepoint is rolled back.
func (tx *Tx) Rollback() error {

• if len(tx.savePointNames) != 0 {
• savePointName := tx.savePointNames[len(tx.savePointNames)-1]
• tx.savePointNames = tx.savePointNames[:len(tx.savePointNames)-1]
• _, err := tx.ExecContext(tx.ctx, "ROLLBACK TO " + savePointName)
• return err
• }
  return tx.rollback(false)
  }
  ```

[jonbodner]

If there's a better forum for discussing this, please let me know.

[kardianos]

Reviewing here initially is fine. You can also submit the change for review: https://golang.org/doc/contribute.html#sending_a_change_gerrit

1. I would avoid random, and just name a sequential name like from the save point name index.
2. I would replace the ExecContext calls with a new interface defined on the tx.txi.
3. It would be good to use BeginTx too, but perhaps (for now, require TxOptions to be nil), but the first pass could just stick with Begin as that would be simpler.
4. I think it would be important for parent Tx to not be able to run a query (except rollback and commit) unless the child Tx has finished with rollback or commit.

Possible interface defined in database/sql/driver/driver.go:

type ChildTx interface {
    Savepoint(ctx context.Context, name string) error
    CommitTo(ctx context.Context, name string) error
    RollbackTo(ctx context.Context, name string) error
}

Different DBMSs use different syntax (MS SQL Server and Oracle use different syntax then MySQL.

In order to accomplish (4), many of the fields in *Tx would need to be defined on an internal new *txState struct that is shared between all related *Tx. The *Tx would then hold the current savepoint name (if any), possibly a ctx and release function (unsure), and a *txState that holds the stmts and txi.

[jonbodner]

On points 3 and 4, I think we can simplify greatly if we don't have a Tx.BeginTx that takes in a context:

• While you can spawn multiple parallel transactions from a single sql.DB, I don't think it makes sense to say that you can create multiple parallel savepoints within a transaction. Is there a database where this isn't the case? Autonomous transactions in Oracle can only be declared in specific situations that I think are out of scope for this issue (https://oracle-base.com/articles/misc/autonomous-transactions). Since savepoints aren't independent, does it make sense for them to potentially have separate contexts?
• The intent was to re-use the existing Tx instance (Tx.Begin returns back the instance), so there's no way for a parent Tx to run a query outside of the nested transaction. If there's a possibility of a separate context for a nested transaction, it isn't possible to return back the same instance, which means we need to put flags into parent transactions to return an error if the user attempts to run a query while there's a live child context.

I have another implementation that I can submit for review via gerrit, but I wanted to get your feedback first.

[kardianos]

Sounds good. I'm fine leaving BeginTx out, at least of this initial implementation.
I agree that you don't want "allow" parallel transactions (most if not all that would be impossible).

If this is implemented, I'm convinced we will need to turn *Tx into some type of wrapper for an inner *txState. Without this, we will be unable to prevent parent queries to be run on "child" transactions.

If this sounds good, we can move over further review to gerrit. But design overview and questions are great to have here first.