[GitHub] [carbondata] marchpure opened a new pull request #3960: [CARBONDATA-4014] Support Change Column Comment

classic Classic list List threaded Threaded
41 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] QiangCai commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox

QiangCai commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-699879164


   if alter column change nothing, the command should return directly without execution.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-699879852


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4252/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-699883892


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2507/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] marchpure commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

marchpure commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700109547


   > if alter column change nothing, the command should return directly without execution.
   
   I have modified code according to you suggestion.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700181171


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2517/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700202083


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4262/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] Klaus-xjp commented on a change in pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

Klaus-xjp commented on a change in pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#discussion_r496322133



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
##########
@@ -258,7 +262,7 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
       }
     } catch {
       case e: Exception =>
-        if (carbonTable != null) {
+       if ( carbonTable != null) {

Review comment:
       there has a blank between the '(' and carbonTable




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] marchpure commented on a change in pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

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



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
##########
@@ -258,7 +262,7 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
       }
     } catch {
       case e: Exception =>
-        if (carbonTable != null) {
+       if ( carbonTable != null) {

Review comment:
       I have modified code according to you suggestion.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700405441


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2520/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700406306


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4265/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] marchpure commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

marchpure commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700407129


   retest this please


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700440309


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4266/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700451914


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2521/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] Zhangshunyu commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

Zhangshunyu commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700485310


   LGTM


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] kunal642 commented on a change in pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700545126


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4268/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700553209


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2523/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700647666


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4269/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700650642


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2524/
   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] kunal642 commented on pull request #3960: [CARBONDATA-4014] Support Change Column Comment

GitBox
In reply to this post by GitBox

kunal642 commented on pull request #3960:
URL: https://github.com/apache/carbondata/pull/3960#issuecomment-700659418


   LGTM


----------------------------------------------------------------
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:
[hidden email]


123