From a2940fd757f489f649dd4c96411c80b54cf8fb3f Mon Sep 17 00:00:00 2001 From: Michael Victor Zink Date: Thu, 7 Aug 2025 09:17:12 -0700 Subject: [PATCH] Improve MySQL `CREATE TRIGGER` parsing MySQL uses a statement body similar to MSSQL (but without the `AS` keyword) instead of `EXECUTE` body style used in Postgres and standard SQL. But unlike MSSQL, MySQL puts the trigger period before the target table. We add some flags to indicate these differences and allow parsing and round tripping MySQL triggers. The main benefit is that we can now handle MySQL triggers which include `BEGIN; ... END;` compound statements. --- src/ast/mod.rs | 20 ++++++++++++++++++-- src/dialect/mssql.rs | 2 ++ src/parser/mod.rs | 18 ++++++++++++------ tests/sqlparser_mssql.rs | 2 ++ tests/sqlparser_mysql.rs | 17 +++++++++++------ tests/sqlparser_postgres.rs | 14 +++++++++++++- 6 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 4a864cfa..0bf412e8 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -3962,6 +3962,15 @@ pub enum Statement { /// EXECUTE FUNCTION trigger_function(); /// ``` period: TriggerPeriod, + /// Whether the trigger period was specified before the target table name. + /// + /// ```sql + /// -- period_before_table == true: Postgres, MySQL, and standard SQL + /// CREATE TRIGGER t BEFORE INSERT ON table_name ...; + /// -- period_before_table == false: MSSQL + /// CREATE TRIGGER t ON table_name BEFORE INSERT ...; + /// ``` + period_before_table: bool, /// Multiple events can be specified using OR, such as `INSERT`, `UPDATE`, `DELETE`, or `TRUNCATE`. events: Vec, /// The table on which the trigger is to be created. @@ -3980,6 +3989,8 @@ pub enum Statement { condition: Option, /// Execute logic block exec_body: Option, + /// For MSSQL and dialects where statements are preceded by `AS` + statements_as: bool, /// For SQL dialects with statement(s) for a body statements: Option, /// The characteristic of the trigger, which include whether the trigger is `DEFERRABLE`, `INITIALLY DEFERRED`, or `INITIALLY IMMEDIATE`, @@ -4944,6 +4955,7 @@ impl fmt::Display for Statement { or_replace, is_constraint, name, + period_before_table, period, events, table_name, @@ -4953,6 +4965,7 @@ impl fmt::Display for Statement { condition, include_each, exec_body, + statements_as, statements, characteristics, } => { @@ -4964,7 +4977,7 @@ impl fmt::Display for Statement { is_constraint = if *is_constraint { "CONSTRAINT " } else { "" }, )?; - if exec_body.is_some() { + if *period_before_table { write!(f, "{period}")?; if !events.is_empty() { write!(f, " {}", display_separated(events, " OR "))?; @@ -5002,7 +5015,10 @@ impl fmt::Display for Statement { write!(f, " EXECUTE {exec_body}")?; } if let Some(statements) = statements { - write!(f, " AS {statements}")?; + if *statements_as { + write!(f, " AS")?; + } + write!(f, " {statements}")?; } Ok(()) } diff --git a/src/dialect/mssql.rs b/src/dialect/mssql.rs index 36bd222b..518dab24 100644 --- a/src/dialect/mssql.rs +++ b/src/dialect/mssql.rs @@ -257,6 +257,7 @@ impl MsSqlDialect { is_constraint: false, name, period, + period_before_table: false, events, table_name, referenced_table_name: None, @@ -265,6 +266,7 @@ impl MsSqlDialect { include_each: false, condition: None, exec_body: None, + statements_as: true, statements, characteristics: None, }) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index d9fe7999..5f8ee6c9 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5593,9 +5593,13 @@ impl<'a> Parser<'a> { .then(|| self.parse_expr()) .transpose()?; - self.expect_keyword_is(Keyword::EXECUTE)?; - - let exec_body = self.parse_trigger_exec_body()?; + let mut exec_body = None; + let mut statements = None; + if self.parse_keyword(Keyword::EXECUTE) { + exec_body = Some(self.parse_trigger_exec_body()?); + } else { + statements = Some(self.parse_conditional_statements(&[Keyword::END])?); + } Ok(Statement::CreateTrigger { or_alter, @@ -5603,6 +5607,7 @@ impl<'a> Parser<'a> { is_constraint, name, period, + period_before_table: true, events, table_name, referenced_table_name, @@ -5610,8 +5615,9 @@ impl<'a> Parser<'a> { trigger_object, include_each, condition, - exec_body: Some(exec_body), - statements: None, + exec_body, + statements_as: false, + statements, characteristics, }) } @@ -6537,7 +6543,7 @@ impl<'a> Parser<'a> { let args = if self.consume_token(&Token::LParen) { if self.consume_token(&Token::RParen) { - None + Some(vec![]) } else { let args = self.parse_comma_separated(Parser::parse_function_arg)?; self.expect_token(&Token::RParen)?; diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 50c6448d..63e4eecb 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -2376,6 +2376,7 @@ fn parse_create_trigger() { is_constraint: false, name: ObjectName::from(vec![Ident::new("reminder1")]), period: TriggerPeriod::After, + period_before_table: false, events: vec![TriggerEvent::Insert, TriggerEvent::Update(vec![]),], table_name: ObjectName::from(vec![Ident::new("Sales"), Ident::new("Customer")]), referenced_table_name: None, @@ -2384,6 +2385,7 @@ fn parse_create_trigger() { include_each: false, condition: None, exec_body: None, + statements_as: true, statements: Some(ConditionalStatements::Sequence { statements: vec![Statement::RaisError { message: Box::new(Expr::Value( diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index cf4e24ad..c13f2266 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -3914,11 +3914,8 @@ fn parse_looks_like_single_line_comment() { #[test] fn parse_create_trigger() { - let sql_create_trigger = r#" - CREATE TRIGGER emp_stamp BEFORE INSERT ON emp - FOR EACH ROW EXECUTE FUNCTION emp_stamp(); - "#; - let create_stmt = mysql().one_statement_parses_to(sql_create_trigger, ""); + let sql_create_trigger = r#"CREATE TRIGGER emp_stamp BEFORE INSERT ON emp FOR EACH ROW EXECUTE FUNCTION emp_stamp()"#; + let create_stmt = mysql().verified_stmt(sql_create_trigger); assert_eq!( create_stmt, Statement::CreateTrigger { @@ -3927,6 +3924,7 @@ fn parse_create_trigger() { is_constraint: false, name: ObjectName::from(vec![Ident::new("emp_stamp")]), period: TriggerPeriod::Before, + period_before_table: true, events: vec![TriggerEvent::Insert], table_name: ObjectName::from(vec![Ident::new("emp")]), referenced_table_name: None, @@ -3938,15 +3936,22 @@ fn parse_create_trigger() { exec_type: TriggerExecBodyType::Function, func_desc: FunctionDesc { name: ObjectName::from(vec![Ident::new("emp_stamp")]), - args: None, + args: Some(vec![]), } }), + statements_as: false, statements: None, characteristics: None, } ); } +#[test] +fn parse_create_trigger_compound_statement() { + mysql_and_generic().verified_stmt("CREATE TRIGGER mytrigger BEFORE INSERT ON mytable FOR EACH ROW BEGIN SET NEW.a = 1; SET NEW.b = 2; END"); + mysql_and_generic().verified_stmt("CREATE TRIGGER tr AFTER INSERT ON t1 FOR EACH ROW BEGIN INSERT INTO t2 VALUES (NEW.id); END"); +} + #[test] fn parse_drop_trigger() { let sql_drop_trigger = "DROP TRIGGER emp_stamp;"; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 6c00b1f1..997384a6 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -5552,6 +5552,7 @@ fn parse_create_simple_before_insert_trigger() { is_constraint: false, name: ObjectName::from(vec![Ident::new("check_insert")]), period: TriggerPeriod::Before, + period_before_table: true, events: vec![TriggerEvent::Insert], table_name: ObjectName::from(vec![Ident::new("accounts")]), referenced_table_name: None, @@ -5566,6 +5567,7 @@ fn parse_create_simple_before_insert_trigger() { args: None, }, }), + statements_as: false, statements: None, characteristics: None, }; @@ -5582,6 +5584,7 @@ fn parse_create_after_update_trigger_with_condition() { is_constraint: false, name: ObjectName::from(vec![Ident::new("check_update")]), period: TriggerPeriod::After, + period_before_table: true, events: vec![TriggerEvent::Update(vec![])], table_name: ObjectName::from(vec![Ident::new("accounts")]), referenced_table_name: None, @@ -5603,6 +5606,7 @@ fn parse_create_after_update_trigger_with_condition() { args: None, }, }), + statements_as: false, statements: None, characteristics: None, }; @@ -5619,6 +5623,7 @@ fn parse_create_instead_of_delete_trigger() { is_constraint: false, name: ObjectName::from(vec![Ident::new("check_delete")]), period: TriggerPeriod::InsteadOf, + period_before_table: true, events: vec![TriggerEvent::Delete], table_name: ObjectName::from(vec![Ident::new("accounts")]), referenced_table_name: None, @@ -5633,6 +5638,7 @@ fn parse_create_instead_of_delete_trigger() { args: None, }, }), + statements_as: false, statements: None, characteristics: None, }; @@ -5649,6 +5655,7 @@ fn parse_create_trigger_with_multiple_events_and_deferrable() { is_constraint: true, name: ObjectName::from(vec![Ident::new("check_multiple_events")]), period: TriggerPeriod::Before, + period_before_table: true, events: vec![ TriggerEvent::Insert, TriggerEvent::Update(vec![]), @@ -5667,6 +5674,7 @@ fn parse_create_trigger_with_multiple_events_and_deferrable() { args: None, }, }), + statements_as: false, statements: None, characteristics: Some(ConstraintCharacteristics { deferrable: Some(true), @@ -5687,6 +5695,7 @@ fn parse_create_trigger_with_referencing() { is_constraint: false, name: ObjectName::from(vec![Ident::new("check_referencing")]), period: TriggerPeriod::Before, + period_before_table: true, events: vec![TriggerEvent::Insert], table_name: ObjectName::from(vec![Ident::new("accounts")]), referenced_table_name: None, @@ -5712,6 +5721,7 @@ fn parse_create_trigger_with_referencing() { args: None, }, }), + statements_as: false, statements: None, characteristics: None, }; @@ -5994,6 +6004,7 @@ fn parse_trigger_related_functions() { is_constraint: false, name: ObjectName::from(vec![Ident::new("emp_stamp")]), period: TriggerPeriod::Before, + period_before_table: true, events: vec![TriggerEvent::Insert, TriggerEvent::Update(vec![])], table_name: ObjectName::from(vec![Ident::new("emp")]), referenced_table_name: None, @@ -6005,9 +6016,10 @@ fn parse_trigger_related_functions() { exec_type: TriggerExecBodyType::Function, func_desc: FunctionDesc { name: ObjectName::from(vec![Ident::new("emp_stamp")]), - args: None, + args: Some(vec![]), } }), + statements_as: false, statements: None, characteristics: None }