[GitHub] carbondata pull request #1759: [CARBONDATA-1980][PARTITION]Fix adding partit...

[qiuchenjian-2]

GitHub user ravipesala opened a pull request:

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

    [CARBONDATA-1980][PARTITION]Fix adding partition information while doing refresh(restore) table.And fix the case sensitivity issue of partition column

    Problem
    While restoring the table the partitions are not added to the hive.  And also if any uppercase letters are present in partition column then filter queries on that column gives error.
   
    Solution
    Read all partitionmap files of all segments to add back the partition information to the hive metatstore.
    And always update the partition filter columns to lower case before querying partition information.
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [X] Any interfaces changed? NO
     
     - [X] Any backward compatibility impacted? NO
     
     - [X] Document update required? NO
   
     - [X] Testing done
           Tests added
           
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ravipesala/incubator-carbondata restore-partition

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1759.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1759
   
----
commit 20e411487a11c2219e08e966ff83c6cc0e718678
Author: ravipesala <ravi.pesala@...>
Date:   2018-01-03T19:04:33Z

    Fix adding partition information while doing refresh(restore) table. And fix the case sensitivity issue of partition column

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1759
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1300/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1759#discussion_r159589011
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala ---
    @@ -91,6 +95,11 @@ case class RefreshCarbonTableCommand(
               registerAggregates(databaseName, dataMapSchemaList)(sparkSession)
             }
             registerTableWithHive(databaseName, tableName, tableInfo)(sparkSession)
    +        // Register partitions to hive metastore in case of hive partitioning carbon table
    +        if (tableInfo.getFactTable.getPartitionInfo != null &&
    +            tableInfo.getFactTable.getPartitionInfo.getPartitionType == PartitionType.NATIVE_HIVE) {
    +          registerAllPartitionsToHive(tablePath, databaseName, tableName, sparkSession)
    --- End diff --
   
    I think passing AbsolutionTableIdentifier is better, it has all information. It is created in line 65


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/1759
 
    LGTM after fixing comment and rebasing


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1759#discussion_r159669950
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala ---
    @@ -91,6 +95,11 @@ case class RefreshCarbonTableCommand(
               registerAggregates(databaseName, dataMapSchemaList)(sparkSession)
             }
             registerTableWithHive(databaseName, tableName, tableInfo)(sparkSession)
    +        // Register partitions to hive metastore in case of hive partitioning carbon table
    +        if (tableInfo.getFactTable.getPartitionInfo != null &&
    +            tableInfo.getFactTable.getPartitionInfo.getPartitionType == PartitionType.NATIVE_HIVE) {
    +          registerAllPartitionsToHive(tablePath, databaseName, tableName, sparkSession)
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1759
 
    retest sdv please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1759
 
    Already merged


---