cloud-fan commented on a change in pull request #27243: [SPARK-30535][SQL]
Migrate ALTER TABLE commands to the new framework
URL: https://github.com/apache/spark/pull/27243#discussion_r367776230
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -47,141 +47,119 @@ class ResolveSessionCatalog(
import org.apache.spark.sql.connector.catalog.CatalogV2Util._
override def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp
{
- case AlterTableAddColumnsStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), cols) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- cols.foreach { c =>
- assertTopLevelColumn(c.name, "AlterTableAddColumnsCommand")
- if (!c.nullable) {
- throw new AnalysisException(
- "ADD COLUMN with v1 tables cannot specify NOT NULL.")
- }
- }
- AlterTableAddColumnsCommand(tbl.asTableIdentifier,
cols.map(convertToStructField))
- }.getOrElse {
- val changes = cols.map { col =>
- TableChange.addColumn(
- col.name.toArray,
- col.dataType,
- col.nullable,
- col.comment.orNull,
- col.position.orNull)
+ case AlterTableAddColumns(ResolvedTable(_, ident, _: V1Table), cols) =>
+ cols.foreach { c =>
+ assertTopLevelColumn(c.name, "AlterTableAddColumnsCommand")
+ if (!c.nullable) {
+ throw new AnalysisException(
+ "ADD COLUMN with v1 tables cannot specify NOT NULL.")
}
- createAlterTable(nameParts, catalog, tbl, changes)
}
-
- case a @ AlterTableAlterColumnStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), _, _, _, _, _) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- if (a.column.length > 1) {
- throw new AnalysisException(
- "ALTER COLUMN with qualified column is only supported with v2
tables.")
- }
- if (a.dataType.isEmpty) {
- throw new AnalysisException(
- "ALTER COLUMN with v1 tables must specify new data type.")
- }
- if (a.nullable.isDefined) {
- throw new AnalysisException(
- "ALTER COLUMN with v1 tables cannot specify NOT NULL.")
- }
- if (a.position.isDefined) {
- throw new AnalysisException("" +
- "ALTER COLUMN ... FIRST | ALTER is only supported with v2
tables.")
- }
- val builder = new MetadataBuilder
- // Add comment to metadata
- a.comment.map(c => builder.putString("comment", c))
- // Add Hive type string to metadata.
- val cleanedDataType = HiveStringType.replaceCharType(a.dataType.get)
- if (a.dataType.get != cleanedDataType) {
- builder.putString(HIVE_TYPE_STRING, a.dataType.get.catalogString)
- }
- val newColumn = StructField(
- a.column(0),
- cleanedDataType,
- nullable = true,
- builder.build())
- AlterTableChangeColumnCommand(tbl.asTableIdentifier, a.column(0),
newColumn)
- }.getOrElse {
- val colName = a.column.toArray
- val typeChange = a.dataType.map { newDataType =>
- TableChange.updateColumnType(colName, newDataType)
- }
- val nullabilityChange = a.nullable.map { nullable =>
- TableChange.updateColumnNullability(colName, nullable)
- }
- val commentChange = a.comment.map { newComment =>
- TableChange.updateColumnComment(colName, newComment)
- }
- val positionChange = a.position.map { newPosition =>
- TableChange.updateColumnPosition(colName, newPosition)
- }
- createAlterTable(
- nameParts,
- catalog,
- tbl,
- typeChange.toSeq ++ nullabilityChange ++ commentChange ++
positionChange)
+ AlterTableAddColumnsCommand(ident.asTableIdentifier,
cols.map(convertToStructField))
+
+ case AlterTableAddColumns(r: ResolvedTable, cols) =>
+ val changes = cols.map { col =>
+ TableChange.addColumn(
+ col.name.toArray,
+ col.dataType,
+ col.nullable,
+ col.comment.orNull,
+ col.position.orNull)
}
+ AlterTable(r, changes)
- case AlterTableRenameColumnStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), col, newName) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- throw new AnalysisException("RENAME COLUMN is only supported with v2
tables.")
- }.getOrElse {
- val changes = Seq(TableChange.renameColumn(col.toArray, newName))
- createAlterTable(nameParts, catalog, tbl, changes)
+ case a @ AlterTableAlterColumn(ResolvedTable(_, ident, _: V1Table), _, _,
_, _, _) =>
+ if (a.column.length > 1) {
+ throw new AnalysisException(
+ "ALTER COLUMN with qualified column is only supported with v2
tables.")
}
-
- case AlterTableDropColumnsStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), cols) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- throw new AnalysisException("DROP COLUMN is only supported with v2
tables.")
- }.getOrElse {
- val changes = cols.map(col => TableChange.deleteColumn(col.toArray))
- createAlterTable(nameParts, catalog, tbl, changes)
+ if (a.dataType.isEmpty) {
+ throw new AnalysisException(
+ "ALTER COLUMN with v1 tables must specify new data type.")
}
-
- case AlterTableSetPropertiesStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), props) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- AlterTableSetPropertiesCommand(tbl.asTableIdentifier, props, isView
= false)
- }.getOrElse {
- val changes = props.map { case (key, value) =>
- TableChange.setProperty(key, value)
- }.toSeq
- createAlterTable(nameParts, catalog, tbl, changes)
+ if (a.nullable.isDefined) {
+ throw new AnalysisException(
+ "ALTER COLUMN with v1 tables cannot specify NOT NULL.")
}
-
- case AlterTableUnsetPropertiesStatement(
- nameParts @ SessionCatalogAndTable(catalog, tbl), keys, ifExists) =>
- loadTable(catalog, tbl.asIdentifier).collect {
- case v1Table: V1Table =>
- AlterTableUnsetPropertiesCommand(
- tbl.asTableIdentifier, keys, ifExists, isView = false)
- }.getOrElse {
- val changes = keys.map(key => TableChange.removeProperty(key))
- createAlterTable(nameParts, catalog, tbl, changes)
+ if (a.position.isDefined) {
+ throw new AnalysisException("" +
+ "ALTER COLUMN ... FIRST | ALTER is only supported with v2 tables.")
+ }
+ val builder = new MetadataBuilder
+ // Add comment to metadata
+ a.comment.map(c => builder.putString("comment", c))
+ // Add Hive type string to metadata.
+ val cleanedDataType = HiveStringType.replaceCharType(a.dataType.get)
+ if (a.dataType.get != cleanedDataType) {
+ builder.putString(HIVE_TYPE_STRING, a.dataType.get.catalogString)
+ }
+ val newColumn = StructField(
+ a.column(0),
+ cleanedDataType,
+ nullable = true,
+ builder.build())
+ AlterTableChangeColumnCommand(ident.asTableIdentifier, a.column(0),
newColumn)
+
+ case a @ AlterTableAlterColumn(r: ResolvedTable, _, _, _, _, _) =>
+ val colName = a.column.toArray
+ val typeChange = a.dataType.map { newDataType =>
+ TableChange.updateColumnType(colName, newDataType)
}
+ val nullabilityChange = a.nullable.map { nullable =>
+ TableChange.updateColumnNullability(colName, nullable)
+ }
+ val commentChange = a.comment.map { newComment =>
+ TableChange.updateColumnComment(colName, newComment)
+ }
+ val positionChange = a.position.map { newPosition =>
+ TableChange.updateColumnPosition(colName, newPosition)
+ }
+ AlterTable(
+ r,
+ typeChange.toSeq ++ nullabilityChange ++ commentChange ++
positionChange)
Review comment:
yea sounds good! I think we can even make the logical plan `AlterTable` a
base class for these ALTER TABLE commands, to make the workflow simpler:
"different AlterTable logical plans -> AlterTableExec" instead of "different
AlterTable logical plans -> AlterTable -> AlterTableExec"
----------------------------------------------------------------
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]
