[GitHub] carbondata pull request #1732: [WIP] Fixed select query failure after alter ...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1732
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1245/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1732
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1252/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1732#discussion_r159461116
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/DataTypeUtil.java ---
    @@ -775,4 +775,25 @@ public static DataType valueOf(String name) {
         }
       }
     
    -}
    \ No newline at end of file
    +  /**
    +   * Method to type case the data based on modified data type. This method will used for
    +   * retrieving the data after change in data type restructure operation
    +   *
    +   * @param data
    +   * @param restructuredDataType
    +   * @param currentDataOffset
    +   * @param length
    +   * @return
    +   */
    +  public static long getDataBasedOnRestructuredDataType(byte[] data, DataType restructuredDataType,
    --- End diff --
   
    This case won't exist from short to int or float to double?. Here I can see handling only for INT to LONG


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1732#discussion_r159461589
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/ChangeDataTypeTestCases.scala ---
    @@ -164,6 +164,21 @@ class ChangeDataTypeTestCases extends Spark2QueryTest with BeforeAndAfterAll {
         sql("drop table if exists PreAggMain_preagg1")
       }
     
    +  test("test data type change for dictionary exclude INT type column") {
    --- End diff --
   
    PLease add tests for short to int, short to long and float to double also


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1732#discussion_r159585172
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/DataTypeUtil.java ---
    @@ -775,4 +775,25 @@ public static DataType valueOf(String name) {
         }
       }
     
    -}
    \ No newline at end of file
    +  /**
    +   * Method to type case the data based on modified data type. This method will used for
    +   * retrieving the data after change in data type restructure operation
    +   *
    +   * @param data
    +   * @param restructuredDataType
    +   * @param currentDataOffset
    +   * @param length
    +   * @return
    +   */
    +  public static long getDataBasedOnRestructuredDataType(byte[] data, DataType restructuredDataType,
    --- End diff --
   
    yes because alter datatype change is only for Int to BigInt and decimal from lower precision to higher precision. So float to double case is not required to be handled


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1732#discussion_r159585515
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/restructure/vectorreader/ChangeDataTypeTestCases.scala ---
    @@ -164,6 +164,21 @@ class ChangeDataTypeTestCases extends Spark2QueryTest with BeforeAndAfterAll {
         sql("drop table if exists PreAggMain_preagg1")
       }
     
    +  test("test data type change for dictionary exclude INT type column") {
    --- End diff --
   
    Alter table datatype change does not support short to int, short to long and float to double. It only supports Int to BigInt and decimal from lower precision to higher precision. If any other data type change request comes validation is done and exception is thrown


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1732#discussion_r159589346
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/result/vector/CarbonColumnVector.java ---
    @@ -67,6 +67,20 @@
     
       DataType getType();
     
    +  /**
    +   * Method to be used for getting the restructured data type. This method will used for
    +   * retrieving the data after change in data type restructure operation
    +   *
    +   * @return
    +   */
    +  DataType getRestructuredType();
    --- End diff --
   
    Better rename as BlockDataType to give more meaning ful


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---