rdblue commented on a change in pull request #24723: [SPARK-27857][SQL] Move
ALTER TABLE parsing into Catalyst
URL: https://github.com/apache/spark/pull/24723#discussion_r290469086
##########
File path:
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -113,14 +113,27 @@ statement
LIKE source=tableIdentifier locationSpec?
#createTableLike
| ANALYZE TABLE tableIdentifier partitionSpec? COMPUTE STATISTICS
(identifier | FOR COLUMNS identifierSeq | FOR ALL COLUMNS)? #analyze
- | ALTER TABLE tableIdentifier
- ADD COLUMNS '(' columns=colTypeList ')'
#addTableColumns
+ | ALTER TABLE multipartIdentifier
+ ADD (COLUMN | COLUMNS)
+ columns=qualifiedColTypeWithPositionList
#addTableColumns
+ | ALTER TABLE multipartIdentifier
+ ADD (COLUMN | COLUMNS)
+ '(' columns=qualifiedColTypeWithPositionList ')'
#addTableColumns
+ | ALTER TABLE multipartIdentifier
+ RENAME COLUMN from=qualifiedName TO to=identifier
#renameTableColumn
+ | ALTER TABLE multipartIdentifier
+ DROP (COLUMN | COLUMNS) '(' columns=qualifiedNameList ')'
#dropTableColumns
+ | ALTER TABLE multipartIdentifier
+ DROP (COLUMN | COLUMNS) columns=qualifiedNameList
#dropTableColumns
| ALTER (TABLE | VIEW) from=tableIdentifier
RENAME TO to=tableIdentifier
#renameTable
- | ALTER (TABLE | VIEW) tableIdentifier
+ | ALTER (TABLE | VIEW) multipartIdentifier
SET TBLPROPERTIES tablePropertyList
#setTableProperties
- | ALTER (TABLE | VIEW) tableIdentifier
+ | ALTER (TABLE | VIEW) multipartIdentifier
UNSET TBLPROPERTIES (IF EXISTS)? tablePropertyList
#unsetTableProperties
+ | ALTER TABLE multipartIdentifier
+ (ALTER | CHANGE) COLUMN? qualifiedName
+ (TYPE dataType)? (COMMENT comment=STRING)? colPosition?
#alterTableColumn
| ALTER TABLE tableIdentifier partitionSpec?
CHANGE COLUMN? identifier colType colPosition?
#changeColumn
Review comment:
I think we need to discuss deprecating this form of the command, which is
why I haven't updated it.
The problem is that this requires the user to specify too much of a column's
metadata when it isn't changing. For example, if I'm updating an int to a long,
I also need to specify that it should have the same name (`UPDATE a a BIGINT`).
Similarly, to rename you have to pass the type back in (`UPDATE a b INT`). This
can easily lead to unintended changes that can't be reverted, like widening a
type accidentally.
I think that this form of the command should not be supported in v2. We can
decide that later because all this is doing is updating the parser to add
commands that we need to support.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
