Skip to content

Fixed transaction handling for snowflake - #2263

Merged
iffyio merged 2 commits into
apache:mainfrom
romanoff:fix_begin_transaction_for_snowflake
Mar 9, 2026
Merged

Fixed transaction handling for snowflake#2263
iffyio merged 2 commits into
apache:mainfrom
romanoff:fix_begin_transaction_for_snowflake

Conversation

@romanoff

Copy link
Copy Markdown

Sample query:

BEGIN TRANSACTION;
DROPTABLE IF EXISTS bla;
COMMIT;

Spec: https://docs.snowflake.com/en/sql-reference/sql/begin

Comment threadsrc/dialect/snowflake.rs Outdated
|| parser.peek_keyword(Keyword::WORK)
|| parser.peek_keyword(Keyword::NAME)
|| parser.peek_token().token == Token::SemiColon
|| parser.peek_token().token == Token::EOF

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.

can we add a test case with only a begin keyword (for the EOF case)?

Comment threadsrc/dialect/snowflake.rs Outdated
// Snowflake supports both `BEGIN TRANSACTION` and `BEGIN ... END` blocks.
// If the next keyword indicates a transaction statement, let the
// standard parse_begin() handle it.
if parser.peek_keyword(Keyword::TRANSACTION)

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.

can we use parser.peek_one_of_keywords() and parser.peek_token_ref() for the two cases to simplify or make the op less expensive?

@romanoff

Copy link
Copy Markdown
Author

@iffyio Updated. Thank you

@iffyioiffyio 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.

LGTM! Thanks @romanoff!

@iffyio
iffyio added this pull request to the merge queueMar 9, 2026
Merged via the queue into apache:main with commit 13b88a3Mar 9, 2026
10 checks passed
Sign up for freeto 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.

2 participants

@romanoff@iffyio