kunal642 commented on a change in pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#discussion_r496461049



##########
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
##########
@@ -150,45 +141,50 @@ 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 MalformedCarbonCommandException(
+                s"Change Column Operation failed. Renaming or ChangeType or " +

Review comment:
       Please change this to 
   
   `throw new InvalidOperationException("Alter on partition columns is not 
supported")`
   
   And add a test case for this




----------------------------------------------------------------
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


Reply via email to