LucaCappelletti94 commented on code in PR #2449:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2449#discussion_r4324258532
##########
src/parser/mod.rs:
##########
@@ -11506,7 +11561,18 @@ impl<'a> Parser<'a> {
let only = self.parse_keyword(Keyword::ONLY); // [ ONLY ]
let table_name = self.parse_object_name(false)?;
let on_cluster = self.parse_optional_on_cluster()?;
- let operations =
self.parse_comma_separated(Parser::parse_alter_table_operation)?;
+ let mut operations = vec![];
+ loop {
+ let operation = if
self.can_parse_implicit_alter_table_add_item(&operations) {
+ self.parse_alter_table_add_item()?
Review Comment:
Would preserving the shared `ADD` clause in the AST be better here?
On `dev/mssql-alter-table-add-columns` at `2f1a810`, `ALTER TABLE t ADD a
INT, b INT` renders as `ALTER TABLE t ADD a INT, ADD b INT`, which violates
[SQL Server's
grammar](https://learn.microsoft.com/en-us/sql/t-sql/statements/alter-table-transact-sql?view=sql-server-ver17#syntax-for-disk-based-tables).
Constraints acquire the same extra `ADD`, and reparsing with `sqlparser`
accepts that invalid output.
##########
src/parser/mod.rs:
##########
@@ -11172,6 +11124,109 @@ impl<'a> Parser<'a> {
Ok(operation)
}
+ fn parse_alter_table_add_operation(&mut self) ->
Result<AlterTableOperation, ParserError> {
+ if let Some(constraint) = self.parse_optional_table_constraint()? {
+ let not_valid = self.parse_keywords(&[Keyword::NOT,
Keyword::VALID]);
+ return Ok(AlterTableOperation::AddConstraint {
+ constraint,
+ not_valid,
+ });
+ }
+
+ if dialect_of!(self is ClickHouseDialect | GenericDialect)
+ && self.parse_keyword(Keyword::PROJECTION)
+ {
+ return self.parse_alter_table_add_projection();
+ }
+
+ let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT,
Keyword::EXISTS]);
+ let mut new_partitions = vec![];
+ while self.parse_keyword(Keyword::PARTITION) {
+ new_partitions.push(self.parse_partition()?);
+ }
+ if !new_partitions.is_empty() {
+ return Ok(AlterTableOperation::AddPartitions {
+ if_not_exists,
+ new_partitions,
+ });
+ }
+
+ self.parse_alter_table_add_column(if_not_exists)
+ }
+
+ fn parse_alter_table_add_column(
+ &mut self,
+ if_not_exists: bool,
+ ) -> Result<AlterTableOperation, ParserError> {
+ let column_keyword = self.parse_keyword(Keyword::COLUMN);
+ let if_not_exists = if dialect_of!(self is PostgreSqlDialect |
BigQueryDialect | DuckDbDialect | GenericDialect)
+ {
+ self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS])
|| if_not_exists
+ } else {
+ false
+ };
+ let column_def = self.parse_column_def()?;
+ let column_position = self.parse_column_position()?;
+
+ Ok(AlterTableOperation::AddColumn {
+ column_keyword,
+ if_not_exists,
+ column_def,
+ column_position,
+ })
+ }
+
+ fn parse_alter_table_add_item(&mut self) -> Result<AlterTableOperation,
ParserError> {
+ if let Some(constraint) = self.parse_optional_table_constraint()? {
+ let not_valid = self.parse_keywords(&[Keyword::NOT,
Keyword::VALID]);
+ return Ok(AlterTableOperation::AddConstraint {
+ constraint,
+ not_valid,
+ });
+ }
+
+ self.parse_alter_table_add_column(false)
+ }
+
+ fn can_parse_implicit_alter_table_add_item(&self, operations:
&[AlterTableOperation]) -> bool {
+ self.dialect.supports_alter_table_add_multiple_columns()
+ && matches!(
+ operations.last(),
+ Some(
+ AlterTableOperation::AddColumn { .. }
+ | AlterTableOperation::AddConstraint { .. }
+ )
+ )
+ && !self.is_alter_table_operation_starter()
+ }
+
+ fn is_alter_table_operation_starter(&self) -> bool {
+ if self
+ .peek_one_of_keywords(&[
+ Keyword::ADD,
+ Keyword::ALTER,
+ Keyword::DISABLE,
+ Keyword::DROP,
+ Keyword::ENABLE,
+ Keyword::PARTITION,
+ Keyword::RENAME,
+ Keyword::REPLICA,
+ Keyword::SET,
+ Keyword::SWAP,
+ ])
+ .is_some()
+ {
+ return true;
+ }
+
+ matches!(
+ &self.peek_token_ref().token,
+ Token::Word(word)
+ if word.quote_style.is_none()
+ && matches!(word.value.to_ascii_uppercase().as_str(),
"NOCHECK" | "SWITCH")
+ )
+ }
Review Comment:
You could recognize complete operation prefixes so nonreserved column names
remain usable in the new list syntax. On `dev/mssql-alter-table-add-columns` at
`2f1a810`, `ALTER TABLE t ADD a INT, replica INT` fails at `replica`, and
`swap`, `rename`, `disable`, and `enable` fail similarly, although each
single-column form parses.
```suggestion
fn is_alter_table_operation_starter(&self) -> bool {
let keyword = |n| match &self.peek_nth_token_ref(n).token {
Token::Word(word) if word.quote_style.is_none() => word.keyword,
_ => Keyword::NoKeyword,
};
match keyword(0) {
Keyword::ADD | Keyword::ALTER | Keyword::DROP | Keyword::SET =>
true,
Keyword::DISABLE => {
matches!(keyword(1), Keyword::ROW | Keyword::RULE |
Keyword::TRIGGER)
}
Keyword::ENABLE => matches!(
keyword(1),
Keyword::ALWAYS
| Keyword::ROW
| Keyword::REPLICA
| Keyword::RULE
| Keyword::TRIGGER
),
Keyword::PARTITION => true,
Keyword::RENAME => {
matches!(keyword(1), Keyword::CONSTRAINT | Keyword::TO |
Keyword::AS)
|| keyword(2) == Keyword::TO
|| (keyword(1) == Keyword::COLUMN && keyword(3) ==
Keyword::TO)
}
Keyword::REPLICA => keyword(1) == Keyword::IDENTITY,
Keyword::SWAP => keyword(1) == Keyword::WITH,
_ => false,
}
}
```
--
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]
