Skip to content

Adding support for parsing CREATE TRIGGER and DROP TRIGGER statements #1352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 53 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
a3fa3b5
Added tests for the TRIGGERs
LucaCappelletti94 Jul 25, 2024
308b1a7
Added missing keywords needed for triggers
LucaCappelletti94 Jul 25, 2024
764707f
Created new module for the AST-related objects for the TRIGGERs
LucaCappelletti94 Jul 25, 2024
637c632
Integrated TRIGGERS in AST enum
LucaCappelletti94 Jul 25, 2024
e760c07
Added methods to parse triggers
LucaCappelletti94 Jul 25, 2024
7474836
Extended testing and resolved some corner cases in triggers
LucaCappelletti94 Jul 25, 2024
9fa36d9
Formatted code
LucaCappelletti94 Jul 26, 2024
d6c3e51
Derived visitor trait and resolved clippy code smells
LucaCappelletti94 Jul 26, 2024
8104de9
Resolved documentation issue and extended test coverage for Triggers
LucaCappelletti94 Jul 26, 2024
9c72a62
Update src/parser/mod.rs
LucaCappelletti94 Jul 26, 2024
5a9a19e
Added support for optional "EACH" display and extended test suite
LucaCappelletti94 Jul 26, 2024
d5a6af1
Removed empty spaced with cargo fmt
LucaCappelletti94 Jul 26, 2024
e6294c1
Renamed TRIGGER-related struct and fixed a copy-paste error and a typo
LucaCappelletti94 Jul 27, 2024
6f7c9c3
Reformatted code<
LucaCappelletti94 Jul 27, 2024
429b202
Merged DropFunctionDesc and TriggerFunctionDesc into FunctionDesc
LucaCappelletti94 Jul 27, 2024
dbaca3a
Updated struct documentation
LucaCappelletti94 Jul 27, 2024
3b26bc0
Refactored code to make use of expect_keyboard where appropriate
LucaCappelletti94 Jul 27, 2024
552825c
Extended test suite for multiple events
LucaCappelletti94 Jul 27, 2024
40993e0
Added coverage for both function and procedure in triggers
LucaCappelletti94 Jul 27, 2024
e26ef31
Extended trigger testing which now includes also functions with multi…
LucaCappelletti94 Jul 27, 2024
bf9da2a
Formatted code
LucaCappelletti94 Jul 27, 2024
c83dd34
Added support for deferrability in TRIGGERS and extended test suite
LucaCappelletti94 Jul 27, 2024
4dcee7f
Added combinatorial option for Row/Statement
LucaCappelletti94 Jul 27, 2024
5f4de6b
Added testing for referencing tables in TRIGGERs
LucaCappelletti94 Jul 27, 2024
eeb2eb0
Added some negative testig
LucaCappelletti94 Jul 27, 2024
b23c5fd
Added support for DropTrigger
LucaCappelletti94 Jul 27, 2024
2555c1e
Removed optionality of TriggerObject as it must be defined
LucaCappelletti94 Jul 27, 2024
8cb0ac9
Formatted code
LucaCappelletti94 Jul 27, 2024
47c2f40
Changed attribute event to events as it refers to a vector of events
LucaCappelletti94 Jul 27, 2024
e81a58c
Extended the test suite
LucaCappelletti94 Jul 27, 2024
c3a96e0
Added support for the TRIGGER return type
LucaCappelletti94 Jul 28, 2024
430a163
Update tests/sqlparser_postgres.rs
LucaCappelletti94 Jul 28, 2024
498aada
Update src/parser/mod.rs
LucaCappelletti94 Jul 28, 2024
ba5d473
Finished refactoring dialect check to simplify code
LucaCappelletti94 Jul 28, 2024
a27cf33
Finished test semplification changing `when` to `period`
LucaCappelletti94 Jul 28, 2024
3887809
Formatted code
LucaCappelletti94 Jul 28, 2024
459c111
Removed part of the `and_then` to adequate to codebase style
LucaCappelletti94 Jul 28, 2024
c043b58
Formatted code
LucaCappelletti94 Jul 28, 2024
3fc2684
Removed more uses of and_then and then
LucaCappelletti94 Jul 28, 2024
09581e7
Restored and made use in triggers of original ConstraintCharacteristics
LucaCappelletti94 Jul 28, 2024
058cf7f
Reverted change relative to ConstraintCharacteristics
LucaCappelletti94 Jul 30, 2024
30199bf
Merge branch 'main' into main
LucaCappelletti94 Jul 30, 2024
7ab8fdb
Update tests/sqlparser_postgres.rs
LucaCappelletti94 Jul 30, 2024
6c5487f
Added support for the CONSTRAINT keyword in triggers
LucaCappelletti94 Jul 30, 2024
c0aab06
Extended test suite and added support for CONSTRAINT keyword and FROM…
LucaCappelletti94 Jul 30, 2024
c198e24
Formatted code
LucaCappelletti94 Jul 30, 2024
d8db70e
Removed itertools dependency and tried to clean up code as much as po…
LucaCappelletti94 Jul 30, 2024
0f81194
Reformatted code
LucaCappelletti94 Jul 30, 2024
169a0be
Fixed comments
LucaCappelletti94 Jul 30, 2024
459dae1
Replaced complete test with partial tests
LucaCappelletti94 Aug 13, 2024
be1a682
Removed dead code
LucaCappelletti94 Aug 13, 2024
4268a9a
Merge remote-tracking branch 'origin/main' into LucaCappelletti94/main
alamb Aug 13, 2024
0182a50
Fix clippy
alamb Aug 13, 2024
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
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ sqlparser_derive = { version = "0.2.0", path = "derive", optional = true }
simple_logger = "5.0"
matches = "0.1"
pretty_assertions = "1"
itertools = "0.13"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a huge fan of adding a new dependency...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was either that or a much uglier Cartesian product. I agree that I would prefer to not add dependencies, but I was asked to try and clean the combinatorial for loop used to test the CREATE TRIGGER statement. I thought that with this being a dev-dependency of a well-known crate, it may be okay. Do let me know which other solution you would prefer to go for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the meantime, I will rewrite it without the use of itertools. Do let me know which variant you prefer.

Copy link
Contributor

@lovasoa lovasoa Jul 30, 2024

Choose a reason for hiding this comment

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

@iffyio might disagree, but I think the nested fors are clearer than the itertools version.

The various arrays should be defined separately, but then I have not problem with

for var1 in arr1 {
  for var2 in arr2 {
    for var3 in arr3 {
      assert!(some_function(var1, var2, var3));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the itertools dependency and tried to make a version as clean as possible in commit d8db70e. Do let me know your opinion.

Copy link
Contributor

@lovasoa lovasoa Jul 31, 2024

Choose a reason for hiding this comment

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

I would have written nested for loops, but I think your version is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok! I find the flattened for-loops a bit easier to read, as otherwise, it gets to a very high level of indentation. Do let me know whether you have any more notes on the PR.


[package.metadata.release]
# Instruct `cargo release` to not run `cargo publish` locally:
Expand Down
15 changes: 14 additions & 1 deletion src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2619,6 +2619,8 @@ pub enum Statement {
/// EXECUTE FUNCTION trigger_function();
/// ```
or_replace: bool,
/// The `CONSTRAINT` keyword is used to create a trigger as a constraint.
is_constraint: bool,
/// The name of the trigger to be created.
name: ObjectName,
/// Determines whether the function is called before, after, or instead of the event.
Expand Down Expand Up @@ -2654,6 +2656,9 @@ pub enum Statement {
events: Vec<TriggerEvent>,
/// The table on which the trigger is to be created.
table_name: ObjectName,
/// The optional referenced table name that can be referenced via
/// the `FROM` keyword.
referenced_table_name: Option<ObjectName>,
/// This keyword immediately precedes the declaration of one or two relation names that provide access to the transition relations of the triggering statement.
referencing: Vec<TriggerReferencing>,
/// This specifies whether the trigger function should be fired once for
Expand Down Expand Up @@ -3447,10 +3452,12 @@ impl fmt::Display for Statement {
}
Statement::CreateTrigger {
or_replace,
is_constraint,
name,
period,
events,
table_name,
referenced_table_name,
referencing,
trigger_object,
condition,
Expand All @@ -3460,14 +3467,20 @@ impl fmt::Display for Statement {
} => {
write!(
f,
"CREATE {or_replace}TRIGGER {name} {period}",
"CREATE {or_replace}{is_constraint}TRIGGER {name} {period}",
or_replace = if *or_replace { "OR REPLACE " } else { "" },
is_constraint = if *is_constraint { "CONSTRAINT " } else { "" },
)?;

if !events.is_empty() {
write!(f, " {}", display_separated(events, " OR "))?;
}
write!(f, " ON {table_name}")?;

if let Some(referenced_table_name) = referenced_table_name {
write!(f, " FROM {referenced_table_name}")?;
}

if let Some(characteristics) = characteristics {
write!(f, " {characteristics}")?;
}
Expand Down
18 changes: 16 additions & 2 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3576,7 +3576,9 @@ impl<'a> Parser<'a> {
} else if self.parse_keyword(Keyword::FUNCTION) {
self.parse_create_function(or_replace, temporary)
} else if self.parse_keyword(Keyword::TRIGGER) {
self.parse_create_trigger(or_replace)
self.parse_create_trigger(or_replace, false)
} else if self.parse_keywords(&[Keyword::CONSTRAINT, Keyword::TRIGGER]) {
self.parse_create_trigger(or_replace, true)
} else if self.parse_keyword(Keyword::MACRO) {
self.parse_create_macro(or_replace, temporary)
} else if self.parse_keyword(Keyword::SECRET) {
Expand Down Expand Up @@ -4196,7 +4198,11 @@ impl<'a> Parser<'a> {
})
}

pub fn parse_create_trigger(&mut self, or_replace: bool) -> Result<Statement, ParserError> {
pub fn parse_create_trigger(
&mut self,
or_replace: bool,
is_constraint: bool,
) -> Result<Statement, ParserError> {
if !dialect_of!(self is PostgreSqlDialect | GenericDialect) {
self.prev_token();
return self.expected("an object type after CREATE", self.peek_token());
Expand All @@ -4209,6 +4215,12 @@ impl<'a> Parser<'a> {
self.expect_keyword(Keyword::ON)?;
let table_name = self.parse_object_name(false)?;

let referenced_table_name = if self.parse_keyword(Keyword::FROM) {
self.parse_object_name(true).ok()
} else {
None
};

let characteristics = self.parse_constraint_characteristics()?;

let mut referencing = vec![];
Expand Down Expand Up @@ -4238,10 +4250,12 @@ impl<'a> Parser<'a> {

Ok(Statement::CreateTrigger {
or_replace,
is_constraint,
name,
period,
events,
table_name,
referenced_table_name,
referencing,
trigger_object,
include_each,
Expand Down
Loading