[GitHub] carbondata pull request #1417: [WIP] Add precision and scale to DecimalType

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1417#discussion_r147639115
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/column/ColumnSchema.java ---
    @@ -87,13 +88,6 @@
        */
       private int columnGroupId = -1;
     
    -  /**
    -   * Used when this column contains decimal data.
    -   */
    -  private int scale;
    -
    -  private int precision;
    --- End diff --
   
    I think there would be an issue while reading gson if we remove these fields. we will lose the data of it if we remove them. Better keep them for backward compatibility.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1417#discussion_r147664880
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/UnsafeVarLengthColumnPage.java ---
    @@ -42,6 +42,16 @@
         baseOffset = memoryBlock.getBaseOffset();
       }
     
    +  UnsafeVarLengthColumnPage(TableSpec.ColumnSpec columnSpec, DataType dataType, int pageSize,
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1417#discussion_r147670083
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/column/ColumnSchema.java ---
    @@ -87,13 +88,6 @@
        */
       private int columnGroupId = -1;
     
    -  /**
    -   * Used when this column contains decimal data.
    -   */
    -  private int scale;
    -
    -  private int precision;
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1417#discussion_r147670204
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
    @@ -1862,9 +1863,31 @@ public static TableInfo convertGsonToTableInfo(Map<String, String> properties) {
           builder.append(part);
         }
         TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class);
    +    updateDecimalType(tableInfo);
         return tableInfo;
       }
     
    +  // update decimal type inside `tableInfo` to set scale and precision, if there are any decimal
    +  private static void updateDecimalType(TableInfo tableInfo) {
    +    List<ColumnSchema> deserializedColumns = tableInfo.getFactTable().getListOfColumns();
    +    for (ColumnSchema column : deserializedColumns) {
    +      DataType dataType = column.getDataType();
    +      if (DataTypes.isDecimal(dataType)) {
    +        column.setDataType(DataTypes.createDefaultDecimalType());
    --- End diff --
   
    ok, I will add back scale and precision in `ColumnSchema` and use it here


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1417
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/1417


---