Repository navigation
Accept CREATE SEQUENCE options in any order - #2414
LucaCappelletti94 wants to merge 2 commits into
Conversation
CREATE SEQUENCE options in any order
f0ba878 to
b5f4edb
Compare
b5f4edb to
fb40c1a
Compare
2f43c43 to
15c849c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2414 +/- ##
==========================================
+ Coverage 81.28% 81.33% +0.05%
==========================================
Files 42 42
Lines 33920 33952 +32
Branches 33920 33952 +32
==========================================
+ Hits 27571 27615 +44
+ Misses 2785 2783 -2
+ Partials 3564 3554 -10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4d8634f to
b296c52
Compare
b296c52 to
ff3cc30
Compare
ff3cc30 to
1bde6d1
Compare
s5dsn-eqee
left a comment
There was a problem hiding this comment.
Great idea, but it needs some minor changes
| } | ||
| pg().verified_stmt(r#"SELECT a AS "x" FROM t AS "y""#); | ||
| } | ||
|
|
There was a problem hiding this 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();
}
}
| } 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]) { |
There was a problem hiding this 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")
There was a problem hiding this comment.
Added as a red test and fixed
CREATE SEQUENCEoptions were parsed by a fixed chain of independent if blocks, one per option, so only a single order was accepted.CREATE SEQUENCE s INCREMENT BY 1 START WITH 1parsed, butCREATE SEQUENCE s START WITH 1 INCREMENT BY 1left theINCREMENTkeyword unconsumed and failed withExpected: end of statement. PostgreSQL treats these as an unordered list.Therefore, following the same pattern we used in other methods,
parse_create_sequence_optionsnow loops, dispatching on whichever option keyword comes next and breaking when none matches, and rejects repeated options. Options are recorded in the order written, so Display round-trips the input unchanged.