diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala index 04bf8c6dd917..bfdf5b92c212 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala @@ -32,7 +32,7 @@ import org.apache.spark.sql.catalyst.TableIdentifier import org.apache.spark.sql.catalyst.analysis.{NoSuchTableException, Resolver} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec -import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference} +import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference, Cast} import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan import org.apache.spark.sql.execution.datasources.{HadoopFsRelation, LogicalRelation, PartitioningUtils} import org.apache.spark.sql.execution.datasources.orc.OrcFileFormat @@ -309,7 +309,7 @@ case class AlterTableChangeColumnCommand( columnName: String, newColumn: StructField) extends RunnableCommand { - // TODO: support change column name/dataType/metadata/position. + // TODO: support change column name/metadata/position. override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog val table = catalog.getTableMetadata(tableName) @@ -318,18 +318,25 @@ case class AlterTableChangeColumnCommand( // Find the origin column from dataSchema by column name. val originColumn = findColumnByName(table.dataSchema, columnName, resolver) - // Throw an AnalysisException if the column name/dataType is changed. - if (!columnEqual(originColumn, newColumn, resolver)) { + // Throw an AnalysisException if the column name is changed or type change is incompatible. + if (!columnCheck(originColumn, newColumn, resolver)) { throw new AnalysisException( "ALTER TABLE CHANGE COLUMN is not supported for changing column " + s"'${originColumn.name}' with type '${originColumn.dataType}' to " + s"'${newColumn.name}' with type '${newColumn.dataType}'") } + val typeChanged = originColumn.dataType != newColumn.dataType + val newDataSchema = table.dataSchema.fields.map { field => if (field.name == originColumn.name) { - // Create a new column from the origin column with the new comment. - addComment(field, newColumn.getComment) + // Add the comment to a column, if comment is empty, return the original column. + val newField = newColumn.getComment().map(field.withComment).getOrElse(field) + if (typeChanged) { + newField.copy(dataType = newColumn.dataType) + } else { + newField + } } else { field } @@ -350,16 +357,11 @@ case class AlterTableChangeColumnCommand( s"${schema.fieldNames.mkString("[`", "`, `", "`]")}")) } - // Add the comment to a column, if comment is empty, return the original column. - private def addComment(column: StructField, comment: Option[String]): StructField = { - comment.map(column.withComment(_)).getOrElse(column) - } - // Compare a [[StructField]] to another, return true if they have the same column - // name(by resolver) and dataType. - private def columnEqual( + // name(by resolver) and dataType and data type compatible. + private def columnCheck( field: StructField, other: StructField, resolver: Resolver): Boolean = { - resolver(field.name, other.name) && field.dataType == other.dataType + resolver(field.name, other.name) && Cast.canCast(field.dataType, other.dataType) } } diff --git a/sql/core/src/test/resources/sql-tests/inputs/change-column.sql b/sql/core/src/test/resources/sql-tests/inputs/change-column.sql index 2909024e4c9f..dd8d82a1a5f2 100644 --- a/sql/core/src/test/resources/sql-tests/inputs/change-column.sql +++ b/sql/core/src/test/resources/sql-tests/inputs/change-column.sql @@ -6,7 +6,7 @@ DESC test_change; ALTER TABLE test_change CHANGE a a1 INT; DESC test_change; --- Change column dataType (not supported yet) +-- Change column dataType ALTER TABLE test_change CHANGE a a STRING; DESC test_change; diff --git a/sql/core/src/test/resources/sql-tests/results/change-column.sql.out b/sql/core/src/test/resources/sql-tests/results/change-column.sql.out index ff1ecbcc44c2..b7f164eb86be 100644 --- a/sql/core/src/test/resources/sql-tests/results/change-column.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/change-column.sql.out @@ -44,8 +44,7 @@ ALTER TABLE test_change CHANGE a a STRING -- !query 4 schema struct<> -- !query 4 output -org.apache.spark.sql.AnalysisException -ALTER TABLE CHANGE COLUMN is not supported for changing column 'a' with type 'IntegerType' to 'a' with type 'StringType'; + -- !query 5 @@ -53,7 +52,7 @@ DESC test_change -- !query 5 schema struct -- !query 5 output -a int +a string b string c int @@ -91,7 +90,7 @@ DESC test_change -- !query 8 schema struct -- !query 8 output -a int +a string b string c int diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index ca95aad3976e..245ee4f2d7fb 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -1697,6 +1697,27 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { sql("ALTER TABLE dbx.tab1 CHANGE COLUMN col1 col1 INT COMMENT 'this is col1'") assert(getMetadata("col1").getString("key") == "value") assert(getMetadata("col1").getString("comment") == "this is col1") + + // Ensure that changing column type takes effect + sql("ALTER TABLE dbx.tab1 CHANGE COLUMN col1 col1 STRING") + val column = catalog.getTableMetadata(tableIdent).schema.fields.find(_.name == "col1") + assert(column.get.dataType == StringType) + + // Ensure that changing partition column type throw exception + var msg = intercept[AnalysisException] { + sql("ALTER TABLE dbx.tab1 CHANGE COLUMN a a STRING") + } + assert(msg.getMessage.startsWith( + "Can't find column `a` given table data columns")) + + withTable("t") { + sql("CREATE TABLE t(s STRUCT) USING PARQUET") + msg = intercept[AnalysisException]{ + sql("ALTER TABLE t CHANGE COLUMN s s INT") + } + assert(msg.getMessage.startsWith( + "ALTER TABLE CHANGE COLUMN is not supported for changing column ")) + } } test("drop build-in function") {