database/sql: add Context support

[bradfitz]

This bug is about adding Context support the database/sql package.

Parent bug is #14660

[bradfitz]

@pbnjay wrote on the parent bug:

I'm a bit late to the discussion, but would it make more sense to add a Context to sql.Tx instead of the Query args? Automatic rollback on cancellation would be desirable, no? Alternative, maybe less desirable version would be a method like BeginWithContext(ctx)

"instead of"? Remember you can also query outside of transactions.

[cihangir]

I started implementing this a while back ago with having context as the first parameter to all the api functions. Apart from SetMaxIdleConns and SetMaxOpenConns all functionality implemented. Given the fact that my implementation breaks the whole api, implementation is purely done in a third party manner (without touching to std lib code)

For the ref, you can find the docs here https://godoc.org/github.com/cihangir/ctxdb

I would like to help on the implementation if need.

Automatic rollback on cancellation would be desirable, no?

I tried to achieve this by closing the connection to the database Some of the database vendors handle it properly, but i am not sure if it is a general solution.

[pbnjay]

You can query outside of transactions, but I think combining a user-space context with the database transaction (even for a single-query transaction) is troublesome. If you run an UPDATE but your context times out, user space indicates a cancellation/error, but the database may complete the request successfully. Sure this would be a user error, but ensuring that all Context usage occurs within a transaction allows for consistent results.

Surely I'm missing something?

[extemporalgenome]

I like @pbnjay's idea of having tying contexts into Tx -- even if we have to add a way to "begin" a fake transaction, or create an interface that *sql.Tx will be made to implement, and which is additionally consumed for non-transactional database contexts.

With the "magic first argument" behavior, there are too many open questions. What happens if a user passes in a first argument that implements both driver.Value and context.Context? There is already also the open question about whether it should be the first or last value, and users will have the same question (and make mistakes which may pass silently, depending on how much validation the driver does). The only way to find out if a context with a 5 minute deadline is actually being used for query aborts is to construct a query that takes 5 minutes.

Also, users may infer that multiple contexts may be passed in (whichever is cancelled first), or have subtle bugs that didn't occur before but can occur now, such as sorting the parameter []interface{} for a generated "IN" query, say for debugging clarity, in a wrapper in front of Query/Exec.

I don't think we need a parallel API, but we can shift the proposal to a couple new methods that flow into the existing API (and don't sacrifice type-safety).

[ymmt2005]

FWIW, I have just implemented a context-oriented query canceler for MySQL.

Not only in Go, but MySQL server should also be able to cancel running queries
to avoid wasting CPU and memory resources. In my implementation, I used two
techniques:

1. (for MySQL < 5.7.8) KILL by another connection.
2. (for MySQL >= 5.7.8) SET max_execution_time.

This was possible because I knew my program issues only SELECT statements.

From this experience, I'm not sure whether it is good or not that database drivers
implicitly executes this kind of queries with side effects for native support of contexts.

The code is here for your reference:
https://github.com/cybozu-go/goma/blob/ac585a21752104d5ce69601c310eed753bf8f135/probes/mysql/probe.go#L27

[pbnjay]

This one may need sql/driver support to function, I did some testing here with Postgres and the query keeps running even if you kill the code entirely with ctrl-C :

https://gist.github.com/pbnjay/96f829f8aa77d7ae1d88bca094e5d6ec

Since @ymmt2005 also mentioned mysql-specific commands, it leads me to think it can't be done from the current generic interface.

[j7b]

Context support isn't a good fit for database/sql, there's no portable way to add deadlines or cancellation signals, I'd prefer to see a separate package like database/context that wraps *database.DB methods with checks to see if Done() is closed for a passed context and adds appropriate errors.

[johto]

If you run an UPDATE but your context times out, user space indicates a cancellation/error, but the database may complete the request successfully. Sure this would be a user error, but ensuring that all Context usage occurs within a transaction allows for consistent results.

With explicit transactions you're merely moving this problem from the UPDATE to the COMMIT command, not eliminating it.

[derekperkins]

This one may need sql/driver support to function

Seems very likely.

[kardianos]

@bradfitz

Context is a great fit for sql. Almost all rdbms have a query cancel method and all databases need to ensure their connections are returned to the pool when the request is finished.

One option I could see is in sql.DB add a Ctx(context.Context) *sql.DB method who'd methods use a context under the covers.

In the driver you might need new interfaces that could be tested for, such as

type ConnCtx interface {
        PrepareCtx(ctx context.Context, query string) (sql.Stmt, error)
        CloseCtx(ctx context.Context) error
        BeginCtx(ctx context.Context) (sql.Tx, error)
}
type DriverCtx interface {
        OpenCtx(ctx context.Context, name string) (Conn, error)
}
type ExecerCtx interface {
        ExecCtx(ctx context.Context, query string, args []Value) (Result, error)
}
type QueryerCtx interface {
        QueryCtx(ctx context.Context, query string, args []Value) (Rows, error)
}
type StmtCtx interface {
        CloseCtx(ctx context.Context) error
        ExecCtx(ctx context.Context, args []Value) (Result, error)
        QueryCtx(ctx context.Context, args []Value) (Rows, error)
}
type TxCtx interface {
        CommitCtx(ctx context.Context) error
        RollbackCtx(ctx context.Context) error
}

Does that sound about right?