[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4129: [CARBONDATA-4179] Support renaming of complex columns (array/struct)

Posted by GitBox on
URL: http://apache-carbondata-dev-mailing-list-archive.168.s1.nabble.com/GitHub-carbondata-akkio-97-opened-a-new-pull-request-4129-WIP-alter-rename-complex-types-tp108015p108687.html


Indhumathi27 commented on a change in pull request #4129:
URL: https://github.com/apache/carbondata/pull/4129#discussion_r644509268



##########
File path: docs/ddl-of-carbondata.md
##########
@@ -819,33 +819,47 @@ Users can specify which columns to include and exclude for local dictionary gene
      - 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
+       * Rename operation fails if the structure of the complex column has been altered. Please ensure the old and new columns are compatible with

Review comment:
       Looks like, the links under RENAME COLUMN / CHANGE COLUMN NAME/TYPE/COMMENT is not working. Can you check and fix it

##########
File path: docs/ddl-of-carbondata.md
##########
@@ -819,33 +819,47 @@ Users can specify which columns to include and exclude for local dictionary gene
      - 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
+       * Rename operation fails if the structure of the complex column has been altered. Please ensure the old and new columns are compatible with

Review comment:
       Looks like, the links under RENAME COLUMN / CHANGE COLUMN NAME/TYPE/COMMENT is not navigating to the description. Can you check and fix it

##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
##########
@@ -1132,10 +1135,60 @@ object CarbonParserUtil {
         } else if (scale < 0 || scale > 38) {
           throw new MalformedCarbonCommandException("Invalid value for scale")
         }
-        DataTypeInfo("decimal", precision, scale)
+        DataTypeInfo(columnName, "decimal", precision, scale)
+      case _ =>
+        DataTypeInfo(columnName,
+          DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
+    }
+  }
+
+  /**
+   * This method will return the instantiated DataTypeInfo by parsing the column
+   */
+  def parseColumn(columnName: String, dataType: DataType,
+      values: Option[List[(Int, Int)]]): DataTypeInfo = {
+    // creates parent dataTypeInfo first
+    val dataTypeInfo = CarbonParserUtil.parseDataType(

Review comment:
       ```suggestion
       val dataTypeName = DataTypeConverterUtil.convertToCarbonType(dataType.typeName).getName
       val dataTypeInfo = CarbonParserUtil.parseDataType(columnName, dataTypeName.toLowerCase,values)
   ```

##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
##########
@@ -1132,10 +1135,60 @@ object CarbonParserUtil {
         } else if (scale < 0 || scale > 38) {
           throw new MalformedCarbonCommandException("Invalid value for scale")
         }
-        DataTypeInfo("decimal", precision, scale)
+        DataTypeInfo(columnName, "decimal", precision, scale)
+      case _ =>
+        DataTypeInfo(columnName,
+          DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
+    }
+  }
+
+  /**
+   * This method will return the instantiated DataTypeInfo by parsing the column
+   */
+  def parseColumn(columnName: String, dataType: DataType,
+      values: Option[List[(Int, Int)]]): DataTypeInfo = {
+    // creates parent dataTypeInfo first
+    val dataTypeInfo = CarbonParserUtil.parseDataType(
+      columnName,
+      DataTypeConverterUtil
+        .convertToCarbonType(dataType.typeName)
+        .getName
+        .toLowerCase,
+      values)
+    // check which child type is present and create children dataTypeInfo accordingly
+    dataType match {
+      case arrayType: ArrayType =>
+        val childType: DataType = arrayType.elementType
+        val childName = columnName + ".val"
+        val childValues = childType match {
+          case d: DecimalType => Some(List((d.precision, d.scale)))
+          case _ => None
+        }
+        var childTypeInfoList: List[DataTypeInfo] = null
+        val childDatatypeInfo = parseColumn(childName, childType, childValues)
+        childTypeInfoList = List(childDatatypeInfo)

Review comment:
       Could directly set, dataTypeInfo.setChildren( List(childDatatypeInfo)), since childTypeInfoList is not used anywhere else




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