From f3ac0841fdb0bfcccbec24f4117f80dfd1fc5a5f Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Sat, 18 Oct 2025 17:06:53 +0100 Subject: [PATCH 1/4] SQLite: make period optional for CREATE TRIGGER --- src/ast/ddl.rs | 18 ++++++++---- src/dialect/mssql.rs | 2 +- src/parser/mod.rs | 15 +++++++++- tests/sqlparser_mssql.rs | 2 +- tests/sqlparser_mysql.rs | 2 +- tests/sqlparser_postgres.rs | 12 ++++---- tests/sqlparser_sqlite.rs | 56 +++++++++++++++++++++++++++++++++---- 7 files changed, 85 insertions(+), 22 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 84dc74d96..3a1a2a81d 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -3029,7 +3029,9 @@ pub struct CreateTrigger { /// FOR EACH ROW /// EXECUTE FUNCTION trigger_function(); /// ``` - pub period: TriggerPeriod, + /// + /// This may be omitted in SQLite, the effect is equivalent to BEFORE. + pub period: Option, /// 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. @@ -3098,14 +3100,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, ", "))?; } diff --git a/src/dialect/mssql.rs b/src/dialect/mssql.rs index f1d54cd67..e1902b389 100644 --- a/src/dialect/mssql.rs +++ b/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, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index ef31c41f2..5edc1072d 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5592,7 +5592,20 @@ impl<'a> Parser<'a> { } let name = self.parse_object_name(false)?; - let period = self.parse_trigger_period()?; + let period = if dialect_of!(self is SQLiteDialect) + && self + .peek_one_of_keywords(&[ + Keyword::INSERT, + Keyword::UPDATE, + Keyword::DELETE, + Keyword::TRUNCATE, + ]) + .is_some() + { + None // SQLite: period can be skipped (equivalent to BEFORE) + } else { + Some(self.parse_trigger_period()?) + }; let events = self.parse_keyword_separated(Keyword::OR, Parser::parse_trigger_event)?; self.expect_keyword_is(Keyword::ON)?; diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index e11c79f01..a947db49b 100644 --- a/tests/sqlparser_mssql.rs +++ b/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")]), diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index e0ddecf32..7f728d60b 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -4022,7 +4022,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")]), diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 9d08540ad..b32e538cc 100644 --- a/tests/sqlparser_postgres.rs +++ b/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")]), @@ -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")]), diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index f0d6d9b72..15e539b7c 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -639,7 +639,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"); @@ -685,7 +685,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"); @@ -725,7 +725,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"); @@ -765,7 +765,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"); @@ -809,7 +809,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"); @@ -850,7 +850,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"); @@ -863,6 +863,50 @@ 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"; + match sqlite().verified_stmt(statement7) { + Statement::CreateTrigger(CreateTrigger { + or_alter, + temporary, + or_replace, + is_constraint, + name, + period, + period_before_table, + events, + table_name, + referenced_table_name, + referencing, + trigger_object, + condition, + exec_body: _, + statements_as, + statements: _, + characteristics, + }) => { + assert!(!or_alter); + assert!(!temporary); + assert!(!or_replace); + assert!(!is_constraint); + assert_eq!(name.to_string(), "trg_inherit_asset_models"); + assert_eq!(period, None); + assert!(period_before_table); + assert_eq!(events, vec![TriggerEvent::Insert]); + assert_eq!(table_name.to_string(), "assets"); + assert!(referenced_table_name.is_none()); + assert!(referencing.is_empty()); + assert_eq!( + trigger_object, + Some(TriggerObjectKind::ForEach(TriggerObject::Row)) + ); + assert!(condition.is_none()); + assert!(!statements_as); + assert!(characteristics.is_none()); + } + _ => unreachable!("Expected CREATE TRIGGER statement"), + } } #[test] From b9ea8ed6f7864cfafcb98f5ac0f32d6138bd905d Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Tue, 21 Oct 2025 08:39:29 +0100 Subject: [PATCH 2/4] Remove superfluous description Co-authored-by: Ifeanyi Ubah --- src/ast/ddl.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 3a1a2a81d..d8cd9db83 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -3029,8 +3029,6 @@ pub struct CreateTrigger { /// FOR EACH ROW /// EXECUTE FUNCTION trigger_function(); /// ``` - /// - /// This may be omitted in SQLite, the effect is equivalent to BEFORE. pub period: Option, /// 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, From c50b996582c3e8fc0dc3291fb312e35b7125b5aa Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Tue, 21 Oct 2025 08:39:45 +0100 Subject: [PATCH 3/4] Shorten test for new condition Co-authored-by: Ifeanyi Ubah --- tests/sqlparser_sqlite.rs | 42 +-------------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index 15e539b7c..108488a08 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -866,47 +866,7 @@ fn test_create_trigger() { // 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"; - match sqlite().verified_stmt(statement7) { - Statement::CreateTrigger(CreateTrigger { - or_alter, - temporary, - or_replace, - is_constraint, - name, - period, - period_before_table, - events, - table_name, - referenced_table_name, - referencing, - trigger_object, - condition, - exec_body: _, - statements_as, - statements: _, - characteristics, - }) => { - assert!(!or_alter); - assert!(!temporary); - assert!(!or_replace); - assert!(!is_constraint); - assert_eq!(name.to_string(), "trg_inherit_asset_models"); - assert_eq!(period, None); - assert!(period_before_table); - assert_eq!(events, vec![TriggerEvent::Insert]); - assert_eq!(table_name.to_string(), "assets"); - assert!(referenced_table_name.is_none()); - assert!(referencing.is_empty()); - assert_eq!( - trigger_object, - Some(TriggerObjectKind::ForEach(TriggerObject::Row)) - ); - assert!(condition.is_none()); - assert!(!statements_as); - assert!(characteristics.is_none()); - } - _ => unreachable!("Expected CREATE TRIGGER statement"), - } + sqlite().verified_stmt(statement7); } #[test] From b4cfbd55ff19639800b8aae4e81fafd188df2754 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Tue, 21 Oct 2025 08:47:28 +0100 Subject: [PATCH 4/4] Make trigger period optional in all dialects --- src/parser/mod.rs | 15 +-------------- tests/sqlparser_postgres.rs | 2 +- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 5edc1072d..4ed60f007 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5592,20 +5592,7 @@ impl<'a> Parser<'a> { } let name = self.parse_object_name(false)?; - let period = if dialect_of!(self is SQLiteDialect) - && self - .peek_one_of_keywords(&[ - Keyword::INSERT, - Keyword::UPDATE, - Keyword::DELETE, - Keyword::TRUNCATE, - ]) - .is_some() - { - None // SQLite: period can be skipped (equivalent to BEFORE) - } else { - Some(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)?; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index b32e538cc..bcc154287 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -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",