s5dsn-eqee commented on code in PR #2414:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2414#discussion_r4315419942
##########
src/parser/mod.rs:
##########
@@ -20650,72 +20651,88 @@ impl<'a> Parser<'a> {
let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT,
Keyword::EXISTS]);
//name
let name = self.parse_object_name(false)?;
- //[ AS data_type ]
- let mut data_type: Option<DataType> = None;
- if self.parse_keywords(&[Keyword::AS]) {
- data_type = Some(self.parse_data_type()?)
- }
- let sequence_options = self.parse_create_sequence_options()?;
- // [ OWNED BY { table_name.column_name | NONE } ]
- let owned_by = if self.parse_keywords(&[Keyword::OWNED, Keyword::BY]) {
- if self.parse_keywords(&[Keyword::NONE]) {
- Some(ObjectName::from(vec![Ident::new("NONE")]))
- } else {
- Some(self.parse_object_name(false)?)
- }
- } else {
- None
- };
+ let sequence_options = self.parse_sequence_options(true)?;
Ok(Statement::CreateSequence {
temporary,
if_not_exists,
name,
- data_type,
sequence_options,
- owned_by,
})
}
- 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));
+ /// Parse the sequence options shared by `CREATE SEQUENCE` and identity
+ /// columns. The options form an unordered list, each allowed at most once.
+ ///
+ /// `AS <data_type>` and `OWNED BY` are options of `CREATE SEQUENCE` only,
so
+ /// `allow_type_and_owner` gates them off for an identity column.
+ fn parse_sequence_options(
+ &mut self,
+ allow_type_and_owner: bool,
+ ) -> Result<Vec<SequenceOptions>, ParserError> {
+ let mut sequence_options: Vec<SequenceOptions> = vec![];
+ loop {
+ let (option, name) = if self.parse_keyword(Keyword::INCREMENT) {
+ //[ INCREMENT [ BY ] increment ]
+ let by = self.parse_keyword(Keyword::BY);
+ (
+ SequenceOptions::IncrementBy(self.parse_number()?, by),
+ "INCREMENT",
+ )
+ } else if self.parse_keyword(Keyword::MINVALUE) {
+ //[ MINVALUE minvalue | NO MINVALUE ]
+ (
+ SequenceOptions::MinValue(Some(self.parse_number()?)),
+ "MINVALUE | NO MINVALUE",
+ )
+ } else if self.parse_keywords(&[Keyword::NO, Keyword::MINVALUE]) {
+ (SequenceOptions::MinValue(None), "MINVALUE | NO MINVALUE")
+ } else if self.parse_keyword(Keyword::MAXVALUE) {
+ //[ MAXVALUE maxvalue | NO MAXVALUE ]
+ (
+ SequenceOptions::MaxValue(Some(self.parse_number()?)),
+ "MAXVALUE | NO MAXVALUE",
+ )
+ } else if self.parse_keywords(&[Keyword::NO, Keyword::MAXVALUE]) {
+ (SequenceOptions::MaxValue(None), "MAXVALUE | NO MAXVALUE")
+ } else if self.parse_keyword(Keyword::START) {
+ //[ START [ WITH ] start ]
+ let with = self.parse_keyword(Keyword::WITH);
+ (
+ SequenceOptions::StartWith(self.parse_number()?, with),
+ "START",
+ )
+ } else if self.parse_keyword(Keyword::CACHE) {
+ //[ CACHE cache ]
+ (SequenceOptions::Cache(self.parse_number()?), "CACHE")
+ } else if self.parse_keywords(&[Keyword::NO, Keyword::CYCLE]) {
+ // [ [ NO ] CYCLE ]
+ (SequenceOptions::Cycle(true), "CYCLE | NO CYCLE")
+ } else if self.parse_keyword(Keyword::CYCLE) {
+ (SequenceOptions::Cycle(false), "CYCLE | NO CYCLE")
+ } else if allow_type_and_owner && self.parse_keyword(Keyword::AS) {
+ //[ AS data_type ]
+ (SequenceOptions::DataType(self.parse_data_type()?), "AS")
+ } else if allow_type_and_owner &&
self.parse_keywords(&[Keyword::OWNED, Keyword::BY]) {
Review Comment:
PostgreSQL's grammar takes any qualified name after OWNED BY and only the
single bare word none means no owner.
https://www.postgresql.org/docs/current/sql-createsequence.html#SQL-CREATESEQUENCE-PARAMETERS
Pls fix and add test
`pg().verified_stmt("CREATE SEQUENCE s OWNED BY none.c")`
##########
tests/sqlparser_postgres.rs:
##########
@@ -10202,3 +10202,81 @@ fn parse_string_literal_alias_rejected() {
}
pg().verified_stmt(r#"SELECT a AS "x" FROM t AS "y""#);
}
+
Review Comment:
"CREATE SEQUENCE and the identity paths are not dialect-gated, so per
AGENTS.md pls add more tests to sqlparser_common.rs.
```
#[test]
fn parse_sequence_options_in_any_order() {
let d = all_dialects();
d.verified_stmt("CREATE SEQUENCE s START WITH 1 INCREMENT BY 1");
d.verified_stmt("CREATE TABLE t (id INT GENERATED ALWAYS AS IDENTITY (
START WITH 10 INCREMENT BY 1 CYCLE ))");
d.verified_stmt("ALTER TABLE t ALTER COLUMN id ADD GENERATED AS
IDENTITY ( CACHE 2 INCREMENT 1 )");
for (sql, err) in [
("CREATE SEQUENCE s INCREMENT 1 INCREMENT BY 2", "INCREMENT
specified more than once"),
("CREATE SEQUENCE s MAXVALUE 1 NO MAXVALUE", "MAXVALUE | NO
MAXVALUE specified more than once"),
("CREATE SEQUENCE s CACHE 1 CACHE 2", "CACHE specified more than
once"),
("CREATE TABLE t (id INT GENERATED ALWAYS AS IDENTITY (START 1
START 2))", "START specified more than once"),
("CREATE SEQUENCE s AS", "Expected: a data type name, found: EOF"),
("CREATE SEQUENCE s OWNED BY", "Expected: identifier, found: EOF"),
] {
assert_eq!(d.parse_sql_statements(sql).unwrap_err(),
ParserError::ParserError(err.to_string()));
}
// Identity columns take the shared list without AS / OWNED BY.
for sql in [
"CREATE TABLE t (id INT GENERATED ALWAYS AS IDENTITY (AS BIGINT))",
"CREATE TABLE t (id INT GENERATED ALWAYS AS IDENTITY (OWNED BY
t.id))",
] {
d.parse_sql_statements(sql).unwrap_err();
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
