Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/ast/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<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.
Expand Down Expand Up @@ -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, ", "))?;
}
Expand Down
2 changes: 1 addition & 1 deletion src/dialect/mssql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl MsSqlDialect {
or_replace: false,
is_constraint: false,
name,
period,
period: Some(period),
period_before_table: false,
events,
table_name,
Expand Down
15 changes: 14 additions & 1 deletion src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?)
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do something like this to support optionality

let period = self.maybe_parse(|parser| parser.parse_trigger_period());

It would imply the same behavior for other dialects than sqlite but that's okay for the parser to be more permissive

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Allowing this for all dialects changes an error message that is tested for the Postgres dialect. For now I'll change that test so that everything passes, but if you'd prefer a different direction, like making it conditional on the dialect again, I can easily cut that commit off.


let events = self.parse_keyword_separated(Keyword::OR, Parser::parse_trigger_event)?;
self.expect_keyword_is(Keyword::ON)?;
Expand Down
2 changes: 1 addition & 1 deletion tests/sqlparser_mssql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]),
Expand Down
2 changes: 1 addition & 1 deletion tests/sqlparser_mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]),
Expand Down
12 changes: 6 additions & 6 deletions tests/sqlparser_postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]),
Expand Down Expand Up @@ -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")]),
Expand Down Expand Up @@ -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")]),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")]),
Expand Down Expand Up @@ -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")]),
Expand Down
56 changes: 50 additions & 6 deletions tests/sqlparser_sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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]
Expand Down