remove prefixdb and replace with mysql driver params #2871
Conversation
This uses the mysql driver library's capability to use `SET` to set the system variables that prefixdb previously was.
|
Hello! First PR in a while. This is output that came about as a side-effect of go-sql-driver/mysql#635 and also me not having read some docs and needing to do a penance. I could break the mysql driver update into another PR if you'd prefer. |
| // In MariaDB, max_statement_time and long_query_time are both seconds. | ||
| // Note: in MySQL (which we don't use), max_statement_time is millis. | ||
| readTimeout := conf.ReadTimeout.Seconds() | ||
| conf.Params["max_statement_time"] = fmt.Sprintf("%g", readTimeout*0.95) |
There was a problem hiding this comment.
@jsha: random aside, we were prviously using it but do we really want to be using %g here? Documentation for it says %e for large exponents, %f otherwise where %e is scientific notation, e.g. -1.234456e+78. Can MariaDB really understand scientific notation or should we just be using %f consistently?
There was a problem hiding this comment.
MariaDB understands scientific notation, so this can stay.
|
One small nit that may not need to be fixed but generally LGTM. Could you run the tests for the new vendored version of Thanks! |
|
Same as Roland: LGTM, but please document in the PR desc that you ran tests in the vendored code. |
|
Ran it in a checkout of the repo since godeps now doesn't include the test files (which is great!). |
|
The price is paid and penance completed. Thanks! |
This uses the mysql driver library's capability to use
SETto set the systemvariables that prefixdb previously was.
Unfortunately, the library doesn't sort the params when making the string, so we
have to do a little munging to TestNewDbMap.
Ran it in a checkout of the repo since godeps now doesn't include the test files (which is great!).