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

Can't START TRANSACTION ... READ ONLY #817

Closed
taspeotis opened this issue May 11, 2020 · 6 comments
Closed

Can't START TRANSACTION ... READ ONLY #817

taspeotis opened this issue May 11, 2020 · 6 comments
Assignees

Comments

@taspeotis
Copy link

taspeotis commented May 11, 2020

Hi, thanks for the library. I appreciate your effort in giving first-class support to asynchronous operations.

I'd like to START TRANSACTION ... READ ONLY. Unfortunately it seems like nothing in MySqlConnection.BeginDbTransactionAsync will append READ ONLY to the START TRANSACTION command.

var isolationLevelValue = isolationLevel switch
{
	IsolationLevel.ReadUncommitted => "read uncommitted",
	IsolationLevel.ReadCommitted => "read committed",
	IsolationLevel.RepeatableRead => "repeatable read",
	IsolationLevel.Serializable => "serializable",
	IsolationLevel.Snapshot => "repeatable read",

	// "In terms of the SQL:1992 transaction isolation levels, the default InnoDB level is REPEATABLE READ." - http://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-model.html
	IsolationLevel.Unspecified => "repeatable read",

	_ => throw new NotSupportedException("IsolationLevel.{0} is not supported.".FormatInvariant(isolationLevel))
};

using (var cmd = new MySqlCommand($"set session transaction isolation level {isolationLevelValue};", this))
{
	await cmd.ExecuteNonQueryAsync(ioBehavior, cancellationToken).ConfigureAwait(false);

	var consistentSnapshotText = isolationLevel == IsolationLevel.Snapshot ? " with consistent snapshot" : "";
	cmd.CommandText = $"start transaction{consistentSnapshotText};";
	await cmd.ExecuteNonQueryAsync(ioBehavior, cancellationToken).ConfigureAwait(false);
}

There's a corresponding READ WRITE option, too. The relevant MySQL documentation is here.

For now I'm eschewing MySqlTransaction in favour of managing it myself with commands.

@bgrainger
Copy link
Member

bgrainger commented May 11, 2020

Would an additional overload to BeginTransaction(Async) meet your needs?

BeginTransaction(IsolationLevel isolationLevel, bool isReadOnly);
BeginTransactionAsync((IsolationLevel isolationLevel, bool isReadOnly, CancellationToken cancellationToken = default);

@taspeotis
Copy link
Author

taspeotis commented May 12, 2020

I think that would be adequate, thanks. Reading the documentation I would note:

The READ WRITE and READ ONLY modifiers set the transaction access mode. They permit or prohibit changes to tables used in the transaction. The READ ONLY restriction prevents the transaction from modifying or locking both transactional and nontransactional tables that are visible to other transactions; the transaction can still modify or lock temporary tables. These modifiers are available as of MySQL 5.6.5.

...

If no access mode is specified, the default mode applies. Unless the default has been changed, it is read/write.

Given the version limitation and desire to have a default (i.e. omit READ ONLY or READ WRITE) I take it only these overloads of BeginTransaction(Async) would write the modifiers.

@bgrainger bgrainger self-assigned this May 30, 2020
@bgrainger
Copy link
Member

bgrainger commented Jun 6, 2020

Added in 0.68.0.

@taspeotis
Copy link
Author

taspeotis commented Jun 8, 2020

I've tested this method and it doesn't work in all cases.

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'read only' at line 1

The offending code seems to be here. Basically if snapshot isolation is specified then the SQL that is executed is ... with consistent snapshot read only when it should be ... with consistent snapshot, read only.

I can tee up a pull request for it in my spare time, and/or log this as a new issue.

@bgrainger
Copy link
Member

bgrainger commented Jun 8, 2020

I did not read the documentation (or test) thoroughly enough.

@bgrainger bgrainger reopened this Jun 8, 2020
@bgrainger
Copy link
Member

bgrainger commented Jun 8, 2020

Fixed in 0.68.1.

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

No branches or pull requests

2 participants