From 3397ab35b5b57f6be740b89c2f78879942e604e9 Mon Sep 17 00:00:00 2001 From: Michael Victor Zink Date: Wed, 27 Nov 2024 17:14:01 -0800 Subject: [PATCH 1/3] Fix displaying WORK or TRANSACTION after BEGIN --- src/ast/mod.rs | 31 +++++++++++++++++++++++++++---- src/parser/mod.rs | 8 +++++++- tests/sqlparser_common.rs | 6 +++--- tests/sqlparser_mysql.rs | 5 +++++ tests/sqlparser_sqlite.rs | 3 --- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 366bf4d25..3f2fbeb01 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -885,7 +885,7 @@ pub enum Expr { /// Example: /// /// ```sql - /// SELECT (SELECT ',' + name FROM sys.objects FOR XML PATH(''), TYPE).value('.','NVARCHAR(MAX)') + /// SELECT (SELECT ',' + name FROM sys.objects FOR XML PATH(''), TYPE).value('.','NVARCHAR(MAX)') /// SELECT CONVERT(XML,'abc').value('.','NVARCHAR(MAX)').value('.','NVARCHAR(MAX)') /// ``` /// @@ -2929,6 +2929,7 @@ pub enum Statement { StartTransaction { modes: Vec, begin: bool, + transaction: Option, /// Only for SQLite modifier: Option, }, @@ -4629,16 +4630,20 @@ impl fmt::Display for Statement { Statement::StartTransaction { modes, begin: syntax_begin, + transaction, modifier, } => { if *syntax_begin { if let Some(modifier) = *modifier { - write!(f, "BEGIN {} TRANSACTION", modifier)?; + write!(f, "BEGIN {}", modifier)?; } else { - write!(f, "BEGIN TRANSACTION")?; + write!(f, "BEGIN")?; } } else { - write!(f, "START TRANSACTION")?; + write!(f, "START")?; + } + if let Some(transaction) = transaction { + write!(f, " {transaction}")?; } if !modes.is_empty() { write!(f, " {}", display_comma_separated(modes))?; @@ -5133,6 +5138,24 @@ pub enum TruncateCascadeOption { Restrict, } +/// Transaction started with [ TRANSACTION | WORK ] +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum TransactionWorkOption { + Transaction, + Work, +} + +impl Display for TransactionWorkOption { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + TransactionWorkOption::Transaction => write!(f, "TRANSACTION"), + TransactionWorkOption::Work => write!(f, "WORK"), + } + } +} + /// Can use to describe options in create sequence or table column type identity /// [ MINVALUE minvalue | NO MINVALUE ] [ MAXVALUE maxvalue | NO MAXVALUE ] #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b7f5cb866..0a2fb8d25 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -12084,6 +12084,7 @@ impl<'a> Parser<'a> { Ok(Statement::StartTransaction { modes: self.parse_transaction_modes()?, begin: false, + transaction: Some(TransactionWorkOption::Transaction), modifier: None, }) } @@ -12100,10 +12101,15 @@ impl<'a> Parser<'a> { } else { None }; - let _ = self.parse_one_of_keywords(&[Keyword::TRANSACTION, Keyword::WORK]); + let transaction = match self.parse_one_of_keywords(&[Keyword::TRANSACTION, Keyword::WORK]) { + Some(Keyword::TRANSACTION) => Some(TransactionWorkOption::Transaction), + Some(Keyword::WORK) => Some(TransactionWorkOption::Work), + _ => None, + }; Ok(Statement::StartTransaction { modes: self.parse_transaction_modes()?, begin: true, + transaction, modifier, }) } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 4e0cac45b..96c7553fd 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -7736,9 +7736,9 @@ fn parse_start_transaction() { } verified_stmt("START TRANSACTION"); - one_statement_parses_to("BEGIN", "BEGIN TRANSACTION"); - one_statement_parses_to("BEGIN WORK", "BEGIN TRANSACTION"); - one_statement_parses_to("BEGIN TRANSACTION", "BEGIN TRANSACTION"); + verified_stmt("BEGIN"); + verified_stmt("BEGIN WORK"); + verified_stmt("BEGIN TRANSACTION"); verified_stmt("START TRANSACTION ISOLATION LEVEL READ UNCOMMITTED"); verified_stmt("START TRANSACTION ISOLATION LEVEL READ COMMITTED"); diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 943a61718..9539dd810 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -3014,3 +3014,8 @@ fn parse_bitstring_literal() { ))] ); } + +#[test] +fn parse_begin_without_transaction() { + mysql().verified_stmt("BEGIN"); +} diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index c3cfb7a63..a48b9826a 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -527,9 +527,6 @@ fn parse_start_transaction_with_modifier() { sqlite_and_generic().verified_stmt("BEGIN DEFERRED TRANSACTION"); sqlite_and_generic().verified_stmt("BEGIN IMMEDIATE TRANSACTION"); sqlite_and_generic().verified_stmt("BEGIN EXCLUSIVE TRANSACTION"); - sqlite_and_generic().one_statement_parses_to("BEGIN DEFERRED", "BEGIN DEFERRED TRANSACTION"); - sqlite_and_generic().one_statement_parses_to("BEGIN IMMEDIATE", "BEGIN IMMEDIATE TRANSACTION"); - sqlite_and_generic().one_statement_parses_to("BEGIN EXCLUSIVE", "BEGIN EXCLUSIVE TRANSACTION"); let unsupported_dialects = TestedDialects::new( all_dialects() From 0203c17cd69852ba256fc136d3b7eaf166b7ce21 Mon Sep 17 00:00:00 2001 From: Michael Victor Zink Date: Mon, 2 Dec 2024 10:03:54 -0800 Subject: [PATCH 2/3] Rename TransactionWorkOption to BeginTransactionKind --- src/ast/mod.rs | 10 +++++----- src/parser/mod.rs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 3f2fbeb01..f06f77db4 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -2929,7 +2929,7 @@ pub enum Statement { StartTransaction { modes: Vec, begin: bool, - transaction: Option, + transaction: Option, /// Only for SQLite modifier: Option, }, @@ -5142,16 +5142,16 @@ pub enum TruncateCascadeOption { #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub enum TransactionWorkOption { +pub enum BeginTransactionKind { Transaction, Work, } -impl Display for TransactionWorkOption { +impl Display for BeginTransactionKind { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - TransactionWorkOption::Transaction => write!(f, "TRANSACTION"), - TransactionWorkOption::Work => write!(f, "WORK"), + BeginTransactionKind::Transaction => write!(f, "TRANSACTION"), + BeginTransactionKind::Work => write!(f, "WORK"), } } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 0a2fb8d25..8e629f52f 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -12084,7 +12084,7 @@ impl<'a> Parser<'a> { Ok(Statement::StartTransaction { modes: self.parse_transaction_modes()?, begin: false, - transaction: Some(TransactionWorkOption::Transaction), + transaction: Some(BeginTransactionKind::Transaction), modifier: None, }) } @@ -12102,8 +12102,8 @@ impl<'a> Parser<'a> { None }; let transaction = match self.parse_one_of_keywords(&[Keyword::TRANSACTION, Keyword::WORK]) { - Some(Keyword::TRANSACTION) => Some(TransactionWorkOption::Transaction), - Some(Keyword::WORK) => Some(TransactionWorkOption::Work), + Some(Keyword::TRANSACTION) => Some(BeginTransactionKind::Transaction), + Some(Keyword::WORK) => Some(BeginTransactionKind::Work), _ => None, }; Ok(Statement::StartTransaction { From 1e6e33a9af858158698387bfdee818cc8dc7157c Mon Sep 17 00:00:00 2001 From: Michael Victor Zink Date: Mon, 2 Dec 2024 10:04:00 -0800 Subject: [PATCH 3/3] Add back valid sqlite tests --- tests/sqlparser_sqlite.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index a48b9826a..4f23979c5 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -527,6 +527,9 @@ fn parse_start_transaction_with_modifier() { sqlite_and_generic().verified_stmt("BEGIN DEFERRED TRANSACTION"); sqlite_and_generic().verified_stmt("BEGIN IMMEDIATE TRANSACTION"); sqlite_and_generic().verified_stmt("BEGIN EXCLUSIVE TRANSACTION"); + sqlite_and_generic().verified_stmt("BEGIN DEFERRED"); + sqlite_and_generic().verified_stmt("BEGIN IMMEDIATE"); + sqlite_and_generic().verified_stmt("BEGIN EXCLUSIVE"); let unsupported_dialects = TestedDialects::new( all_dialects()