SQLite: make period optional for CREATE TRIGGER (#2071)

Co-authored-by: Ifeanyi Ubah <[email protected]>

Author: takluyver

src/ast/ddl.rs

@@ -3063,7 +3063,7 @@ pub struct CreateTrigger {
     /// FOR EACH ROW
     /// EXECUTE FUNCTION trigger_function();
     /// ```
-    pub period: TriggerPeriod,
+    pub period: Option<TriggerPeriod>,
     /// Whether the trigger period was specified before the target table name.
     /// This does not refer to whether the period is BEFORE, AFTER, or INSTEAD OF,
     /// but rather the position of the period clause in relation to the table name.
@@ -3132,14 +3132,18 @@ impl Display for CreateTrigger {
         )?;
 
         if *period_before_table {
-            write!(f, "{period}")?;
+            if let Some(p) = period {
+                write!(f, "{p} ")?;
+            }
             if !events.is_empty() {
-                write!(f, " {}", display_separated(events, " OR "))?;
+                write!(f, "{} ", display_separated(events, " OR "))?;
             }
-            write!(f, " ON {table_name}")?;
-        } else {
             write!(f, "ON {table_name}")?;
-            write!(f, " {period}")?;
+        } else {
+            write!(f, "ON {table_name} ")?;
+            if let Some(p) = period {
+                write!(f, "{p}")?;
+            }
             if !events.is_empty() {
                 write!(f, " {}", display_separated(events, ", "))?;
             }

src/dialect/mssql.rs

@@ -258,7 +258,7 @@ impl MsSqlDialect {
             or_replace: false,
             is_constraint: false,
             name,
-            period,
+            period: Some(period),
             period_before_table: false,
             events,
             table_name,

src/parser/mod.rs

@@ -5592,7 +5592,7 @@ impl<'a> Parser<'a> {
         }
 
         let name = self.parse_object_name(false)?;
-        let period = self.parse_trigger_period()?;
+        let period = self.maybe_parse(|parser| parser.parse_trigger_period())?;
 
         let events = self.parse_keyword_separated(Keyword::OR, Parser::parse_trigger_event)?;
         self.expect_keyword_is(Keyword::ON)?;

tests/sqlparser_mssql.rs

@@ -2392,7 +2392,7 @@ fn parse_create_trigger() {
             or_replace: false,
             is_constraint: false,
             name: ObjectName::from(vec![Ident::new("reminder1")]),
-            period: TriggerPeriod::After,
+            period: Some(TriggerPeriod::After),
             period_before_table: false,
             events: vec![TriggerEvent::Insert, TriggerEvent::Update(vec![]),],
             table_name: ObjectName::from(vec![Ident::new("Sales"), Ident::new("Customer")]),

tests/sqlparser_mysql.rs

@@ -4034,7 +4034,7 @@ fn parse_create_trigger() {
             or_replace: false,
             is_constraint: false,
             name: ObjectName::from(vec![Ident::new("emp_stamp")]),
-            period: TriggerPeriod::Before,
+            period: Some(TriggerPeriod::Before),
             period_before_table: true,
             events: vec![TriggerEvent::Insert],
             table_name: ObjectName::from(vec![Ident::new("emp")]),

tests/sqlparser_postgres.rs

@@ -5640,7 +5640,7 @@ fn parse_create_simple_before_insert_trigger() {
         or_replace: false,
         is_constraint: false,
         name: ObjectName::from(vec![Ident::new("check_insert")]),
-        period: TriggerPeriod::Before,
+        period: Some(TriggerPeriod::Before),
         period_before_table: true,
         events: vec![TriggerEvent::Insert],
         table_name: ObjectName::from(vec![Ident::new("accounts")]),
@@ -5672,7 +5672,7 @@ fn parse_create_after_update_trigger_with_condition() {
         or_replace: false,
         is_constraint: false,
         name: ObjectName::from(vec![Ident::new("check_update")]),
-        period: TriggerPeriod::After,
+        period: Some(TriggerPeriod::After),
         period_before_table: true,
         events: vec![TriggerEvent::Update(vec![])],
         table_name: ObjectName::from(vec![Ident::new("accounts")]),
@@ -5711,7 +5711,7 @@ fn parse_create_instead_of_delete_trigger() {
         or_replace: false,
         is_constraint: false,
         name: ObjectName::from(vec![Ident::new("check_delete")]),
-        period: TriggerPeriod::InsteadOf,
+        period: Some(TriggerPeriod::InsteadOf),
         period_before_table: true,
         events: vec![TriggerEvent::Delete],
         table_name: ObjectName::from(vec![Ident::new("accounts")]),
@@ -5743,7 +5743,7 @@ fn parse_create_trigger_with_multiple_events_and_deferrable() {
         or_replace: false,
         is_constraint: true,
         name: ObjectName::from(vec![Ident::new("check_multiple_events")]),
-        period: TriggerPeriod::Before,
+        period: Some(TriggerPeriod::Before),
         period_before_table: true,
         events: vec![
             TriggerEvent::Insert,
@@ -5783,7 +5783,7 @@ fn parse_create_trigger_with_referencing() {
         or_replace: false,
         is_constraint: false,
         name: ObjectName::from(vec![Ident::new("check_referencing")]),
-        period: TriggerPeriod::Before,
+        period: Some(TriggerPeriod::Before),
         period_before_table: true,
         events: vec![TriggerEvent::Insert],
         table_name: ObjectName::from(vec![Ident::new("accounts")]),
@@ -5830,7 +5830,7 @@ fn parse_create_trigger_invalid_cases() {
         ),
         (
             "CREATE TRIGGER check_update TOMORROW UPDATE ON accounts EXECUTE FUNCTION check_account_update",
-            "Expected: one of FOR or BEFORE or AFTER or INSTEAD, found: TOMORROW"
+            "Expected: one of INSERT or UPDATE or DELETE or TRUNCATE, found: TOMORROW"
         ),
         (
             "CREATE TRIGGER check_update BEFORE SAVE ON accounts EXECUTE FUNCTION check_account_update",
@@ -6099,7 +6099,7 @@ fn parse_trigger_related_functions() {
             or_replace: false,
             is_constraint: false,
             name: ObjectName::from(vec![Ident::new("emp_stamp")]),
-            period: TriggerPeriod::Before,
+            period: Some(TriggerPeriod::Before),
             period_before_table: true,
             events: vec![TriggerEvent::Insert, TriggerEvent::Update(vec![])],
             table_name: ObjectName::from(vec![Ident::new("emp")]),

tests/sqlparser_sqlite.rs

@@ -647,7 +647,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "trg_inherit_asset_models");
-            assert_eq!(period, TriggerPeriod::After);
+            assert_eq!(period, Some(TriggerPeriod::After));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Insert]);
             assert_eq!(table_name.to_string(), "assets");
@@ -693,7 +693,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "log_new_user");
-            assert_eq!(period, TriggerPeriod::After);
+            assert_eq!(period, Some(TriggerPeriod::After));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Insert]);
             assert_eq!(table_name.to_string(), "users");
@@ -733,7 +733,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "cleanup_orders");
-            assert_eq!(period, TriggerPeriod::After);
+            assert_eq!(period, Some(TriggerPeriod::After));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Delete]);
             assert_eq!(table_name.to_string(), "customers");
@@ -773,7 +773,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "trg_before_update");
-            assert_eq!(period, TriggerPeriod::Before);
+            assert_eq!(period, Some(TriggerPeriod::Before));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Update(Vec::new())]);
             assert_eq!(table_name.to_string(), "products");
@@ -817,7 +817,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "trg_instead_of_insert");
-            assert_eq!(period, TriggerPeriod::InsteadOf);
+            assert_eq!(period, Some(TriggerPeriod::InsteadOf));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Insert]);
             assert_eq!(table_name.to_string(), "my_view");
@@ -858,7 +858,7 @@ fn test_create_trigger() {
             assert!(!or_replace);
             assert!(!is_constraint);
             assert_eq!(name.to_string(), "temp_trigger");
-            assert_eq!(period, TriggerPeriod::After);
+            assert_eq!(period, Some(TriggerPeriod::After));
             assert!(period_before_table);
             assert_eq!(events, vec![TriggerEvent::Insert]);
             assert_eq!(table_name.to_string(), "temp_table");
@@ -871,6 +871,10 @@ fn test_create_trigger() {
         }
         _ => unreachable!("Expected CREATE TRIGGER statement"),
     }
+
+    // We test a trigger defined without a period (BEFORE/AFTER/INSTEAD OF)
+    let statement7 = "CREATE TRIGGER trg_inherit_asset_models INSERT ON assets FOR EACH ROW BEGIN INSERT INTO users (name) SELECT pam.name FROM users AS pam; END";
+    sqlite().verified_stmt(statement7);
 }
 
 #[test]