[GitHub] carbondata pull request #2396: [WIP] Complex data type enhancements

[qiuchenjian-2]

Github user Indhumathi27 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197678288
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -27,6 +28,7 @@
     import org.apache.carbondata.core.keygenerator.directdictionary.DirectDictionaryKeyGeneratorFactory;
     import org.apache.carbondata.core.metadata.datatype.DataTypes;
     import org.apache.carbondata.core.metadata.encoder.Encoding;
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonDimension;
    --- End diff --
   
    Dictionary will be handled with Adaptive encoding PR


---

[qiuchenjian-2]

Github user Indhumathi27 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197678648
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -67,17 +69,38 @@
       int noDictionaryColumnIndex;
       int complexTypeColumnIndex;
     
    +  int noDictionaryComplexColumnIndex = 0;
    +  int complexTypeComplexColumnIndex = 0;
    +
       boolean isDimensionExists;
     
    +  private int[] surrogateResult;
    +  private byte[][] noDictionaryKeys;
    +  private byte[][] complexTypeKeyArray;
    +
       protected Map<Integer, GenericQueryType> comlexDimensionInfoMap;
     
    +  /**
    +   * Field of this Map is the parent Column and associated child columns.
    +   * Final Projection shuld be a merged list consist of only parents.
    +   */
    +  public Map<Integer, List<Integer>> mergedComplexDimensionColumns;
    +
    +  /**
    +   * Fields of this Map of Parent Ordinal with the List is the Child Column Dimension and
    +   * the corresponding data buffer of that column.
    +   */
    +
    +  public Map<Integer, Map<CarbonDimension, ByteBuffer>> mergedComplexDimensionDataMap;
    --- End diff --
   
    It is already at class level and initialised only once and reused


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5342/



---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197723498
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -67,17 +69,38 @@
       int noDictionaryColumnIndex;
       int complexTypeColumnIndex;
     
    +  int noDictionaryComplexColumnIndex = 0;
    --- End diff --
   
    Why it cannot be moved?


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197723962
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -67,17 +69,38 @@
       int noDictionaryColumnIndex;
       int complexTypeColumnIndex;
     
    +  int noDictionaryComplexColumnIndex = 0;
    +  int complexTypeComplexColumnIndex = 0;
    +
       boolean isDimensionExists;
     
    +  private int[] surrogateResult;
    +  private byte[][] noDictionaryKeys;
    +  private byte[][] complexTypeKeyArray;
    +
       protected Map<Integer, GenericQueryType> comlexDimensionInfoMap;
     
    +  /**
    +   * Field of this Map is the parent Column and associated child columns.
    +   * Final Projection shuld be a merged list consist of only parents.
    +   */
    +  public Map<Integer, List<Integer>> mergedComplexDimensionColumns;
    +
    +  /**
    +   * Fields of this Map of Parent Ordinal with the List is the Child Column Dimension and
    +   * the corresponding data buffer of that column.
    +   */
    +
    +  public Map<Integer, Map<CarbonDimension, ByteBuffer>> mergedComplexDimensionDataMap;
    --- End diff --
   
    it should be private


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197724373
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -102,6 +123,9 @@ public DictionaryBasedResultCollector(BlockExecutionInfo blockExecutionInfos) {
             dictionaryColumnIndex = 0;
             noDictionaryColumnIndex = 0;
             complexTypeColumnIndex = 0;
    +        mergedComplexDimensionDataMap = new HashMap<>();
    --- End diff --
   
    Why it is created for each row? why not reuse ?


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2396#discussion_r197724441
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/collector/impl/DictionaryBasedResultCollector.java ---
    @@ -67,17 +69,38 @@
       int noDictionaryColumnIndex;
       int complexTypeColumnIndex;
     
    +  int noDictionaryComplexColumnIndex = 0;
    +  int complexTypeComplexColumnIndex = 0;
    +
       boolean isDimensionExists;
     
    +  private int[] surrogateResult;
    +  private byte[][] noDictionaryKeys;
    +  private byte[][] complexTypeKeyArray;
    +
       protected Map<Integer, GenericQueryType> comlexDimensionInfoMap;
     
    +  /**
    +   * Field of this Map is the parent Column and associated child columns.
    +   * Final Projection shuld be a merged list consist of only parents.
    +   */
    +  public Map<Integer, List<Integer>> mergedComplexDimensionColumns;
    --- End diff --
   
    it should be at method level


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6521/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5352/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5436/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6527/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6532/



---

[qiuchenjian-2]

Github user Indhumathi27 commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5361/



---

[qiuchenjian-2]

Github user Indhumathi27 commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5439/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6534/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5364/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6543/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2396
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5447/



---