Implement BEGIN [RW] ISOLATION LEVEL

[apstndb]

This PR implements ISOLATION LEVEL clause for BEGIN and BEGIN RW.

Note: REPEATABLE READ seems not yet to be enabled.

Reference

• feat: specify isolation level per transaction googleapis/java-spanner#3704
  • It is not yet merged so it can be changed. If it is changed, spanner-mycli will do a breaking change. (We don't guarantee stability!)
• Add transaction-level isolation level setting cloudspannerecosystem/spanner-cli#206

[gemini-code-assist]

Hello @apstndb, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request implements the ISOLATION LEVEL clause for BEGIN and BEGIN RW statements in spanner-cli. It modifies the client-side statement definitions to include the isolation level option and updates the session management to handle the specified isolation level when starting transactions. The supported isolation levels are SERIALIZABLE and REPEATABLE READ.

Highlights

• New Feature: Implements the ISOLATION LEVEL clause for BEGIN and BEGIN RW statements.
• Statement Parsing: Updates the regular expressions and handling logic for BEGIN and BEGIN RW statements to parse the ISOLATION LEVEL clause.
• Transaction Management: Modifies the session management to incorporate the specified isolation level when starting read-write transactions.
• Testing: Adds tests to verify the correct behavior of the isolation level feature.

Changelog

Click here to see the changelog

• README.md
  • Adds documentation for setting session-level default isolation level and transaction-level isolation level using SET DEFAULT_ISOLATION_LEVEL and BEGIN ISOLATION LEVEL.
• client_side_statement_def.go
  • Adds import for cloud.google.com/go/spanner/apiv1/spannerpb.
  • Updates the syntax for BEGIN RW to include ISOLATION LEVEL {SERIALIZABLE|REPEATABLE READ}.
  • Updates the regular expression for BEGIN RW to match the new syntax.
  • Updates the HandleSubmatch function for BEGIN RW to parse the isolation level.
  • Updates the syntax for BEGIN to include ISOLATION LEVEL {SERIALIZABLE|REPEATABLE READ}.
  • Updates the regular expression for BEGIN to match the new syntax.
  • Updates the HandleSubmatch function for BEGIN to parse the isolation level.
  • Adds the parseIsolationLevel function to convert the isolation level string to the corresponding enum value.
• session.go
  • Adds isolationLevel field to the transactionContext struct.
  • Modifies the BeginPendingTransaction function to accept an isolation level and store it in the transactionContext.
  • Modifies the BeginPendingTransaction function to use the session's default isolation level if none is specified in the BEGIN statement.
  • Modifies the BeginReadWriteTransaction function to accept an isolation level.
  • Modifies the BeginReadWriteTransaction function to use the session's default isolation level if none is specified in the BEGIN statement.
  • Passes the isolation level to spanner.NewReadWriteStmtBasedTransactionWithOptions.
  • Stores the isolation level in the transactionContext.
  • Modifies the RunInNewOrExistRwTx function to pass a default isolation level of 0 to BeginReadWriteTransaction.
• session_slow_test.go
  • Modifies TestRequestPriority to pass a default isolation level of 0 to BeginReadWriteTransaction.
  • Adds transactionIsolationLevel to the test struct.
  • Adds new test cases for different isolation levels.
  • Modifies TestIsolationLevel to pass the isolation level to BeginReadWriteTransaction.
• statement_processing_test.go
  • Adds test cases for BEGIN and BEGIN RW statements with different isolation levels and priorities.
  • Adds test cases for BEGIN and BEGIN RW statements with SERIALIZABLE isolation level.
  • Adds test cases for BEGIN and BEGIN RW statements with REPEATABLE READ isolation level.
  • Adds test cases for BEGIN and BEGIN RW statements with REPEATABLE READ isolation level and PRIORITY.
• statements_transaction.go
  • Adds IsolationLevel field to the BeginRwStatement struct.
  • Modifies the BeginRwStatement.Execute function to pass the isolation level to session.BeginReadWriteTransaction.
  • Adds IsolationLevel field to the BeginStatement struct.
  • Modifies the BeginStatement.Execute function to pass the isolation level to session.BeginPendingTransaction.
  • Modifies the BeginStatement.Execute function to pass the isolation level of 0 to session.BeginReadWriteTransaction.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Trivia time!

What are the four standard isolation levels defined by the ANSI SQL standard?

Click here for the answer

The four standard isolation levels defined by the ANSI SQL standard are:

1. Read Uncommitted
2. Read Committed
3. Repeatable Read
4. Serializable

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This PR implements the ISOLATION LEVEL clause for BEGIN and BEGIN RW statements in spanner-mycli. The changes look good overall, but there are a few areas that could be improved for clarity and correctness.

Summary of Findings

• Regex submatch indexing: The indexing of submatches in the regular expressions for BEGIN and BEGIN RW statements is fragile. If the order of optional clauses changes, the indexes will need to be manually updated, which is error-prone. Consider using named capture groups to improve maintainability.
• Default isolation level for SET TRANSACTION: The SET TRANSACTION statement does not currently support setting the isolation level. It might be useful to extend this statement to allow users to explicitly set the isolation level for the current transaction.
• Missing test case: There is no test case for setting the default isolation level to REPEATABLE_READ and then starting a transaction without specifying an isolation level. This scenario should be tested to ensure that the default isolation level is correctly applied.

Merge Readiness

The pull request is almost ready for merging. However, the issues related to regex submatch indexing and the missing test case should be addressed before merging. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. Given the medium severity of the issues found, I recommend that the pull request not be merged until those are addressed.

[gemini-code-assist]

The regex (?is)^BEGIN\s+RW(?:\s+TRANSACTION)?(?:\s+ISOLATION\s+LEVEL\s+(SERIALIZABLE|REPEATABLE\s+READ))?(?:\s+PRIORITY\s+(HIGH|MEDIUM|LOW))?$ uses numeric indexing for submatches. If the order of the optional clauses changes, the indexes in HandleSubmatch will need to be manually updated. Consider using named capture groups to improve maintainability. For example, (?is)^BEGIN\s+RW(?:\s+TRANSACTION)?(?:\s+ISOLATION\s+LEVEL\s+(?P<isolation>SERIALIZABLE|REPEATABLE\s+READ))?(?:\s+PRIORITY\s+(?P<priority>HIGH|MEDIUM|LOW))?$

[gemini-code-assist]

Since you're using numeric indexing for the regex submatches, it's important to ensure that the indexes are correct. It looks like parsePriority is using matched[2] but it's not clear that the second submatch will always be the priority. Consider using named capture groups in the regex to avoid this issue.

[gemini-code-assist]

Similar to the BEGIN RW statement, the regex for the BEGIN statement also uses numeric indexing for submatches. Consider using named capture groups to improve maintainability.

[gemini-code-assist]

The isolation level is hardcoded to 0 here. It would be better to use the session's default isolation level, or to pass the isolation level from the SetTransactionStatement if it was set explicitly.

Suggested change

	if err := s.BeginReadWriteTransaction(ctx, 0, s.currentPriority()); err != nil {
	if err := s.BeginReadWriteTransaction(ctx, s.systemVariables.DefaultIsolationLevel, s.currentPriority()); err != nil {

[gemini-code-assist]

The isolation level is hardcoded to 0 here. It would be better to use the session's default isolation level, or to pass the isolation level from the SetTransactionStatement if it was set explicitly.

Suggested change

	err := session.BeginReadWriteTransaction(ctx, 0, session.tc.priority)
	err := session.BeginReadWriteTransaction(ctx, session.tc.isolationLevel, session.tc.priority)

[gemini-code-assist]

Consider adding a test case where the defaultIsolationLevel is set to REPEATABLE_READ and the transactionIsolationLevel is unspecified. This will ensure that the default isolation level is correctly applied when no explicit isolation level is specified in the transaction.