Skip to content

feat: support SHOW TRANSACTION variables and transaction_isolation alias#822

Open
olavloite wants to merge 3 commits into
mainfrom
feat/604-show-transaction-variables
Open

feat: support SHOW TRANSACTION variables and transaction_isolation alias#822
olavloite wants to merge 3 commits into
mainfrom
feat/604-show-transaction-variables

Conversation

@olavloite

@olavloite olavloite commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Support SHOW TRANSACTION ISOLATION LEVEL, SHOW TRANSACTION READ ONLY, and SHOW TRANSACTION DEFERRABLE syntax (standard in some PostgreSQL clients) and map transaction_isolation to the isolation_level property in the connection.

Fixes #604

@olavloite olavloite requested a review from a team as a code owner June 11, 2026 12:32

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for parsing and executing SHOW TRANSACTION statements (such as ISOLATION LEVEL, READ ONLY, and DEFERRABLE) in the Spanner driver, along with corresponding tests. The review feedback suggests several improvements: ensuring case-insensitive matching for connection variables, removing support for the invalid SHOW TRANSACTION READ WRITE command (which incorrectly maps to transaction_read_only), updating syntax error messages to accurately list supported options, and adjusting the test suite accordingly.

Comment thread conn.go Outdated
Comment thread parser/statements.go
Comment thread parser/statements.go
Comment thread parser/statements.go
Comment thread parser/statements_test.go
@olavloite olavloite force-pushed the feat/604-show-transaction-variables branch from 0109a96 to 9dfec4a Compare June 11, 2026 12:54
@olavloite

Copy link
Copy Markdown
Collaborator Author

LGTM. This implements standard PG compatibility for SHOW TRANSACTION ISOLATION LEVEL, SHOW TRANSACTION READ ONLY, SHOW TRANSACTION DEFERRABLE, and aliases like transaction_isolation. It nicely handles PostgreSQL lowercase formatting for isolation level values and defaults to serializable in PG dialect. Good test coverage.

@olavloite

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for PostgreSQL-specific transaction-related SHOW statements (such as SHOW TRANSACTION ISOLATION LEVEL) and property aliases (like transaction_isolation) in the connection state. Feedback on the changes includes addressing a resource leak in TestStatementExecutor_ShowTransaction caused by deferred Close calls on a reassigned rows variable, handling an ignored error when setting the default isolation level, and correcting test logic in parser/statements_test.go that incorrectly overrides wantErr for an invalid PostgreSQL statement.

Comment thread client_side_statement_test.go
Comment thread connection_properties.go
Comment thread parser/statements_test.go Outdated
@olavloite olavloite force-pushed the feat/604-show-transaction-variables branch from 0faae9f to 50ca434 Compare June 12, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support SHOW TRANSACTION variables

1 participant