Skip to content
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

database/sql: common interface for query functions in sql.DB and sql.Tx #14468

Open
KilledKenny opened this issue Feb 22, 2016 · 22 comments
Open
Assignees
Milestone

Comments

@KilledKenny
Copy link
Contributor

Hi,

I'm proposing a interface that implements all functions in sql.DB and sql.Tx that takes a query string as one of its arguments. This could be useful when implementing "dumb" functions that retrieves information but in itself dose not need to be prepared however might be used to verify whether to commit or rollback.

The interface I'm proposing is: (The name can of course be changed)

type QueryAble interface {
    Exec(query string, args ...interface{}) (sql.Result, error)
    Prepare(query string) (*sql.Stmt, error)
    Query(query string, args ...interface{}) (*sql.Rows, error)
    QueryRow(query string, args ...interface{}) *sql.Row
}

This is an example function Get that can be used with or without transactions.

func Get (q Queryable)(int){
    var test int
    res , _ := q.Query("SELECT COUNT(*) FROM table;")
    res.Next()
    res.Scan(&test)
    res.Close()
    return test
}

func GetExample(db *sql.DB){

    //When you just want to retrieve the information, no need for a transaction
    fmt.Printf("Current len of Table %d\n", Get(db))

    //Make a change
    tx, _ :=db.Begin()
    // Add data to table
    if  Get(tx) > 2 {
        fmt.Printf("Table to large: %d robacking to: %d \n", Get(tx) , Get(db))
        tx.Rollback()
    } else {
        fmt.Printf("Table update new len %d\n", Get(tx))
        tx.Commit()
    }
}
@ianlancetaylor ianlancetaylor changed the title database/sql Common interface for query functions in sql.DB and sql.Tx database/sql: Common interface for query functions in sql.DB and sql.Tx Feb 22, 2016
@ianlancetaylor ianlancetaylor changed the title database/sql: Common interface for query functions in sql.DB and sql.Tx database/sql: common interface for query functions in sql.DB and sql.Tx Feb 22, 2016
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Feb 22, 2016
@ianlancetaylor
Copy link
Contributor

I'm sorry, I don't entirely understand this proposal. Are you suggesting that we should add the type QueryAble to database/sql? How would that make the package easier to use than it is today?

@KilledKenny
Copy link
Contributor Author

Yes, I want to add QueryAble to database/sql

The reason is that: I want to be able to create functions that execute SQL query's and retrieve data regardless of if its inside a prepared statement (and preferably only use prepared statements when i really need to). This is not possible without a interface.

To provide a transaction and non transaction solution today you have to do something like this:

func SqlMathWrapper(db *sql.DB , a, b int)(int, error){
    tx, _ := db.Begin()
    defer tx.Commit()
    return SqlMath(tx, a,b)
}

func SqlMath(tx *sql.Tx, a, b int)(num int, err error){
    err = tx.QueryRow("SELECT ? + ?", a, b).Scan(&num)
    return
}

(This dose always use transaction)

Using the interface the code would look like this

func SqlMath(qa *sql.QueryAble, a, b int)(num int, err error){
    err = qa.QueryRow("SELECT ? + ?", a, b).Scan(&num)
    return
}

(This would not use prepared statements if called with a sql.DB object)

@ianlancetaylor
Copy link
Contributor

Just to be clear, you could define the interface yourself, right?

@KilledKenny
Copy link
Contributor Author

@ianlancetaylor Yes, and that's what I will do until its implemented.

@kardianos
Copy link
Contributor

Another discission at #14468 .

@kardianos
Copy link
Contributor

The API for rollback / savepoint may also factor into this conversation: #7898.

@vwochnik
Copy link

vwochnik commented May 9, 2017

I am strongly in favor of the Queryable interface as it's proposed here. At the company I am working for I am running into the same use case where I have a few CRUD functions which don't need to care if it's a sql.DB or sql.Tx. In fact, it's counter intuitive. I want to use these functions both with and without transactions. The responsibility of handling the transactions is outside of the scope of the functions.

@kardianos
Copy link
Contributor

@vwochnik I'm not convinced such an interface needs to live in the std lib.
Here is one implementation: https://godoc.org/github.com/golang-sql/sqlexp#Querier
You could also define your own.

@kPshi
Copy link

kPshi commented May 10, 2017

Absolutely common issue it seems and I have the same case here. Of course we can all write an interface definition on our own but that would be done again and again causing much more work in the end (when summing up each individual time spent or working around that).
Libraries are used to ease up things and to not make everyone repeat the very same work. So the question in my opinion is not whether one is able to write that but the question is if that would make things easier and for how many people.

@kardianos
Copy link
Contributor

Great @kPshi . Start using the interface defined in sqlexp and spread the word. Then when the tree opens in 3 months we can see how widely it is used.

@posener
Copy link

posener commented Feb 24, 2018

How can one use an http.RoundTripper-like interface for SQL queries? The http library has really nice behaviors/abstractions/profiling hooks around the http.Client, which we miss in the sql.DB client.

@kardianos
Copy link
Contributor

kardianos commented Feb 24, 2018 via email

@posener
Copy link

posener commented Feb 24, 2018

Not at this level. Some drivers a not network based, others are cgo based.
Ask the driver for a connector with that capacity.

Why? They all implement the ExecContext, QueryContext, and so... function for example, wouldn't it be nice to have an option to "wrap" calls for those function, and add instrumentation, such as logging, timing, and so?
Why should it be a driver-specific functionality?

Some use cases I thought of:

  • If I'm using an ORM, like gorm, I am "stuck" with the logging gorm had defined.
  • If I want to use open tracing, or server timing, I have to run function before and after each call for a database exec/query and can't wrap in a general way all the calls to the database.

I could have a struct implementing the non-existent interface, that wrap a given database connection:

type SQLLogger struct {
    *sql.DB
}

func (s *SQLLogger) ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) {
    log.Printf("whatever")
    defer log.Printf("finished!")
    return s.DB.ExecContext(ctx, query, args...)
}

The problem is that since this interface does not exists in the standard library, most libraries expect an *sql.DB and can not accept the new instrument struct that implements the query/exec function.

@romeovs
Copy link

romeovs commented Aug 23, 2018

I agree.

Libraries are used to ease up things and to not make everyone repeat the very same work. So the question in my opinion is not whether one is able to write that but the question is if that would make things easier and for how many people.

This is what makes Go so powerful IMHO: a well-defined set of shared libraries that can work together seamlessly, because most high-level concepts are expertly expressed in the STL.

@vituchon
Copy link

vituchon commented Dec 30, 2019

I have the same issue of the author, so 👍

Why don't you define a new interface like DbTx honoring tha pattern invented for the interface like
TB. Leaving aside functional differences the "scenario" is quite the similar.

@arianitu
Copy link

I'm a 👍 on this one, it would be nice to be able to have a shared interface that you can pass a Txt or a normal db.

We ended up creating our own interface which isn't too bad, it just may spook some people coming to the codebase that see db.Querier instead of something like sql.Querier

I think Querier and Queryable are actually tricky to type so we just went with db.Conn

@irl-segfault
Copy link

irl-segfault commented Jul 30, 2021

The problem is that since this interface does not exists in the standard library, most libraries expect an *sql.DB and can not accept the new instrument struct that implements the query/exec function.

This is the root of the problem. Makes it impossible to create wrapper DBs for use in shared libraries, for example gorm. One use case is adding instrumentation -- ideally this is done by wrapping or embedding the *sql.DB object and doing things before and after the calls to the underlying *sql.DB. This simple idea breaks down due to the set of objects (sql.DB, sql.Stmt, sql.Tx) that are used together. For example, the gorm library requires a *sql.DB (this is not technically true as it requires an interface type with methods that the *sql.DB provides, but the net effect is the same because you will not be able to satisfy that interface with new wrapper types, since the concrete return types on the interface methods are sql package structs not interfaces).

The http std lib makes greate use of interfaces so we can provide our own pieces of the puzzle like RoundTrippers where we can add instrumentation. I don't see a good reason why the sql package is not designed in the same way.

@irl-segfault
Copy link

^ It turns out that you can easily accomplish this kind of instrumentation by implementing your own driver (or wrapping an existing driver). The issue with this is that telemetry instrumentation isn't necessarily specific to any given implementation of a DB, so the driver isn't the best place to do this. However, it seems to be our only option currently.

@sfllaw
Copy link
Contributor

sfllaw commented Oct 16, 2021

It occurs to me that a sql.DBTx is most valuable for third-party library code which requires a *sql.DB or *sql.Tx as a database parameter, when they don’t actually care about whether there is a transaction or not.

There is precedent in testing.TB for an intersection interface. This kind of interface seems to be discovered again and again by library consumers, who have to petition their authors to change their APIs.

I don’t see it mentioned here, but in #14674, it was pointed out that we probably want to implement (*DB).Stmt so that DBTx.Stmt can exist. This can basically be a no-op.

@rhcarvalho
Copy link
Contributor

In reading the previous comments I noticed mentions to the testing.TB interface. I would like to note that testing.TB is an internal implementation detail and has a private() unexported method so that it cannot the implemented by other types without embedding.

I realized that if such an intersection interface existed in database/sql, we would also want it to be "private" as to not limit the future addition of new methods, however unlikely that might be.

@elgs

This comment was marked as off-topic.

@mfridman
Copy link

mfridman commented Apr 4, 2023

+1 A common interface in the standard library would be very useful. For example, in pressly/goose (a migration tool) users are able to build their own goose binaries and "register" Go-based migrations alongside their SQL migrations.

The goose package exposes the following types and registration functions:

type GoMigration func(tx *sql.Tx) error

func AddMigration(up, down GoMigration) {
  ...
}

type GoMigrationNoTx func(db *sql.DB) error

func AddMigrationNoTx(up, down GoMigrationNoTx) {
  ...
}

With these primitives, users can write Go migration files (e.g., 00002_rename_root.go) that get compiled as part of the executable. E.g.,

func init() {
	goose.AddMigration(Up00002, nil)
}

func Up00002(tx *sql.Tx) error {
	_, err := tx.Exec("UPDATE users SET username='admin' WHERE username='root';")
	return err
}

We could define a common interface in goose, but then the caller would be stuck with a bespoke interface that doesn't interoperate with the rest of the ecosystem. In other words, popular frameworks would still accept *sql.DB, *sql.Tx or *sql.Conn .. or even worse their own bespoke interface. The burden then falls on the caller to do assertion acrobatics to glue the various tools.

Much like the lack of a Logger interface in the standard library (although there is some hope with slog), the lack of a common SQL interface makes various tools throughout the ecosystem less interoperable and more fragmented.

It doesn't have to be like this, if the standard library defined the contract(s) we could have a much richer ecosystem where the end-users enjoy packages and frameworks that work cohesively.

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

No branches or pull requests