[GitHub] [carbondata] jackylk opened a new pull request #3598: [WIP] Remove MDK and cardinality in write path

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377104995
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/block/SegmentProperties.java
 ##########
 @@ -640,15 +377,91 @@ public int getNumberOfSortColumns() {
     return numberOfSortColumns;
   }
 
-  public int getNumberOfNoDictSortColumns() {
-    return numberOfNoDictSortColumns;
+  public int getLastDimensionColOrdinal() {
+    return lastDimensionColOrdinal;
+  }
+
+  public int getNumberOfColumns() {
+    return numberOfColumnsAfterFlatten;
   }
 
-  public int getNumberOfDictSortColumns() {
-    return this.numberOfSortColumns - this.numberOfNoDictSortColumns;
+  public int getNumberOfDictDimensions() {
+    return numberOfDictDimensions;
   }
 
-  public int getLastDimensionColOrdinal() {
-    return lastDimensionColOrdinal;
+  public int getNumberOfSimpleDimensions() {
 
 Review comment:
   fixed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377105758
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/block/TableBlockInfo.java
 ##########
 @@ -74,31 +72,10 @@
    */
   private Segment segment;
 
-  /**
-   * id of the Blocklet.
-   */
-  private String blockletId;
 
 Review comment:
   It is not used anywhere

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377105882
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/blocklet/EncodedBlocklet.java
 ##########
 @@ -38,11 +37,6 @@
    */
   private int blockletSize;
 
-  /**
-   * list of page metadata
-   */
-  private List<TablePageKey> pageMetadataList;
 
 Review comment:
   yes, it is not used in query flow

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377106134
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/scan/processor/DataBlockIterator.java
 ##########
 @@ -217,7 +217,8 @@ public BlockletScannedResult call() throws Exception {
             nextRead.set(true);
             futureIo = readNextBlockletAsync();
           }
-          return blockletScanner.scanBlocklet(rawBlockletColumnChunks);
+          BlockletScannedResult result = blockletScanner.scanBlocklet(rawBlockletColumnChunks);
 
 Review comment:
   fixed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377106925
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java
 ##########
 @@ -756,4 +756,45 @@ public static long toLongLittleEndian(byte[] bytes, int offset) {
         ((long) bytes[offset + 3] & 0xff) << 24) | (((long) bytes[offset + 2] & 0xff) << 16) | (
         ((long) bytes[offset + 1] & 0xff) << 8) | (((long) bytes[offset] & 0xff)));
   }
+
+  public static byte[] convertDateToBytes(int date) {
+    return ByteUtil.toBytes(date);
+  }
+
+  public static byte[] convertDateToBytes(long[] date) {
+    byte[] output = new byte[date.length * 4];
+    for (int i = 0; i < date.length; i++) {
+      System.arraycopy(ByteUtil.toBytes(date[i]), 0, output, i * 4, 4);
+    }
+    return output;
+  }
+
+  public static int convertBytesToDate(byte[] date) {
 
 Review comment:
   fixed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377106994
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java
 ##########
 @@ -756,4 +756,45 @@ public static long toLongLittleEndian(byte[] bytes, int offset) {
         ((long) bytes[offset + 3] & 0xff) << 24) | (((long) bytes[offset + 2] & 0xff) << 16) | (
         ((long) bytes[offset + 1] & 0xff) << 8) | (((long) bytes[offset] & 0xff)));
   }
+
+  public static byte[] convertDateToBytes(int date) {
+    return ByteUtil.toBytes(date);
+  }
+
+  public static byte[] convertDateToBytes(long[] date) {
+    byte[] output = new byte[date.length * 4];
+    for (int i = 0; i < date.length; i++) {
+      System.arraycopy(ByteUtil.toBytes(date[i]), 0, output, i * 4, 4);
+    }
+    return output;
+  }
+
+  public static int convertBytesToDate(byte[] date) {
+    return ByteUtil.toInt(date, 0);
+  }
+
+  public static int convertBytesToDate(byte[] date, int offset) {
+    return ByteUtil.toInt(date, offset);
+  }
+
+  public static int dateBytesSize() {
+    return 4;
+  }
+
+  public static int[] convertBytesToDateIntArray(byte[] input) {
 
 Review comment:
   fixed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377107352
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java
 ##########
 @@ -756,4 +756,45 @@ public static long toLongLittleEndian(byte[] bytes, int offset) {
         ((long) bytes[offset + 3] & 0xff) << 24) | (((long) bytes[offset + 2] & 0xff) << 16) | (
         ((long) bytes[offset + 1] & 0xff) << 8) | (((long) bytes[offset] & 0xff)));
   }
+
+  public static byte[] convertDateToBytes(int date) {
+    return ByteUtil.toBytes(date);
+  }
+
+  public static byte[] convertDateToBytes(long[] date) {
+    byte[] output = new byte[date.length * 4];
+    for (int i = 0; i < date.length; i++) {
+      System.arraycopy(ByteUtil.toBytes(date[i]), 0, output, i * 4, 4);
+    }
+    return output;
+  }
+
+  public static int convertBytesToDate(byte[] date) {
+    return ByteUtil.toInt(date, 0);
+  }
+
+  public static int convertBytesToDate(byte[] date, int offset) {
+    return ByteUtil.toInt(date, offset);
+  }
+
+  public static int dateBytesSize() {
+    return 4;
+  }
+
+  public static int[] convertBytesToDateIntArray(byte[] input) {
 
 Review comment:
   fixed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377107585
 
 

 ##########
 File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonMergerRDD.scala
 ##########
 @@ -578,15 +562,11 @@ class CarbonMergerRDD[K, V](
       }
     }
     val updatedMaxSegmentColumnList = new util.ArrayList[ColumnSchema]()
-    // update cardinality and column schema list according to master schema
-    val cardinality = CarbonCompactionUtil
 
 Review comment:
   It is needed, otherwise testcase failed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377107585
 
 

 ##########
 File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonMergerRDD.scala
 ##########
 @@ -578,15 +562,11 @@ class CarbonMergerRDD[K, V](
       }
     }
     val updatedMaxSegmentColumnList = new util.ArrayList[ColumnSchema]()
-    // update cardinality and column schema list according to master schema
-    val cardinality = CarbonCompactionUtil
 
 Review comment:
   It is needed, otherwise testcase failed

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377110655
 
 

 ##########
 File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonMergerRDD.scala
 ##########
 @@ -578,15 +562,11 @@ class CarbonMergerRDD[K, V](
       }
     }
     val updatedMaxSegmentColumnList = new util.ArrayList[ColumnSchema]()
-    // update cardinality and column schema list according to master schema
-    val cardinality = CarbonCompactionUtil
 
 Review comment:
   I will remove it and try the CI

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584171568
 
 
   Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/217/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584198947
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1919/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584467260
 
 
   Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/225/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377110655
 
 

 ##########
 File path: integration/spark-common/src/main/scala/org/apache/carbondata/spark/rdd/CarbonMergerRDD.scala
 ##########
 @@ -578,15 +562,11 @@ class CarbonMergerRDD[K, V](
       }
     }
     val updatedMaxSegmentColumnList = new util.ArrayList[ColumnSchema]()
-    // update cardinality and column schema list according to master schema
-    val cardinality = CarbonCompactionUtil
 
 Review comment:
   I have created JIRA for this: CARBONDATA-3691, will fix later

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#discussion_r377436567
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/block/SegmentProperties.java
 ##########
 @@ -640,15 +377,91 @@ public int getNumberOfSortColumns() {
     return numberOfSortColumns;
   }
 
-  public int getNumberOfNoDictSortColumns() {
-    return numberOfNoDictSortColumns;
+  public int getLastDimensionColOrdinal() {
+    return lastDimensionColOrdinal;
+  }
+
+  public int getNumberOfColumns() {
+    return numberOfColumnsAfterFlatten;
   }
 
-  public int getNumberOfDictSortColumns() {
-    return this.numberOfSortColumns - this.numberOfNoDictSortColumns;
+  public int getNumberOfDictDimensions() {
+    return numberOfDictDimensions;
   }
 
-  public int getLastDimensionColOrdinal() {
-    return lastDimensionColOrdinal;
+  public int getNumberOfSimpleDimensions() {
+    return numberOfDictDimensions + numberOfNoDictionaryDimension;
+  }
+
+  public int getNumberOfComplexDimensions() {
+    return complexDimensions.size();
+  }
+
+  public int getNumberOfMeasures() {
+    return measures.size();
+  }
+
+  /**
+   * Return column value length in byte for all dimension columns in the table
+   * for dimension it is -1 (for DATE it is 4),
+   */
+  public int[] createDimColumnValueLength() {
+    int[] length = new int[dimensions.size()];
+    int index = 0;
+    for (CarbonDimension dimension : dimensions) {
+      DataType dataType = dimension.getDataType();
+      if (dataType == DataTypes.DATE) {
+        length[index] = 4;
+      } else {
+        length[index] = -1;
+      }
+      index++;
+    }
+    return length;
+  }
+
+  /**
+   * Return column value length in byte for all columns in the table
+   * for dimension and complex column it is -1 (for DATE it is 4),
+   * for measure is 8 (for decimal is -1)
+   */
+  public int[] createColumnValueLength() {
+    int[] length = new int[numberOfColumnsAfterFlatten];
+    int index = 0;
+    for (CarbonDimension dimension : dimensions) {
+      DataType dataType = dimension.getDataType();
+      if (dataType == DataTypes.DATE) {
+        length[index] = 4;
+      } else {
+        length[index] = -1;
+      }
+      index++;
+    }
+    for (CarbonDimension complexDimension : complexDimensions) {
+      int depth = getNumColumnsAfterFlatten(complexDimension);
+      for (int i = 0; i < depth; i++) {
+        length[index++] = -1;
+      }
+    }
+    for (CarbonMeasure measure : measures) {
+      DataType dataType = measure.getDataType();
+      if (DataTypes.isDecimal(dataType)) {
+        length[index++] = -1;
+      } else {
+        length[index++] = 8;
 
 Review comment:
   I have created JIRA for this: CARBONDATA-3690, will fix later

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584476188
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1927/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584611034
 
 
   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]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584618073
 
 
   Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/239/
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584647238
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1941/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on issue #3598: [CARBONDATA-3684] Remove MDK and cardinality in write path
URL: https://github.com/apache/carbondata/pull/3598#issuecomment-584734576
 
 
   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]


With regards,
Apache Git Services