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

[GitBox <gi...@apache.org>]

akkio-97 commented on a change in pull request #4129:
URL: https://github.com/apache/carbondata/pull/4129#discussion_r638736579



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
##########
@@ -50,6 +52,10 @@
  * Utility class for restructuring
  */
 public class RestructureUtil {
+  // if table column is of complex type- this look up stores the column id of the parent and their
+  // children. This helps to determine the existence of incoming query column by matching based
+  // on id.
+  private static Map<String, String> existingTableColumnIDMap = new HashMap<>();

Review comment:
       In createDimensionInfoAndGetCurrentBlockQueryDimension(), if you see we have two for loops - 1) for tableComplexDimensions. 2) for query dimensions.
   Table and query dimensions are compared using the map. If you dont re-initialize for every new incoming table dimension, then map may contain unwanted old dimensions.

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/executor/util/RestructureUtil.java
##########
@@ -154,11 +160,21 @@
     return presentDimension;
   }
 
+  public static void fillExistingTableColumnIDMap(CarbonDimension tableColumn) {

Review comment:
       yes we dont get child related dimensions. For ex, query is on struct.id. This name may not be same with the table dimension name if schema has evolved.
   So first we populate the map with table dimensions(including children). Later matching happens based on id for each query name.




-- 
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:
users@infra.apache.org