Transaction isolation levels

[zimnx]

Description

Support setting transaction isolation level

Currently supported isolation levels are:

• Serializable
• Repeatable read
• Read committed
• Read uncommited
  Which are all available in InnoDB (https://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-isolation-levels.html)

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

[methane]

You shouldn't use mc.finish() here. It should be paried with mc.watchCancel().
Move whole this if statement after watchCancel(), and remove mc.finish().

[shogo82148]

Move whole this if statement after watchCancel(), and remove mc.finish().

In addition, please don't forget to call mc.finish() before return nil, err.
mc.finish() must called before all return.

[zimnx]

@methane @shogo82148 I've applied your comments. Do you guys think defering mc.finish() after watchCancel() is OK?

[shogo82148]

@zimnx not ok.
because mc.finish() should be called before tx.Rollback().

... but, calling tx.Rollback() may be very rare case.
so, if you delete the following code, and then you can add defer mc.finish()

select {
default:
case <-ctx.Done():
	tx.Rollback()
	return nil, ctx.Err()
}

this code comes from ctxutil.go.

@methane How do you think which is better calling tx.Rollback() in BeginTx, or defering mc.finish()?

[shogo82148]

nice test👍

but, this test can be written more concisely, can't it?
for example

tx1, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
    Isolation: sql.LevelRepeatableRead,
})
tx2, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
    Isolation: sql.LevelReadCommitted,
})

row := tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.

_, err = tx1.ExecContext(ctx, "INSERT INTO test VALUES (1)")

row = tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.

[zimnx]

Done, thanks for advice.

[methane]

Please insert your name to alphabetical order

[methane]

@shogo82148 +1 to just remove it.
The ctx can be cancelled right after the select statement anyway.

[zimnx]

@methane Done. Do you have any more comments? I would like to squash commits.

[methane]

No need to manual squash. We use "Squash and merge".

I'll review whole pull request again in tomorrow. Please wait.

[zimnx]

Sure, thanks!

[methane]

return mc.Begin() is preferred Go style.

[shogo82148]

thanks for your opinion.
LGTM

[bweston92]

Any news on this? Literally just got an error about this not working.