issues with sqlx.TransactContext

[Minnowo]

Thank you for continuing to maintain this project!

I recently was doing some work and decided to make use of:

func TransactContext(ctx context.Context, db Queryable, txFunc func(context.Context, Queryable) error) error

But ended up confused as to why I kept getting a nil pointer exception when calling this.
Eventually I figured out that it's due to the internal cast of the sqlx.Queryable parameter:

func TransactContext(ctx context.Context, db Queryable, txFunc func(context.Context, Queryable) error) error {
    depth := getTxDepth(ctx)
    tx := getTransactionDB(ctx)
    var err error
    if tx != nil {
    	// already in a transaction, push down the stack
    	ctx = context.WithValue(ctx, dbKeyDepth, depth+1)
    } else if base, ok := db.(*DB); ok { /* <--- if this is false, we get a nil pointer trying to use tx later */
    	tx, err = base.Beginx()
    	if err != nil {
    		return err
    	}
    	ctx = context.WithValue(ctx, txKey, tx)
    	ctx = context.WithValue(ctx, dbKeyDepth, depth+1)
    }
    ...

Here is the minimal code to reproduce this:

ctx := context.Background()

// some type that embeds the db
var dbWrapper struct {
	*DB
}
dbWrapper.DB = db // where db is an instance of *sqlx.DB

txFunc := func(ctx context.Context, db Queryable) error {
	return nil
}

TransactContext(ctx, dbWrapper, txFunc)

I was going to open a PR for this changing the db parameter to something like:

type Transactable interface {
	Beginx() (*Tx, error) 
}

But the sqlx.Queryable doesn't have the Beginx function on it which makes the signature of this function confusing, since it accepts Tx even though that would cause the same nil pointer here if the Context doesn't return an existing transaction.

I can't think of anyway to improve this API so that the caller would know this though. Which is why I'm opening an issue instead.

My current proposed solution is that we just do something like this:

type Transactable interface {
	Beginx() (*Tx, error) 
}

func TransactContext(ctx context.Context, db Queryable, txFunc func(context.Context, Queryable) error) error {
      depth := getTxDepth(ctx)
      tx := getTransactionDB(ctx)
      var err error
      if tx != nil {
      	// already in a transaction, push down the stack
      	ctx = context.WithValue(ctx, dbKeyDepth, depth+1)
      } else if base, ok := db.(Transactable); ok { /* this should allow more choice */
      	tx, err = base.Beginx()
      	if err != nil {
      		return err
      	}
      	ctx = context.WithValue(ctx, txKey, tx)
      	ctx = context.WithValue(ctx, dbKeyDepth, depth+1)
      } else {
        // improve the error message so the caller can fix their code
      	panic("the given sqlx.Queryable could not be cast into sqlx.Transactable, please fix your code")
      }
      ...

What do you think about this? I think this would prevent the nil pointer issue for structs, while making it clear what goes wrong if no transaction can be started.

Adding Beginx to the transaction struct where it just panics or return itself might be another option. This would make db.Queryable not need any type cast. But this might introduce more confusion in some cases.