Describe the feature

Commit and rollback should return an error and not the DB instance.

Motivation

In Go, it's common to use interfaces to define behavior for many reasons. One common reason is the fact that organizations often change providers for things such as the cloud, databases, etc. In our use case, we want to abstract Gorm into an interface of actions for future flexibility that usually only returns an error because the DB transactions are self-contained and self-dependent.

In the use-case for updating the database and publishing the updates to a queue with the abstraction over gorm, we'd like to rollback the transaction if the push to SQS fails. Unfortunately, we're unable to do this because gorm Commit / Rollback returns *gorm.DB instead of an error like the standard library's tx.Commit() and tx.Rollback(). This breaks the point behind creating an interface to be vendor-agnostic. If error is returned instead, it allows us to continue our abstraction and create a committable interface for returning back the commit and rollback functions from a struct to give control to the abstraction caller over commit and rollback. Returning gorm.DB instead of an error is fairly useless because the transaction is over at either commit or rollback and there's nothing more to do. Returning an error in both Commit and Rollback is compliant with the standard library's Commit and Rollback interface in the SQL package. Therefore I think it would be beneficial for the community at large to introduce this breaking change into the API.

Related Issues

Example

type Commitable interface {
  Commit() error
  Rollback() error
}

type Database interface { 
 CreateUser(ctx context.Context, user User)  (*Commitable, error)
}

type Commitable struct {
    Commit func() error
    Begin func() error
}

func (db DB) CreateUser(ctx context.Context, user database.User) (*database.Comittable, error) {
    session := db.gorm.Session(gorm.SessionOpts{Context: ctx})

    db = db.Begin()

    if err := db.gorm.Create(&user).Error; err != nil {
       return nil, error
    } 

    return &Comittable{
       Commit: db.Commit
       Rollback: db.Rollback
    } , nil 
} 

  ... business logic ... 

  comittable, err := postgres.CreateUser(ctx, User)
  if err != nil {
    return nil, err
  } 

  err = sqs.Queue(ctx, database.QueueMSG{UserId: User.ID, Action: database.CREATE})
  if err != nil {
   comittable.Rollback(); 
  } 

  comittable.Commit();

  return

I know the code isn't 100% to the API but you get the gist. Apologies in advance if I missed something and this is a moot point.

Comment From: quesurifn

@jinzhu I'd appreciate some insight from you.

Comment From: a631807682

you can wrap it youself.

func wrap(tx *gorm.DB) error {
    return tx.Error
}

...
return &Comittable{
   Commit: wrap(db.Commit)
   Rollback: wrap(db.Rollback)
} , nil 

Comment From: jinzhu

Actually, I would prefer to use the Transaction method instead of Begin, Commit...

In you sample code, you have to check the error of db = db.Begin(), or it might panic if you failed to create a transaction when your db in high load.

Comment From: quesurifn

@a631807682 Thanks for pointing out the obvious and thanks @jinzhu for responding. I really appreciate it. Transaction is what I'll do.