support READ-ONLY transactions #618
Conversation
|
We need to discuss about error handling. Which implementation is best? Option 1: Do not check errors in the driver (now implementation). |
|
Option 2: Check errors in the driver (implementation: shogo82148@371a560) |
|
Option 3: Check more carefully (implementation: shogo82148@b437c9d) |
|
TestContextCancelStmtExec fails https://travis-ci.org/go-sql-driver/mysql/jobs/241696327#L237. |
|
This PR needs an update since #619 was merged |
|
I updated, and all tests passing! |
|
Do we want to merge this? |
connection_go18.go
Outdated
| return mc.Begin() | ||
| } | ||
|
|
||
| func (mc *mysqlConn) beginReadOnly() (driver.Tx, error) { |
There was a problem hiding this comment.
just add a parameter to mc.Begin and reuse it.
The implementation is already inconsistent between the two functions.
|
@julienschmidt Thank you for your review. I fixed it. please review it again. The tests fail, while I think that its cause is not my pull request. |
|
It seems like there was a change to the MySQL server on Travis causing an old test to fail. Unrelated to this PR, but we marked Travis as a required check on Github, so we can't merge before it is fixed. See #660 |
There was a problem hiding this comment.
Please also rebase this PR onto the current master branch when #661 is merged.
connection.go
Outdated
| err := mc.exec("START TRANSACTION") | ||
| var err error | ||
| if readOnly { | ||
| err = mc.exec("START TRANSACTION READ ONLY") |
There was a problem hiding this comment.
It's likely better if the if-else just changes the query string, e.g. it should result in a bit less generated code.
|
#661 was merged. If you rebase it on current master, then the tests should pass again |
shogo82148
force-pushed
the
support-read-only-transaction
branch
from
3dc9673 to
124ead1
Compare
|
I rebased and squashed it. All tests are passed, thanks :) |
|
thanks for your patience! |
Description
support READ-ONLY transactions.
separated from #608.
Checklist