Improve MySQL CREATE TRIGGER parsing (apache#1998)

Author: mvzink

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<TriggerEvent>,
         /// The table on which the trigger is to be created.
@@ -3980,6 +3989,8 @@ pub enum Statement {
         condition: Option<Expr>,
         /// Execute logic block
         exec_body: Option<TriggerExecBody>,
+        /// For MSSQL and dialects where statements are preceded by `AS`
+        statements_as: bool,
         /// For SQL dialects with statement(s) for a body
         statements: Option<ConditionalStatements>,
         /// 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(())
             }

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,
         })

src/parser/mod.rs

@@ -5593,25 +5593,31 @@ 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,
             or_replace,
             is_constraint,
             name,
             period,
+            period_before_table: true,
             events,
             table_name,
             referenced_table_name,
             referencing,
             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)?;
@@ -9307,10 +9313,10 @@ impl<'a> Parser<'a> {
                 }),
             }))
         } else {
-            return self.expected_ref(
+            self.expected_ref(
                 "{RENAME TO | { RENAME | ADD } VALUE}",
                 self.peek_token_ref(),
-            );
+            )
         }
     }
 

tests/sqlparser_bigquery.rs

@@ -2806,9 +2806,9 @@ fn test_export_data() {
     );
 
     let err = bigquery()
-        .parse_sql_statements(concat!(
+        .parse_sql_statements(
             "EXPORT DATA AS SELECT field1, field2 FROM mydataset.table1 ORDER BY field1 LIMIT 10",
-        ))
+        )
         .unwrap_err();
     assert_eq!(
         err.to_string(),

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(

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;";

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
         }