brkyvz commented on a change in pull request #24937: [SPARK-28139][SQL] Add v2
ALTER TABLE implementation.
URL: https://github.com/apache/spark/pull/24937#discussion_r302686124
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -787,6 +789,86 @@ class Analyzer(
}
}
+ /**
+ * Resolve ALTER TABLE statements that use a DSv2 catalog.
+ *
+ * This rule converts unresolved ALTER TABLE statements to v2 when a v2
catalog is responsible
+ * for the table identifier. A v2 catalog is responsible for an identifier
when the identifier
+ * has a catalog specified, like prod_catalog.db.table, or when a default v2
catalog is set and
+ * the table identifier does not include a catalog.
+ */
+ object ResolveAlterTable extends Rule[LogicalPlan] {
+ import org.apache.spark.sql.catalog.v2.CatalogV2Implicits._
+ override def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators
{
+ case alter @ AlterTableAddColumnsStatement(
+ CatalogObjectIdentifier(Some(v2Catalog), ident), cols) =>
+ val changes = cols.map { col =>
+ TableChange.addColumn(col.name.toArray, col.dataType, true,
col.comment.orNull)
+ }
+
+ AlterTable(
+ v2Catalog.asTableCatalog, ident,
+ UnresolvedRelation(alter.tableName),
+ changes)
+
+ case alter @ AlterTableAlterColumnStatement(
+ CatalogObjectIdentifier(Some(v2Catalog), ident), colName, dataType,
comment) =>
+ val typeChange = dataType.map { newDataType =>
+ TableChange.updateColumnType(colName.toArray, newDataType, true)
+ }
+
+ val commentChange = comment.map { newComment =>
+ TableChange.updateColumnComment(colName.toArray, newComment)
+ }
+
+ AlterTable(
+ v2Catalog.asTableCatalog, ident,
+ UnresolvedRelation(alter.tableName),
+ typeChange.toSeq ++ commentChange.toSeq)
+
+ case alter @ AlterTableRenameColumnStatement(
+ CatalogObjectIdentifier(Some(v2Catalog), ident), col, newName) =>
+ AlterTable(
+ v2Catalog.asTableCatalog, ident,
+ UnresolvedRelation(alter.tableName),
+ Seq(TableChange.renameColumn(col.toArray, newName)))
+
+ case alter @ AlterTableDropColumnsStatement(
+ CatalogObjectIdentifier(Some(v2Catalog), ident), 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)
+ }
+
+ 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)))
Review comment:
Hmm, I feel changing the location of a table deserves its own special
`TableChange` operator, like `ChangeLocation` rather than a simple
`setProperty`. When I think property, I think `TBLPROPERTIES`, and the location
is separate from that
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
