Accept CREATE SEQUENCE options in any order

Author: LucaCappelletti94

src/ast/mod.rs

@@ -4787,12 +4787,8 @@ pub enum Statement {
         if_not_exists: bool,
         /// Sequence name.
         name: ObjectName,
-        /// Optional data type for the sequence.
-        data_type: Option<DataType>,
-        /// Sequence options (INCREMENT, MINVALUE, etc.).
+        /// Sequence options, in the order they were written.
         sequence_options: Vec<SequenceOptions>,
-        /// Optional `OWNED BY` target.
-        owned_by: Option<ObjectName>,
     },
     /// A `CREATE DOMAIN` statement.
     CreateDomain(CreateDomain),
@@ -6236,32 +6232,19 @@ impl fmt::Display for Statement {
                 temporary,
                 if_not_exists,
                 name,
-                data_type,
                 sequence_options,
-                owned_by,
             } => {
-                let as_type: String = if let Some(dt) = data_type.as_ref() {
-                    //Cannot use format!(" AS {}", dt), due to format! is not available in --target thumbv6m-none-eabi
-                    // " AS ".to_owned() + &dt.to_string()
-                    [" AS ", &dt.to_string()].concat()
-                } else {
-                    "".to_string()
-                };
                 write!(
                     f,
-                    "CREATE {temporary}SEQUENCE {if_not_exists}{name}{as_type}",
+                    "CREATE {temporary}SEQUENCE {if_not_exists}{name}",
                     if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" },
                     temporary = if *temporary { "TEMPORARY " } else { "" },
                     name = name,
-                    as_type = as_type
                 )?;
                 for sequence_option in sequence_options {
                     write!(f, "{sequence_option}")?;
                 }
-                if let Some(ob) = owned_by.as_ref() {
-                    write!(f, " OWNED BY {ob}")?;
-                }
-                write!(f, "")
+                Ok(())
             }
             Statement::CreateStage {
                 or_replace,
@@ -6547,14 +6530,21 @@ impl fmt::Display for Statement {
 
 /// Can use to describe options in create sequence or table column type identity
 /// ```sql
-/// [ INCREMENT [ BY ] increment ]
+/// [ AS data_type ] [ INCREMENT [ BY ] increment ]
 ///     [ MINVALUE minvalue | NO MINVALUE ] [ MAXVALUE maxvalue | NO MAXVALUE ]
 ///     [ START [ WITH ] start ] [ CACHE cache ] [ [ NO ] CYCLE ]
+///     [ OWNED BY { table_name.column_name | NONE } ]
 /// ```
+///
+/// The options form an unordered list, so they are stored in the order written.
+/// `AS` and `OWNED BY` are only accepted by `CREATE SEQUENCE`, not by an
+/// identity column.
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
 pub enum SequenceOptions {
+    /// `AS <data_type>` option.
+    DataType(DataType),
     /// `INCREMENT [BY] <expr>` option; second value indicates presence of `BY` keyword.
     IncrementBy(Expr, bool),
     /// `MINVALUE <expr>` or `NO MINVALUE`.
@@ -6567,11 +6557,16 @@ pub enum SequenceOptions {
     Cache(Expr),
     /// `CYCLE` or `NO CYCLE` option.
     Cycle(bool),
+    /// `OWNED BY <object_name>`, or `OWNED BY NONE` when no target is given.
+    OwnedBy(Option<ObjectName>),
 }
 
 impl fmt::Display for SequenceOptions {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
+            SequenceOptions::DataType(data_type) => {
+                write!(f, " AS {data_type}")
+            }
             SequenceOptions::IncrementBy(increment, by) => {
                 write!(
                     f,
@@ -6606,6 +6601,12 @@ impl fmt::Display for SequenceOptions {
             SequenceOptions::Cycle(no) => {
                 write!(f, " {}CYCLE", if *no { "NO " } else { "" })
             }
+            SequenceOptions::OwnedBy(Some(object_name)) => {
+                write!(f, " OWNED BY {object_name}")
+            }
+            SequenceOptions::OwnedBy(None) => {
+                write!(f, " OWNED BY NONE")
+            }
         }
     }
 }

src/parser/mod.rs

@@ -23,6 +23,7 @@ use alloc::{
 };
 use core::{
     fmt::{self, Display},
+    mem::discriminant,
     str::FromStr,
 };
 #[cfg(feature = "std")]
@@ -9829,7 +9830,7 @@ impl<'a> Parser<'a> {
         if self.parse_keywords(&[Keyword::ALWAYS, Keyword::AS, Keyword::IDENTITY]) {
             let mut sequence_options = vec![];
             if self.expect_token(&Token::LParen).is_ok() {
-                sequence_options = self.parse_create_sequence_options()?;
+                sequence_options = self.parse_sequence_options(false)?;
                 self.expect_token(&Token::RParen)?;
             }
             Ok(Some(ColumnOption::Generated {
@@ -9847,7 +9848,7 @@ impl<'a> Parser<'a> {
         ]) {
             let mut sequence_options = vec![];
             if self.expect_token(&Token::LParen).is_ok() {
-                sequence_options = self.parse_create_sequence_options()?;
+                sequence_options = self.parse_sequence_options(false)?;
                 self.expect_token(&Token::RParen)?;
             }
             Ok(Some(ColumnOption::Generated {
@@ -10985,7 +10986,7 @@ impl<'a> Parser<'a> {
 
                 if self.peek_token_ref().token == Token::LParen {
                     self.expect_token(&Token::LParen)?;
-                    sequence_options = Some(self.parse_create_sequence_options()?);
+                    sequence_options = Some(self.parse_sequence_options(false)?);
                     self.expect_token(&Token::RParen)?;
                 }
 
@@ -20310,72 +20311,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]) {
+                // [ OWNED BY { table_name.column_name | NONE } ]
+                let owner = if self.parse_keyword(Keyword::NONE) {
+                    None
+                } else {
+                    Some(self.parse_object_name(false)?)
+                };
+                (SequenceOptions::OwnedBy(owner), "OWNED BY")
             } 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));
-        }
+                break;
+            };
 
-        //[ 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));
+            if sequence_options
+                .iter()
+                .any(|seen| discriminant(seen) == discriminant(&option))
+            {
+                return Err(ParserError::ParserError(format!(
+                    "{name} specified more than once"
+                )));
             }
-        }
-        //[ CACHE cache ]
-        if self.parse_keywords(&[Keyword::CACHE]) {
-            sequence_options.push(SequenceOptions::Cache(self.parse_number()?));
-        }
-        // [ [ NO ] CYCLE ]
-        if self.parse_keywords(&[Keyword::NO, Keyword::CYCLE]) {
-            sequence_options.push(SequenceOptions::Cycle(true));
-        } else if self.parse_keywords(&[Keyword::CYCLE]) {
-            sequence_options.push(SequenceOptions::Cycle(false));
+            sequence_options.push(option);
         }
 
         Ok(sequence_options)

tests/sqlparser_postgres.rs

@@ -10022,3 +10022,81 @@ fn parse_pg_roots_render_apart_from_operand() {
     pg().verified_stmt("SELECT ||/ -2");
     pg().verified_stmt("SELECT |/ ||/ 2");
 }
+
+#[test]
+fn parse_create_sequence_options_in_any_order() {
+    // PostgreSQL treats the sequence options as an unordered list, so every
+    // permutation below is accepted and round-trips in the order written.
+    pg().verified_stmt("CREATE SEQUENCE sa START WITH 1 INCREMENT BY 1");
+    pg().verified_stmt("CREATE SEQUENCE sb INCREMENT BY 1 START WITH 1");
+    pg().verified_stmt("CREATE SEQUENCE sc CACHE 1 START WITH 1");
+    pg().verified_stmt("CREATE SEQUENCE sd CYCLE INCREMENT BY 2");
+    pg().verified_stmt("CREATE SEQUENCE se NO MAXVALUE NO MINVALUE NO CYCLE INCREMENT 1");
+    pg().verified_stmt(
+        "CREATE SEQUENCE sf AS INTEGER START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1",
+    );
+    // `AS` and `OWNED BY` are options too, so they interleave with the rest.
+    pg().verified_stmt("CREATE SEQUENCE sg START WITH 1 AS BIGINT");
+    pg().verified_stmt("CREATE SEQUENCE sh OWNED BY t.c START WITH 5");
+    pg().verified_stmt("CREATE SEQUENCE si OWNED BY NONE INCREMENT BY 2");
+    pg().verified_stmt("CREATE SEQUENCE sj CACHE 1 AS BIGINT OWNED BY a.b START WITH 3");
+
+    // This is what `pg_dump -s` emits for a table with a SERIAL column.
+    pg().one_statement_parses_to(
+        r#"CREATE SEQUENCE public.accounts_id_seq
+    AS integer
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1"#,
+        "CREATE SEQUENCE public.accounts_id_seq AS INTEGER START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1",
+    );
+
+    // The parsed options keep the order they were written in, and `OWNED BY NONE`
+    // is distinct from being owned by something called `NONE`.
+    let Statement::CreateSequence {
+        sequence_options, ..
+    } = pg().verified_stmt("CREATE SEQUENCE sk CACHE 1 OWNED BY NONE AS BIGINT INCREMENT BY 3")
+    else {
+        panic!("expected a CREATE SEQUENCE statement");
+    };
+    assert!(matches!(
+        sequence_options.as_slice(),
+        [
+            SequenceOptions::Cache(_),
+            SequenceOptions::OwnedBy(None),
+            SequenceOptions::DataType(DataType::BigInt(None)),
+            SequenceOptions::IncrementBy(_, true),
+        ]
+    ));
+
+    // Each option may still appear only once.
+    for (sql, expected) in [
+        (
+            "CREATE SEQUENCE s START WITH 1 START 2",
+            "START specified more than once",
+        ),
+        (
+            "CREATE SEQUENCE s MINVALUE 1 NO MINVALUE",
+            "MINVALUE | NO MINVALUE specified more than once",
+        ),
+        (
+            "CREATE SEQUENCE s CYCLE NO CYCLE",
+            "CYCLE | NO CYCLE specified more than once",
+        ),
+        (
+            "CREATE SEQUENCE s AS INT AS BIGINT",
+            "AS specified more than once",
+        ),
+        (
+            "CREATE SEQUENCE s OWNED BY a.b OWNED BY NONE",
+            "OWNED BY specified more than once",
+        ),
+    ] {
+        assert_eq!(
+            pg().parse_sql_statements(sql).unwrap_err(),
+            ParserError::ParserError(expected.to_string()),
+        );
+    }
+}