cloud-fan commented on a change in pull request #25502: [SPARK-28668][SQL]
Support V2SessionCatalog for ALTER TABLE
URL: https://github.com/apache/spark/pull/25502#discussion_r319366124
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -922,51 +916,51 @@ class Analyzer(
TableChange.updateColumnComment(colName.toArray, newComment)
}
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- typeChange.toSeq ++ commentChange.toSeq)
+ resolveV2Alter(tableName, typeChange.toSeq ++
commentChange.toSeq).getOrElse(alter)
- case alter @ AlterTableRenameColumnStatement(
- CatalogObjectIdentifier(Some(v2Catalog), ident), col, newName) =>
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- Seq(TableChange.renameColumn(col.toArray, newName)))
+ case alter @ AlterTableRenameColumnStatement(tableName, col, newName) =>
+ val changes = Seq(TableChange.renameColumn(col.toArray, newName))
+ resolveV2Alter(tableName, changes).getOrElse(alter)
- case alter @ AlterTableDropColumnsStatement(
- CatalogObjectIdentifier(Some(v2Catalog), ident), cols) =>
+ case alter @ AlterTableDropColumnsStatement(tableName, cols) =>
val changes = cols.map(col => TableChange.deleteColumn(col.toArray))
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- changes)
-
- case alter @ AlterTableSetPropertiesStatement(
- CatalogObjectIdentifier(Some(v2Catalog), ident), props) =>
- val changes = props.map {
- case (key, value) =>
- TableChange.setProperty(key, value)
+ resolveV2Alter(tableName, changes).getOrElse(alter)
+
+ case alter @ AlterTableSetPropertiesStatement(tableName, props) =>
+ val changes = props.map { case (key, value) =>
+ TableChange.setProperty(key, value)
}
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- changes.toSeq)
-
- case alter @ AlterTableUnsetPropertiesStatement(
- CatalogObjectIdentifier(Some(v2Catalog), ident), keys, _) =>
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- keys.map(key => TableChange.removeProperty(key)))
-
- case alter @ AlterTableSetLocationStatement(
- CatalogObjectIdentifier(Some(v2Catalog), ident), newLoc) =>
- AlterTable(
- v2Catalog.asTableCatalog, ident,
- UnresolvedRelation(alter.tableName),
- Seq(TableChange.setProperty("location", newLoc)))
+ resolveV2Alter(tableName, changes.toSeq).getOrElse(alter)
+
+ case alter @ AlterTableUnsetPropertiesStatement(tableName, keys, _) =>
+ resolveV2Alter(tableName, keys.map(key =>
TableChange.removeProperty(key))).getOrElse(alter)
+
+ case alter @ AlterTableSetLocationStatement(tableName, newLoc) =>
+ resolveV2Alter(tableName, Seq(TableChange.setProperty("location",
newLoc))).getOrElse(alter)
+ }
+
+ private def resolveV2Alter(
+ tableName: Seq[String],
+ changes: Seq[TableChange]): Option[AlterTable] = {
Review comment:
I also thought about it before. I think the ideal resolution process is:
1. rules like `ResolveAlterTable` are only responsible for converting
XYZStatement to v1 or v2 command
2. `ResolveTables` and `ResolveRelations` are responsible for resolving
`UnresolvedRelation` to v1 or v2 relations
However, some commands like ALTER TABLE also need to get the catalog
instance, which can't be done by `ResolveTables` or `ResolveRelations`. Unlike
table resolution which replaces `UnresolvedRelation` with v1/v2 relation and
can be done by a rule separately. Catalog resolution needs to be done during
the converting from XYZStatement to v1/v2 command and we can't do it in a
separated rule.
I don't have a good idea now but we should definitely revisit it later.
----------------------------------------------------------------
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]
