-
Notifications
You must be signed in to change notification settings - Fork 583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for pg ADD GENERATED in ALTER COLUMN statements #1079
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -552,6 +552,7 @@ define_keywords!( | |
REPLICATION, | ||
RESET, | ||
RESPECT, | ||
RESTART, | ||
RESTRICT, | ||
RESULT, | ||
RETAIN, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4841,7 +4841,11 @@ impl<'a> Parser<'a> { | |
let column_name = self.parse_identifier()?; | ||
let is_postgresql = dialect_of!(self is PostgreSqlDialect); | ||
|
||
let op = if self.parse_keywords(&[Keyword::SET, Keyword::NOT, Keyword::NULL]) { | ||
let op: AlterColumnOperation = if self.parse_keywords(&[ | ||
Keyword::SET, | ||
Keyword::NOT, | ||
Keyword::NULL, | ||
]) { | ||
AlterColumnOperation::SetNotNull {} | ||
} else if self.parse_keywords(&[Keyword::DROP, Keyword::NOT, Keyword::NULL]) { | ||
AlterColumnOperation::DropNotNull {} | ||
|
@@ -4861,11 +4865,37 @@ impl<'a> Parser<'a> { | |
None | ||
}; | ||
AlterColumnOperation::SetDataType { data_type, using } | ||
} else if self.parse_keywords(&[Keyword::ADD, Keyword::GENERATED]) { | ||
let generated_as = if self.parse_keyword(Keyword::ALWAYS) { | ||
Some(GeneratedAs::Always) | ||
} else if self.parse_keywords(&[Keyword::BY, Keyword::DEFAULT]) { | ||
Some(GeneratedAs::ByDefault) | ||
} else { | ||
None | ||
}; | ||
|
||
let _ = self.expect_keywords(&[Keyword::AS, Keyword::IDENTITY]); | ||
|
||
let mut sequence_options: Option<Vec<SequenceOptions>> = None; | ||
|
||
if self.peek_token().token == Token::LParen { | ||
self.expect_token(&Token::LParen)?; | ||
sequence_options = self.parse_create_sequence_options().ok(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise here, I don't understand why the error is ignored. If there is a reason maybe we can add a comment. I also wonder if we are sure that if |
||
self.expect_token(&Token::RParen)?; | ||
} | ||
|
||
AlterColumnOperation::AddGenerated { | ||
generated_as, | ||
sequence_options, | ||
} | ||
} else { | ||
return self.expected( | ||
"SET/DROP NOT NULL, SET DEFAULT, SET DATA TYPE after ALTER COLUMN", | ||
self.peek_token(), | ||
); | ||
let message = if is_postgresql { | ||
"SET/DROP NOT NULL, SET DEFAULT, SET DATA TYPE, or ADD GENERATED after ALTER COLUMN" | ||
} else { | ||
"SET/DROP NOT NULL, SET DEFAULT, or SET DATA TYPE after ALTER COLUMN" | ||
}; | ||
|
||
return self.expected(message, self.peek_token()); | ||
}; | ||
AlterTableOperation::AlterColumn { column_name, op } | ||
} else if self.parse_keyword(Keyword::SWAP) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -611,6 +611,20 @@ fn parse_alter_table_alter_column() { | |
} | ||
} | ||
|
||
#[test] | ||
fn parse_alter_table_alter_column_add_generated() { | ||
pg_and_generic() | ||
.verified_stmt("ALTER TABLE t ALTER COLUMN id ADD GENERATED ALWAYS AS IDENTITY"); | ||
pg_and_generic() | ||
.verified_stmt("ALTER TABLE t ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY"); | ||
pg_and_generic().verified_stmt("ALTER TABLE t ALTER COLUMN id ADD GENERATED AS IDENTITY"); | ||
pg_and_generic().verified_stmt( | ||
"ALTER TABLE t ALTER COLUMN id ADD GENERATED AS IDENTITY ( INCREMENT 1 MINVALUE 1 )", | ||
); | ||
|
||
pg_and_generic().verified_stmt("ALTER TABLE t ALTER COLUMN id ADD GENERATED AS IDENTITY ( )"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given there is some seemingly non trival error handling, can you please add some tests that hit the errors (e.g. when the two statements I identified above actually return an error?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On it |
||
|
||
#[test] | ||
fn parse_alter_table_add_columns() { | ||
match pg().verified_stmt("ALTER TABLE IF EXISTS ONLY tab ADD COLUMN a TEXT, ADD COLUMN b INT") { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this ignoring the result? shouldn't it at least check and return the error, like: