Skip to content

*sql.DB as an interface #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sethgrid opened this issue Jun 1, 2016 · 9 comments
Open

*sql.DB as an interface #227

sethgrid opened this issue Jun 1, 2016 · 9 comments

Comments

@sethgrid
Copy link

sethgrid commented Jun 1, 2016

Some interesting options would open up if your NewDB method took an interface that adheres to sql.DB.

@sintanial
Copy link

it's would be nice, then we can use some wrappers, for example "log sql queries"

@jmoiron
Copy link
Owner

jmoiron commented Jun 10, 2016

This would definitely be a breaking change, because sqlx.DB exposes the *sql.DB that you pass it, since it gets embedded. It's possible to add an extra interface to the sqlx.DB and a separate constructor, something like NewDBInterface that would accept something implementing the DB interface so long as it also had some function that includes the embedded database as well.

Some of the implications of this are really interesting so I'll have to think about whether it's worth a significant restructuring of the code and a slight increase in the API.

@gpopovic
Copy link

+1

@jmoiron
Copy link
Owner

jmoiron commented Feb 28, 2017

sql.DB implements a pretty huge interface. I could see some advantages to being able to do this:

type LoggingDB struct {
    *sql.DB
}

func (l *LoggingDB) Query(q string, args ...interface{}) {
    logf("%v, %+v\n", q, args)
    return l.DB.Query(q, args...)
}

...

But I don't see how this can be accomodated in a way that's worth the complexity, especially now that the DB interface has changed so drastically in the latest release. Instead of doing this in sqlx, I suggest you wrap sqlx.DB in the type of thing you need. Under the hood, sqlx makes use of a large portion of the implicit sql.DB interface, but much client code can get by with only a few changes.

If someone wants to submit a PR that shows how this might look, I'd be willing to entertain it, but it:

  • shouldn't define a very large interface for NewDB
  • shouldn't break compatibility for sqlx.DB.DB being accessible as a valid *sql.DB
  • shouldn't be a huge amount of code

I can't see how to achieve all 3 but it may be possible.

@tj
Copy link

tj commented Jun 12, 2017

I found it a little weird to use sqlx.NamedExec() (just discovered that) in place of the DB/TX satisfying an interface that has NamedExec() and the others. Not hard to add that of course but it's more rewriting than if you could just swap the *sqlx.DB with an interface.

@hsienchiaolee
Copy link

hsienchiaolee commented Jun 19, 2018

I also think it would be nice to wrap *sqlx.DB with an interface as well as *sqlx.Tx. I am pretty new to Go, so perhaps I am missing something here, but what I am trying to do is to mock *sqlx.DB in my unit tests. For example:

type Tx interface {
  NamedExec(query string, arg interface{}) (sql.Result, error)
}

type DB interface {
  Beginx() (Tx, error)
  Close() error
}

type myDB struct {
  db DB
}

func NewDatabase(dsn string) (Database, error) {
  db, _ := sqlx.Connect(dsn)
  return &myDB{db}, nil
}

Then in the tests:

  db := new(FakeDB)
  tx := new(FakeTx)

  db.BeginxReturns(tx, nil)
  // ...do my tests...
  // ...verify tx calls, stub tx calls, etc ...

However, I can't get this to work since it complains about this error:

cannot use db (type *sqlx.DB) as type DB in field value:
	*sqlx.DB does not implement DB (wrong type for Beginx method)
		have Beginx() (*sqlx.Tx, error)
		want Beginx() (Tx, error)

@hsienchiaolee
Copy link

hsienchiaolee commented Jun 22, 2018

Actually, found a way around it:

type DB interface {
  Beginx() (Tx, error)
  Close() error
}

type db struct {
  *sqlx.DB
}

func (db *db) Beginx() (Tx, error) {
  return db.DB.Beginx()
}

func (db *db) Close() error {
  return db.DB.Close()
}

// Has wrapped query functions, etc.
type myDB struct {
  db DB
}

func NewDatabase(dsn string) (Database, error) {
  db, _ := sqlx.Connect(dsn)
  return &myDB{db}, nil
}

@RangelReale
Copy link

I did a quick try of using an interface instead of *sql.DB directly, had to implement a few methods that just forwards to the internal DB ones, but it seems to be fairly easy and backwards-compatible.

Code

@chad-wheeler-sp
Copy link

No need to create an interface for sql.DB that everyone can use. That is the great thing about GO is that you can create your own local interface for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants