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 <[email protected]>
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

Reply via email to