SQLite: make period optional for CREATE TRIGGER #2071
+31
−23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
takluyver
force-pushed
the
sqlite-trigger-no-period
branch
from
6e366a8 to
f3ac084
Compare
iffyio
reviewed
Oct 21, 2025
src/parser/mod.rs
Outdated
Comment on lines
5595
to
5608
| 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()?) | ||
| }; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Co-authored-by: Ifeanyi Ubah <[email protected]>
Co-authored-by: Ifeanyi Ubah <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I just tried out the new CREATE TRIGGER support for SQLite, but ran across a limitation. SQLite's CREATE TRIGGER statement makes the period (
BEFORE,AFTER,INSTEAD OF) optional. Leaving it out is equivalent to BEFORE. This allows that in sqlparser, making the field anOption<TriggerPeriod>.This may well be clumsily implemented - I don't write all that much Rust, so suggestions are welcome. I've enabled this only for SQLite for now - I haven't researched if the same syntax is legal in any other dialects.
cc @LucaCappelletti94 - and thanks for adding the overall CREATE TRIGGER support 🙂