You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ku...@apache.org on 2020/09/29 12:09:41 UTC

[carbondata] branch master updated: [CARBONDATA-4014] Support Change Column Comment

This is an automated email from the ASF dual-hosted git repository.

kunalkapoor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/carbondata.git


The following commit(s) were added to refs/heads/master by this push:
     new 5e81796  [CARBONDATA-4014] Support Change Column Comment
5e81796 is described below

commit 5e817967941eb6bed0432835f317549dd458647b
Author: haomarch <ma...@126.com>
AuthorDate: Mon Sep 28 15:04:20 2020 +0800

    [CARBONDATA-4014] Support Change Column Comment
    
    Why is this PR needed?
    Now, we support add column comment in "CREATE TABLE" and
    "ADD COLUMN". but do not support alter comment of the column,
    which shall be support in 'CHANGE COLUMN'
    
    What changes were proposed in this PR?
    Support "ALTER TABLE table_name CHANGE [COLUMN] col_name
    col_name data_type [COMMENT col_comment]"
    
    Does this PR introduce any user interface change?
    Yes. add CHANGE COLUMN explaination in the ddl document.
    
    Is any new testcase added?
    Yes
    
    This closes #3960
---
 docs/ddl-of-carbondata.md                          |  22 ++--
 .../spark/sql/catalyst/CarbonParserUtil.scala      |   9 +-
 .../command/carbonTableSchemaCommon.scala          |   3 +-
 ...nAlterTableColRenameDataTypeChangeCommand.scala |  99 ++++++++---------
 .../spark/sql/execution/strategy/DDLHelper.scala   |  17 ++-
 .../apache/spark/sql/hive/CarbonSessionUtil.scala  |   7 +-
 .../spark/sql/hive/SqlAstBuilderHelper.scala       |   2 +-
 .../sql/parser/CarbonSparkSqlParserUtil.scala      |   3 +-
 .../StandardPartitionTableQueryTestCase.scala      |   8 +-
 .../restructure/AlterTableValidationTestCase.scala |   2 +-
 .../AlterTableColumnRenameTestCase.scala           | 118 ++++++++++++++++++---
 11 files changed, 200 insertions(+), 90 deletions(-)

diff --git a/docs/ddl-of-carbondata.md b/docs/ddl-of-carbondata.md
index 228d9e7..ca9a321 100644
--- a/docs/ddl-of-carbondata.md
+++ b/docs/ddl-of-carbondata.md
@@ -49,7 +49,7 @@ CarbonData DDL statements are documented here,which includes:
     * [ADD COLUMNS](#add-columns)
     * [DROP COLUMNS](#drop-columns)
     * [RENAME COLUMN](#change-column-nametype)
-    * [CHANGE COLUMN NAME/TYPE](#change-column-nametype)
+    * [CHANGE COLUMN NAME/TYPE/COMMENT](#change-column-nametype)
     * [MERGE INDEXES](#merge-index)
     * [SET/UNSET](#set-and-unset)
   * [DROP TABLE](#drop-table)
@@ -719,18 +719,23 @@ Users can specify which columns to include and exclude for local dictionary gene
 
      **NOTE:** Drop Complex child column is not supported.
 
-   - #### CHANGE COLUMN NAME/TYPE
+   - #### CHANGE COLUMN NAME/TYPE/COMMENT
    
-     This command is used to change column name and the data type from INT to BIGINT or decimal precision from lower to higher.
+     This command is used to change column name and comment and the data type from INT to BIGINT or decimal precision from lower to higher.
      Change of decimal data type from lower precision to higher precision will only be supported for cases where there is no data loss.
+     Change of comment will only be supported for columns other than the partition column
 
      ```
-     ALTER TABLE [db_name.]table_name CHANGE col_old_name col_new_name column_type
+     ALTER TABLE [db_name.]table_name CHANGE col_old_name col_new_name column_type [COMMENT 'col_comment']
      ```
 
      Valid Scenarios
-     - Invalid scenario - Change of decimal precision from (10,2) to (10,5) is invalid as in this case only scale is increased but total number of digits remains the same.
-     - Valid scenario - Change of decimal precision from (10,2) to (12,3) is valid as the total number of digits are increased by 2 but scale is increased only by 1 which will not lead to any data loss.
+     - Invalid scenarios 
+       * Change of decimal precision from (10,2) to (10,5) is invalid as in this case only scale is increased but total number of digits remains the same.
+       * Change the comment of the partition column
+     - Valid scenarios
+       * Change of decimal precision from (10,2) to (12,3) is valid as the total number of digits are increased by 2 but scale is increased only by 1 which will not lead to any data loss.
+       * Change the comment of columns other than partition column
      - **NOTE:** The allowed range is 38,38 (precision, scale) and is a valid upper case scenario which is not resulting in data loss.
 
      Example1:Change column a1's name to a2 and its data type from INT to BIGINT.
@@ -750,6 +755,11 @@ Users can specify which columns to include and exclude for local dictionary gene
      ```
      ALTER TABLE test_db.carbon CHANGE a3 a4 STRING
      ```
+     Example3:Change column a3's comment to "col_comment".
+     
+     ```
+     ALTER TABLE test_db.carbon CHANGE a3 a3 STRING COMMENT 'col_comment'
+     ```
 
      **NOTE:** Once the column is renamed, user has to take care about replacing the fileheader with the new name or changing the column header in csv file.
    
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala b/integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
index e4ec049..9cb8ed7 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
@@ -1110,8 +1110,7 @@ object CarbonParserUtil {
    */
   def parseDataType(
       dataType: String,
-      values: Option[List[(Int, Int)]],
-      isColumnRename: Boolean): DataTypeInfo = {
+      values: Option[List[(Int, Int)]]): DataTypeInfo = {
     var precision: Int = 0
     var scale: Int = 0
     dataType match {
@@ -1135,11 +1134,7 @@ object CarbonParserUtil {
         }
         DataTypeInfo("decimal", precision, scale)
       case _ =>
-        if (isColumnRename) {
-          DataTypeInfo(DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
-        } else {
-          throw new MalformedCarbonCommandException("Data type provided is invalid.")
-        }
+        DataTypeInfo(DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
     }
   }
 
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
index d96c4e1..04ea523 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
@@ -169,7 +169,8 @@ case class AlterTableDataTypeChangeModel(dataTypeInfo: DataTypeInfo,
     tableName: String,
     columnName: String,
     newColumnName: String,
-    isColumnRename: Boolean)
+    isColumnRename: Boolean,
+    newColumnComment: Option[String] = None)
   extends AlterTableColumnRenameModel(columnName, newColumnName, isColumnRename)
 
 case class AlterTableRenameModel(
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
index 8ce774a..b4e73bc 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
@@ -17,16 +17,20 @@
 
 package org.apache.spark.sql.execution.command.schema
 
+import java.util
+
 import scala.collection.JavaConverters._
 import scala.collection.mutable
 
+import org.apache.hadoop.hive.metastore.api.InvalidOperationException
 import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
 import org.apache.spark.sql.execution.command.{AlterTableDataTypeChangeModel, DataTypeInfo, MetadataCommand}
 import org.apache.spark.sql.hive.CarbonSessionCatalogUtil
-import org.apache.spark.util.{AlterTableUtil, SparkUtil}
+import org.apache.spark.util.AlterTableUtil
 
 import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
 import org.apache.carbondata.common.logging.LogServiceFactory
+import org.apache.carbondata.core.constants.CarbonCommonConstants
 import org.apache.carbondata.core.features.TableOperation
 import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
 import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
@@ -57,20 +61,6 @@ abstract class CarbonAlterTableColumnRenameCommand(oldColumnName: String, newCol
                                                 s"column ${ oldCarbonColumn.getColName }")
     }
 
-    // if column rename operation is on partition column, then fail the rename operation
-    if (null != carbonTable.getPartitionInfo) {
-      val partitionColumns = carbonTable.getPartitionInfo.getColumnSchemaList
-      partitionColumns.asScala.foreach {
-        col =>
-          if (col.getColumnName.equalsIgnoreCase(oldColumnName)) {
-            throw new MalformedCarbonCommandException(
-              s"Column Rename Operation failed. Renaming " +
-              s"the partition column $newColumnName is not " +
-              s"allowed")
-          }
-      }
-    }
-
     // if column rename operation is on bucket column, then fail the rename operation
     if (null != carbonTable.getBucketingInfo) {
       val bucketColumns = carbonTable.getBucketingInfo.getListOfColumns
@@ -139,6 +129,8 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
         .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
       val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName.toLowerCase
       val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName.toLowerCase
+      val isColumnRename = alterTableColRenameAndDataTypeChangeModel.isColumnRename
+      val newColumnComment = alterTableColRenameAndDataTypeChangeModel.newColumnComment
       val carbonColumns = carbonTable.getCreateOrderColumn().asScala.filter(!_.isInvisible)
       if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
         throwMetadataException(dbName, tableName, s"Column does not exist: $oldColumnName")
@@ -150,45 +142,48 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
       }
       val newColumnPrecision = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
       val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
-      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
-        // validate the columns to be renamed
-        validColumnsForRenaming(carbonColumns, oldCarbonColumn.head, carbonTable)
-        // if the datatype is source datatype, then it is just a column rename operation, else set
-        // the isDataTypeChange flag to true
-        if (oldCarbonColumn.head.getDataType.getName
-          .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
-          val newColumnPrecision =
-            alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
-          val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
-          // if the source datatype is decimal and there is change in precision and scale, then
-          // along with rename, datatype change is also required for the command, so set the
-          // isDataTypeChange flag to true in this case
-          if (oldCarbonColumn.head.getDataType.getName.equalsIgnoreCase("decimal") &&
-              (oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getPrecision !=
-               newColumnPrecision ||
-               oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getScale !=
-               newColumnScale)) {
-            isDataTypeChange = true
-          }
-        } else {
+      // set isDataTypeChange flag
+      if (oldCarbonColumn.head.getDataType.getName
+        .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
+        val newColumnPrecision =
+          alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
+        val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
+        // if the source datatype is decimal and there is change in precision and scale, then
+        // along with rename, datatype change is also required for the command, so set the
+        // isDataTypeChange flag to true in this case
+        if (oldCarbonColumn.head.getDataType.getName.equalsIgnoreCase("decimal") &&
+            (oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getPrecision !=
+             newColumnPrecision ||
+             oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getScale !=
+             newColumnScale)) {
           isDataTypeChange = true
         }
       } else {
         isDataTypeChange = true
       }
-      if (isDataTypeChange) {
-        // if column datatype change operation is on partition column, then fail the datatype change
-        // operation
-        if (null != carbonTable.getPartitionInfo) {
-          val partitionColumns = carbonTable.getPartitionInfo.getColumnSchemaList
-          partitionColumns.asScala.foreach {
-            col =>
-              if (col.getColumnName.equalsIgnoreCase(oldColumnName)) {
-                throw new MalformedCarbonCommandException(
-                  s"Alter datatype of the partition column $newColumnName is not allowed")
-              }
-          }
+      // If there is no columnrename and datatype change and comment change
+      // return directly without execution
+      if (!isColumnRename && !isDataTypeChange && !newColumnComment.isDefined) {
+        return Seq.empty
+      }
+      // if column datatype change operation is on partition column, then fail the
+      // chang column operation
+      if (null != carbonTable.getPartitionInfo) {
+        val partitionColumns = carbonTable.getPartitionInfo.getColumnSchemaList
+        partitionColumns.asScala.foreach {
+          col =>
+            if (col.getColumnName.equalsIgnoreCase(oldColumnName)) {
+              throw new InvalidOperationException(
+                s"Alter on partition column $newColumnName is not supported")
+            }
         }
+      }
+      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
+        // validate the columns to be renamed
+        validColumnsForRenaming(carbonColumns, oldCarbonColumn.head, carbonTable)
+      }
+      if (isDataTypeChange) {
+        // validate the columns to change datatype
         validateColumnDataType(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo,
           oldCarbonColumn.head)
       }
@@ -222,6 +217,14 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
               .setPrecision(newColumnPrecision)
             columnSchema.setScale(newColumnScale)
           }
+          if (newColumnComment.isDefined && columnSchema.getColumnProperties != null) {
+            columnSchema.getColumnProperties.put(
+              CarbonCommonConstants.COLUMN_COMMENT, newColumnComment.get)
+          } else if (newColumnComment.isDefined) {
+            val newColumnProperties = new util.HashMap[String, String]
+            newColumnProperties.put(CarbonCommonConstants.COLUMN_COMMENT, newColumnComment.get)
+            columnSchema.setColumnProperties(newColumnProperties)
+          }
           addColumnSchema = columnSchema
           timeStamp = System.currentTimeMillis()
           // make a new schema evolution entry after column rename or datatype change
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLHelper.scala b/integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLHelper.scala
index cf00db5..03f4330 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLHelper.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLHelper.scala
@@ -31,12 +31,13 @@ import org.apache.spark.sql.execution.command.table._
 import org.apache.spark.sql.execution.datasources.{LogicalRelation, RefreshResource, RefreshTable}
 import org.apache.spark.sql.hive.execution.CreateHiveTableAsSelectCommand
 import org.apache.spark.sql.parser.{CarbonSpark2SqlParser, CarbonSparkSqlParserUtil}
-import org.apache.spark.sql.types.DecimalType
+import org.apache.spark.sql.types.{DecimalType, Metadata}
 import org.apache.spark.sql.util.SparkSQLUtil
 import org.apache.spark.util.{CarbonReflectionUtils, FileUtils}
 
 import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
 import org.apache.carbondata.common.logging.LogServiceFactory
+import org.apache.carbondata.core.constants.CarbonCommonConstants
 import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, TableInfo}
 import org.apache.carbondata.core.util.{CarbonProperties, ThreadLocalSessionInfo}
 import org.apache.carbondata.spark.util.DataTypeConverterUtil
@@ -218,6 +219,7 @@ object DDLHelper {
     } else {
       val columnName = changeColumnCommand.columnName
       val newColumn = changeColumnCommand.newColumn
+      val newColumnMetaData = newColumn.metadata
       val isColumnRename = !columnName.equalsIgnoreCase(newColumn.name)
       val values = newColumn.dataType match {
         case d: DecimalType => Some(List((d.precision, d.scale)))
@@ -228,8 +230,14 @@ object DDLHelper {
           .convertToCarbonType(newColumn.dataType.typeName)
           .getName
           .toLowerCase,
-        values,
-        isColumnRename)
+        values)
+      var newColumnComment: Option[String] = Option.empty
+      if (newColumnMetaData != null &&
+        newColumnMetaData.contains(CarbonCommonConstants.COLUMN_COMMENT)) {
+        newColumnComment =
+          Some(newColumnMetaData.getString(CarbonCommonConstants.COLUMN_COMMENT))
+      }
+
       val alterTableColRenameAndDataTypeChangeModel =
         AlterTableDataTypeChangeModel(
           dataTypeInfo,
@@ -237,7 +245,8 @@ object DDLHelper {
           tableName.table.toLowerCase,
           columnName.toLowerCase,
           newColumn.name.toLowerCase,
-          isColumnRename)
+          isColumnRename,
+          newColumnComment)
 
       CarbonAlterTableColRenameDataTypeChangeCommand(
         alterTableColRenameAndDataTypeChangeModel
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonSessionUtil.scala b/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonSessionUtil.scala
index 2d3af66..c9f4c2a 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonSessionUtil.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonSessionUtil.scala
@@ -32,6 +32,7 @@ import org.apache.spark.util.{CarbonReflectionUtils, PartitionCacheKey, Partitio
 
 import org.apache.carbondata.common.logging.LogServiceFactory
 import org.apache.carbondata.converter.SparkDataTypeConverterImpl
+import org.apache.carbondata.core.constants.CarbonCommonConstants
 import org.apache.carbondata.core.metadata.schema.table.CarbonTable
 import org.apache.carbondata.core.metadata.schema.table.column.ColumnSchema
 import org.apache.carbondata.core.util.CarbonUtil
@@ -142,14 +143,16 @@ object CarbonSessionUtil {
       if (!column.isInvisible) {
         val structFiled =
           if (null != column.getColumnProperties &&
-              column.getColumnProperties.get("comment") != null) {
+              column.getColumnProperties.get(CarbonCommonConstants.COLUMN_COMMENT) != null) {
             StructField(column.getColumnName,
               SparkTypeConverter
                 .convertCarbonToSparkDataType(column,
                   carbonTable),
               true,
               // update the column comment if present in the schema
-              new MetadataBuilder().putString("comment", column.getColumnProperties.get("comment"))
+              new MetadataBuilder().putString(
+                CarbonCommonConstants.COLUMN_COMMENT,
+                column.getColumnProperties.get(CarbonCommonConstants.COLUMN_COMMENT))
                 .build())
           } else {
             StructField(column.getColumnName,
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/hive/SqlAstBuilderHelper.scala b/integration/spark/src/main/scala/org/apache/spark/sql/hive/SqlAstBuilderHelper.scala
index 2f897b9..2ffb007 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/hive/SqlAstBuilderHelper.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/hive/SqlAstBuilderHelper.scala
@@ -44,7 +44,7 @@ trait SqlAstBuilderHelper extends SparkSqlAstBuilder {
 
     val alterTableColRenameAndDataTypeChangeModel =
       AlterTableDataTypeChangeModel(
-        CarbonParserUtil.parseDataType(typeString, values, isColumnRename),
+        CarbonParserUtil.parseDataType(typeString, values),
         CarbonParserUtil.convertDbNameToLowerCase(Option(ctx.tableIdentifier().db).map(_.getText)),
         ctx.tableIdentifier().table.getText.toLowerCase,
         ctx.identifier.getText.toLowerCase,
diff --git a/integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala b/integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
index d0613ad..144e1fb 100644
--- a/integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
+++ b/integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
@@ -665,8 +665,7 @@ object CarbonSparkSqlParserUtil {
     val alterTableColRenameAndDataTypeChangeModel =
       AlterTableDataTypeChangeModel(
         CarbonParserUtil.parseDataType(dataType.toLowerCase,
-          values,
-          isColumnRename),
+          values),
         CarbonParserUtil.convertDbNameToLowerCase(dbName),
         table.toLowerCase,
         columnName.toLowerCase,
diff --git a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/standardpartition/StandardPartitionTableQueryTestCase.scala b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/standardpartition/StandardPartitionTableQueryTestCase.scala
index 690c724..f0ebfdf 100644
--- a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/standardpartition/StandardPartitionTableQueryTestCase.scala
+++ b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/standardpartition/StandardPartitionTableQueryTestCase.scala
@@ -515,12 +515,10 @@ test("Creation of partition table should fail if the colname in table schema and
       sql("alter table onlyPart drop columns(name)")
     }
     assert(ex1.getMessage.contains("alter table drop column is failed, cannot have the table with all columns as partition column"))
-    if (SparkUtil.isSparkVersionXAndAbove("2.2")) {
-      val ex2 = intercept[MalformedCarbonCommandException] {
-        sql("alter table onlyPart change age age bigint")
-      }
-      assert(ex2.getMessage.contains("Alter datatype of the partition column age is not allowed"))
+    val ex2 = intercept[MalformedCarbonCommandException] {
+      sql("alter table onlyPart change age age bigint")
     }
+    assert(ex2.getMessage.contains("Alter on partition column age is not supported"))
     sql("drop table if exists onlyPart")
   }
 
diff --git a/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala b/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
index 9b74d17..7517eb0 100644
--- a/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
+++ b/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
@@ -366,7 +366,7 @@ class AlterTableValidationTestCase extends QueryTest with BeforeAndAfterAll {
     sql("alter table default.restructure change decimalfield deciMalfield Decimal(11,3)")
     sql("alter table default.restructure change decimalfield deciMalfield Decimal(12,3)")
     intercept[ProcessMetaDataException] {
-      sql("alter table default.restructure change decimalfield deciMalfield Decimal(12,3)")
+      sql("alter table default.restructure change decimalfield deciMalfield Decimal(12,2)")
     }
     intercept[ProcessMetaDataException] {
       sql("alter table default.restructure change decimalfield deciMalfield Decimal(13,1)")
diff --git a/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/AlterTableColumnRenameTestCase.scala b/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/AlterTableColumnRenameTestCase.scala
index 29e4b43..d150abf 100644
--- a/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/AlterTableColumnRenameTestCase.scala
+++ b/integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/AlterTableColumnRenameTestCase.scala
@@ -29,7 +29,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   override def beforeAll(): Unit = {
     dropTable()
-    createTableAndLoad()
+    createNonPartitionTableAndLoad()
   }
 
   test("test only column rename operation") {
@@ -69,6 +69,39 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
     assert(ex.getMessage.contains("New column name workgroupcategoryname already exists in table rename"))
   }
 
+  test("test change column command with comment") {
+    dropTable()
+    createNonPartitionTableAndLoad()
+    createPartitionTableAndLoad()
+
+    // Non-Partition Column with Non-Complex Datatype
+    testChangeColumnWithComment("rename")
+    testChangeColumnWithComment("rename_partition")
+
+    // Non-Partition Column with Complex Datatype
+    sql("DROP TABLE IF EXISTS rename_complextype")
+    sql(s"create table rename_complextype(mapcol map<string,string>," +
+      s" arraycol array<string>) stored as carbondata")
+    testChangeColumnWithComment("rename_complextype", "mapcol",
+      "mapcol", "map<string,string>", "map<string,string>", "map comment", false)
+    testChangeColumnWithComment("rename_complextype", "arraycol",
+      "arraycol", "array<string>", "array<string>", "array comment", false)
+
+    // Partition Column
+    val ex = intercept[ProcessMetaDataException] {
+      sql(s"alter table rename_partition change projectcode projectcode int comment 'partitoncolumn comment'")
+    }
+    ex.getMessage.contains(s"Alter on partition column projectcode is not supported")
+    checkExistence(sql(s"describe formatted rename_partition"), false, "partitoncolumn comment")
+
+    // Bucket Column
+    sql("DROP TABLE IF EXISTS rename_bucket")
+    sql("CREATE TABLE rename_bucket (ID Int, date Timestamp, country String, name String)" +
+      " STORED AS carbondata TBLPROPERTIES ('BUCKET_NUMBER'='4', 'BUCKET_COLUMNS'='name')")
+    testChangeColumnWithComment("rename_bucket", "name",
+      "name", "string", "string", "bucket comment", false)
+  }
+
   test("column rename for different datatype"){
     dropTable()
     createTable()
@@ -83,7 +116,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   test("query count after column rename and filter results"){
     dropTable()
-    createTableAndLoad()
+    createNonPartitionTableAndLoad()
     val df1 = sql("select empname from rename").collect()
     val df3 = sql("select workgroupcategory from rename where empname = 'bill' or empname = 'sibi'").collect()
     sql("alter table rename change empname empAddress string")
@@ -98,7 +131,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   test("compaction after column rename and count"){
     dropTable()
-    createTableAndLoad()
+    createNonPartitionTableAndLoad()
     for(i <- 0 to 2) {
       loadToTable()
     }
@@ -112,7 +145,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   test("test rename after adding column and drop column") {
     dropTable()
-    createTableAndLoad()
+    createNonPartitionTableAndLoad()
     sql("alter table rename add columns(newAdded string)")
     var carbonTable = CarbonMetadata.getInstance().getCarbonTable("default", "rename")
     assert(null != carbonTable.getColumnByName("newAdded"))
@@ -130,7 +163,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   test("test column rename and update and insert and delete") {
     dropTable()
-    createTableAndLoad()
+    createNonPartitionTableAndLoad()
     sql("alter table rename change empname name string")
     sql("update rename set (name) = ('joey') where workgroupcategory = 'developer'").show()
     sql("insert into rename select 20,'bill','PM','01-12-2015',3,'manager',14,'Learning',928479,'01-01-2016','30-11-2016',75,94,13547")
@@ -254,7 +287,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
 
   test("test SET command with column rename") {
     dropTable()
-     createTable()
+    createTable()
     sql("alter table rename change workgroupcategoryname testset string")
     val ex = intercept[Exception] {
       sql("alter table rename set tblproperties('column_meta_cache'='workgroupcategoryname')")
@@ -295,11 +328,7 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
     createTable()
     checkExistence(sql("describe formatted rename"), true, "This column has comment ")
     sql("alter table rename change deptno classno bigint")
-    if (SparkUtil.isSparkVersionEqualTo("2.1")) {
-      checkExistence(sql("describe formatted rename"), false, "This column has comment ")
-    } else if (SparkUtil.isSparkVersionXAndAbove("2.2")) {
-      checkExistence(sql("describe formatted rename"), true, "This column has comment ")
-    }
+    checkExistence(sql("describe formatted rename"), true, "This column has comment ")
   }
 
   test("test compaction after table rename and alter set tblproerties") {
@@ -358,13 +387,64 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
   }
 
   def dropTable(): Unit = {
-    sql("DROP TABLE IF EXISTS RENAME")
+    sql("DROP TABLE IF EXISTS rename")
+    sql("DROP TABLE IF EXISTS rename_partition")
     sql("DROP TABLE IF EXISTS test_rename")
     sql("DROP TABLE IF EXISTS test_rename_compact")
     sql("DROP TABLE IF EXISTS test_alter")
   }
 
-  def createTableAndLoad(): Unit = {
+  def testChangeColumnWithComment(tableName: String): Unit = {
+    // testcase1: columnrename: no; datatypechange: no;
+    testChangeColumnWithComment(tableName, "testcase1_1_col",
+      "testcase1_1_col", "string", "string", "testcase1_1 comment", true)
+    testChangeColumnWithComment(tableName, "testcase1_2_col",
+      "testcase1_2_col", "int", "int", "testcase1_2 comment", true)
+    testChangeColumnWithComment(tableName, "testcase1_3_col",
+      "testcase1_3_col", "decimal(30,10)", "decimal(30,10)", "testcase1_3 comment", true)
+
+    // testcase2: columnrename: yes; datatypechange: no;
+    testChangeColumnWithComment(tableName, "testcase2_col",
+      "testcase2_col_renamed", "string", "string", "testcase2 comment", true)
+
+    // testcase3: columnrename: no; datatypechange: yes
+    testChangeColumnWithComment(tableName, "testcase3_1_col",
+      "testcase3_1_col", "int", "bigint", "testcase3_1 comment", true)
+    testChangeColumnWithComment(tableName, "testcase3_2_col",
+      "testcase3_2_col", "decimal(30,10)", "decimal(32,11)", "testcase3_2 comment", true)
+
+    // testcase4: columnrename: yes; datatypechange: yes,
+    testChangeColumnWithComment(tableName, "testcase4_1_col",
+      "testcase4_1_col_renamed", "int", "bigint", "testcase4_1 comment", true)
+    testChangeColumnWithComment(tableName, "testcase4_2_col",
+      "testcase4_2_col_renmaed", "decimal(30,10)", "decimal(32,11)", "testcase4_2 comment", true)
+
+    // testcase5: special characters in comments
+    testChangeColumnWithComment(tableName, "testcase5_1_col",
+      "testcase5_1_col_renamed", "string", "string", "测试comment", true)
+    testChangeColumnWithComment(tableName, "testcase5_2_col",
+      "testcase5_2_col_renmaed", "decimal(30,10)", "decimal(32,11)", "\001\002comment", true)
+  }
+
+  def testChangeColumnWithComment(tableName: String, oldColumnName: String,
+      newColumnName: String, oldDataType: String, newDataType: String, comment: String,
+      needCreateOldColumn: Boolean): Unit = {
+    checkExistence(sql(s"describe formatted $tableName"), false, comment)
+    if (needCreateOldColumn) {
+      sql(s"alter table $tableName add columns ($oldColumnName $oldDataType)")
+    }
+    sql(s"alter table $tableName change $oldColumnName $newColumnName $newDataType comment '$comment'")
+    checkExistence(sql(s"describe formatted $tableName"), true, comment)
+    if (!newDataType.equalsIgnoreCase(oldDataType)) {
+      sql(s"describe formatted $tableName")
+        .collect.find(_.get(0).toString.contains(newColumnName)) match {
+        case Some(row) => assert(row.get(1).toString.contains(newDataType))
+        case None => assert(false)
+      }
+    }
+  }
+
+  def createNonPartitionTableAndLoad(): Unit = {
     sql(
       "CREATE TABLE rename (empno int, empname String, designation String, doj Timestamp, " +
       "workgroupcategory int, workgroupcategoryname String, deptno int, deptname String, " +
@@ -375,6 +455,18 @@ class AlterTableColumnRenameTestCase extends QueryTest with BeforeAndAfterAll {
          |('DELIMITER'= ',', 'QUOTECHAR'= '\"')""".stripMargin)
   }
 
+  def createPartitionTableAndLoad(): Unit = {
+    sql(
+      "CREATE TABLE rename_partition (empno int, empname String, designation String," +
+        " doj Timestamp, workgroupcategory int, workgroupcategoryname String, deptno int," +
+        " deptname String," +
+        " projectjoindate Timestamp, projectenddate Timestamp,attendance int," +
+        " utilization int,salary int) PARTITIONED BY (projectcode int) STORED AS carbondata")
+    sql(
+      s"""LOAD DATA LOCAL INPATH '$resourcesPath/data.csv' INTO TABLE rename OPTIONS
+         |('DELIMITER'= ',', 'QUOTECHAR'= '\"')""".stripMargin)
+  }
+
   def loadToTable():Unit = {
     sql(
       s"""LOAD DATA LOCAL INPATH '$resourcesPath/data.csv' INTO TABLE rename OPTIONS