You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "stevenliebregt (via GitHub)" <gi...@apache.org> on 2025/09/23 10:06:45 UTC
[PR] Support `CREATE SEQUENCE` options in any order [datafusion-sqlparser-rs]
stevenliebregt opened a new pull request, #2043:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2043
This changes the parsing of the `CREATE SEQUENCE` options to allow the individual options to appear in any order.
The reason I'm opening this pull request is that the Postgres `pg_dump` utility dumps `CREATE SEQUENCE` statements like this:
```sql
CREATE SEQUENCE name6
AS INTEGER
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1
```
Where the `START WITH 1` appears before the `INCREMENT BY 1`, Postgres allows this but the parser did not.
There should not be any noticable performance impact I think. I did run the benchmarks and did not see any impact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support `CREATE SEQUENCE` options in any order [datafusion-sqlparser-rs]
Posted by "stevenliebregt (via GitHub)" <gi...@apache.org>.
stevenliebregt commented on PR #2043:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2043#issuecomment-3333594329
@iffyio thanks for the review, I'll be unable to address the comments for the next 2 weeks, but after that I'll address them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support `CREATE SEQUENCE` options in any order [datafusion-sqlparser-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2043:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2043#issuecomment-3573504411
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support `CREATE SEQUENCE` options in any order [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2043:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2043#discussion_r2378780585
##########
src/parser/mod.rs:
##########
@@ -17019,44 +17019,68 @@ impl<'a> Parser<'a> {
fn parse_create_sequence_options(&mut self) -> Result<Vec<SequenceOptions>, ParserError> {
let mut sequence_options = vec![];
- //[ INCREMENT [ BY ] increment ]
- if self.parse_keywords(&[Keyword::INCREMENT]) {
- if self.parse_keywords(&[Keyword::BY]) {
- sequence_options.push(SequenceOptions::IncrementBy(self.parse_number()?, true));
- } else {
- sequence_options.push(SequenceOptions::IncrementBy(self.parse_number()?, false));
- }
- }
- //[ MINVALUE minvalue | NO MINVALUE ]
- if self.parse_keyword(Keyword::MINVALUE) {
- sequence_options.push(SequenceOptions::MinValue(Some(self.parse_number()?)));
- } else if self.parse_keywords(&[Keyword::NO, Keyword::MINVALUE]) {
- sequence_options.push(SequenceOptions::MinValue(None));
- }
- //[ MAXVALUE maxvalue | NO MAXVALUE ]
- if self.parse_keywords(&[Keyword::MAXVALUE]) {
- sequence_options.push(SequenceOptions::MaxValue(Some(self.parse_number()?)));
- } else if self.parse_keywords(&[Keyword::NO, Keyword::MAXVALUE]) {
- sequence_options.push(SequenceOptions::MaxValue(None));
- }
- //[ START [ WITH ] start ]
- if self.parse_keywords(&[Keyword::START]) {
- if self.parse_keywords(&[Keyword::WITH]) {
- sequence_options.push(SequenceOptions::StartWith(self.parse_number()?, true));
- } else {
- sequence_options.push(SequenceOptions::StartWith(self.parse_number()?, false));
+ // To disallow duplicate options
+ let has_increment = false;
+ let has_minvalue = false;
+ let has_maxvalue = false;
+ let has_start = false;
+ let has_cache = false;
+ let has_cycle = false;
+
+ let mut should_continue_loop = true;
+ while should_continue_loop {
Review Comment:
```suggestion
loop {
```
It looks like we can skip the bool var and instead use a combination of `continue` and `break` to manage the loop?
##########
tests/sqlparser_postgres.rs:
##########
@@ -309,6 +309,15 @@ fn parse_create_sequence() {
pg().parse_sql_statements("CREATE SEQUENCE foo INCREMENT 1 NO MINVALUE NO"),
Err(ParserError::ParserError(_))
));
+
+ let sql9 = "CREATE SEQUENCE name6
+ AS INTEGER
+ START WITH 1
+ INCREMENT BY 1
+ NO MINVALUE
+ NO MAXVALUE
+ CACHE 1";
+ pg().one_statement_parses_to(sql9, "CREATE SEQUENCE name6 AS INTEGER START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1");
Review Comment:
```suggestion
pg().verified_stmt("CREATE SEQUENCE name6 AS INTEGER START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1");
```
it looks like this should be equivalent?
##########
tests/sqlparser_postgres.rs:
##########
@@ -309,6 +309,15 @@ fn parse_create_sequence() {
pg().parse_sql_statements("CREATE SEQUENCE foo INCREMENT 1 NO MINVALUE NO"),
Err(ParserError::ParserError(_))
));
+
+ let sql9 = "CREATE SEQUENCE name6
Review Comment:
Can we add a test case with duplicate options?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support `CREATE SEQUENCE` options in any order [datafusion-sqlparser-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2043: Support `CREATE SEQUENCE` options in any order
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org