[GitHub] carbondata pull request #2209: [WIP][Non Transactional Table]Avro Record Com...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r184976699
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/complextypes/PrimitiveQueryType.java ---
    @@ -53,6 +55,21 @@ public PrimitiveQueryType(String name, String parentname, int blockIndex,
         this.name = name;
         this.parentname = parentname;
         this.isDirectDictionary = isDirectDictionary;
    +    this.isDictionary = true;
    +  }
    +
    +
    +  public PrimitiveQueryType(String name, String parentname, int blockIndex,
    +      org.apache.carbondata.core.metadata.datatype.DataType dataType, int keySize,
    +      Dictionary dictionary, boolean isDirectDictionary, boolean isDictionary) {
    --- End diff --
   
    Done.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r184976884
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/complextypes/PrimitiveQueryType.java ---
    @@ -84,6 +101,9 @@ public PrimitiveQueryType(String name, String parentname, int blockIndex,
           DimensionRawColumnChunk[] rawColumnChunks, int rowNumber,
           int pageNumber, DataOutputStream dataOutputStream) throws IOException {
         byte[] currentVal = copyBlockDataChunk(rawColumnChunks, rowNumber, pageNumber);
    +    if (!this.isDictionary) {
    +      dataOutputStream.writeInt(currentVal.length);
    --- End diff --
   
    Below line 105 we are writing the data. Data writing is without any condition.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r184977270
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -267,14 +267,24 @@ private static String getComplexTypeString(DataField[] dataFields) {
         return dimString.toString();
       }
     
    +  private static String isDictionaryType(CarbonDimension dimension) {
    --- End diff --
   
    This is used to append in a String field.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r184992863
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/TablePage.java ---
    @@ -222,8 +222,9 @@ private void addComplexColumn(int index, int rowId, byte[] complexColumns) {
           ByteBuffer byteArrayInput = ByteBuffer.wrap(complexColumns);
           ByteArrayOutputStream byteArrayOutput = new ByteArrayOutputStream();
           DataOutputStream dataOutputStream = new DataOutputStream(byteArrayOutput);
    -      complexDataType.parseAndBitPack(byteArrayInput, dataOutputStream,
    -          model.getComplexDimensionKeyGenerator());
    +      int startOffset = 0;
    --- End diff --
   
    Removed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2209
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4637/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r185213084
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/datatypes/PrimitiveDataType.java ---
    @@ -211,29 +241,66 @@ public int getSurrogateIndex() {
       /*
        * set surrogate index
        */
    -  @Override
    -  public void setSurrogateIndex(int surrIndex) {
    -    index = surrIndex;
    +  @Override public void setSurrogateIndex(int surrIndex) {
    +    if (this.carbonDimension != null && !this.carbonDimension.hasEncoding(Encoding.DICTIONARY)) {
    +      index = 0;
    +    } else if (this.carbonDimension == null && isDictionary == false) {
    +      index = 0;
    +    } else {
    +      index = surrIndex;
    +    }
    +  }
    +
    +  @Override public Boolean getIsColumnDictionary() {
    +    return isDictionary;
       }
     
       @Override public void writeByteArray(Object input, DataOutputStream dataOutputStream)
           throws IOException, DictionaryGenerationException {
         String parsedValue =
             input == null ? null : DataTypeUtil.parseValue(input.toString(), carbonDimension);
    -    Integer surrogateKey;
    -    if (null == parsedValue) {
    -      surrogateKey = CarbonCommonConstants.MEMBER_DEFAULT_VAL_SURROGATE_KEY;
    -    } else {
    -      surrogateKey = dictionaryGenerator.getOrGenerateKey(parsedValue);
    -      if (surrogateKey == CarbonCommonConstants.INVALID_SURROGATE_KEY) {
    +    if (this.isDictionary) {
    +      Integer surrogateKey;
    +      if (null == parsedValue) {
             surrogateKey = CarbonCommonConstants.MEMBER_DEFAULT_VAL_SURROGATE_KEY;
    +      } else {
    +        surrogateKey = dictionaryGenerator.getOrGenerateKey(parsedValue);
    +        if (surrogateKey == CarbonCommonConstants.INVALID_SURROGATE_KEY) {
    +          surrogateKey = CarbonCommonConstants.MEMBER_DEFAULT_VAL_SURROGATE_KEY;
    +        }
    +      }
    +      dataOutputStream.writeInt(surrogateKey);
    +    } else {
    +      // Transform into ByteArray for No Dictionary.
    +      // TODO have to refactor and place all the cases present in NonDictionaryFieldConverterImpl
    +      if (null == parsedValue && this.carbonDimension.getDataType() != DataTypes.STRING) {
    --- End diff --
   
    The check has to be similar to NonDictionaryFieldConverterImpl


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r185217366
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/SparkTypeConverter.scala ---
    @@ -97,16 +97,30 @@ private[spark] object SparkTypeConverter {
       def getStructChildren(table: CarbonTable, dimName: String): String = {
         table.getChildren(dimName).asScala.map(childDim => {
           childDim.getDataType.getName.toLowerCase match {
    -        case "array" => s"${
    +        case "array" => if (table.isTransactionalTable) {s"${
    --- End diff --
   
    Written the comment of why transactional and no transactional behaviour will be different.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2209#discussion_r185217438
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/SparkTypeConverter.scala ---
    @@ -123,13 +137,29 @@ private[spark] object SparkTypeConverter {
       private def recursiveMethod(
           table: CarbonTable, dimName: String, childDim: CarbonDimension) = {
         childDim.getDataType.getName.toLowerCase match {
    -      case "array" => s"${
    -        childDim.getColName.substring(dimName.length + 1)
    -      }:array<${ getArrayChildren(table, childDim.getColName) }>"
    -      case "struct" => s"${
    -        childDim.getColName.substring(dimName.length + 1)
    -      }:struct<${ getStructChildren(table, childDim.getColName) }>"
    -      case dType => s"${ childDim.getColName.substring(dimName.length + 1) }:${ dType }"
    +      case "array" => if (table.isTransactionalTable) {
    --- End diff --
   
    Or else we can make the behavior of transactional or non transactional same.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2209
 
    @sounakr Please fix the build


---