[GitHub] carbondata pull request #1125: [CarbonData-1250] change default partition id...

[qiuchenjian-2]

GitHub user lionelcao opened a pull request:

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

    [CarbonData-1250] change default partition id & Add TaskId group in partitionInfo

    This PR change the default partition id from Max to 0 and add taskId management related code to support later add(split)/drop(merge) partition operations.
   
    1. change default partition id from Max to 0
    2. add taskIdInPartitionOrder in partitionInfo to manage the taskId to partition map
    3. remove some useless file
    4. extract some common used function to PartitionUtils
    5. change CarbonInputFormat to use correct taskId
    6. modify test case according to the taskId change.
    7. support range info in ascending order validation
    8. fix some typo

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

    $ git pull https://github.com/lionelcao/carbondata carbon_910_13

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

    https://github.com/apache/carbondata/pull/1125.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 #1125
   
----
commit 83d134919b8962ac3d6269f7c910666527c3a4c5
Author: lionelcao <[hidden email]>
Date:   2017-06-29T16:00:32Z

    [CARBONDATA-1250] change default partition id from Max to 0

commit 5fb7447cc03b8204e98e90129037eee828912bf8
Author: lionelcao <[hidden email]>
Date:   2017-06-29T16:06:30Z

    extract common function

commit fd928d8476b0907dd1cb0347803775b495fe5f74
Author: lionelcao <[hidden email]>
Date:   2017-06-30T15:05:06Z

    add taskid group in partition info and change query filter code
   
    remove partition statistics for now
   
    fix style error
   
    empty commit
   
    add taskid group in partitionInfo
   
    remove unrelated code
   
    fix style error
   
    fix style error
   
    fix bug: getSplits error for non-partition table
   
    change import order
   
    remove unrelated code and test CI
   
    change import order
   
    change query filter code according to default partition id changing
   
    thrift converter
   
    remove useless comment
   
    remove unrelated code

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2870/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/287/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125301950
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/converter/ThriftWrapperSchemaConverterImpl.java ---
    @@ -219,6 +219,10 @@
         externalPartitionInfo.setList_info(wrapperPartitionInfo.getListInfo());
         externalPartitionInfo.setRange_info(wrapperPartitionInfo.getRangeInfo());
         externalPartitionInfo.setNum_partitions(wrapperPartitionInfo.getNumPartitions());
    +    externalPartitionInfo.setNumOfPartitions(wrapperPartitionInfo.getNumberOfPartitions());
    +    externalPartitionInfo.setMax_partition(wrapperPartitionInfo.getMAX_PARTITION());
    +    externalPartitionInfo.setTaskId_In_PartitionOrder(wrapperPartitionInfo
    --- End diff --
   
    TaskIdInPartitionOrder is an array used to ensure taskId could mapping to correct partition after alter table partition.
    It will be used when alter partition table. (The operation method will be uploaded with alter table PR)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125304887
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/partition/PartitionFilterUtil.java ---
    @@ -229,35 +229,37 @@ public static BitSet getPartitionMapForRangeFilter(PartitionInfo partitionInfo,
           // filter value is in default partition
           if (isGreaterThan) {
             // GreaterThan(>), GreaterThanEqualTo(>=)
    -        partitionMap.set(numPartitions);
    +        partitionMap.set(0);
           } else {
             // LessThan(<), LessThanEqualTo(<=)
    -        partitionMap.set(0, partitioner.numPartitions());
    +        partitionMap.set(1, partitioner.numPartitions());
           }
         } else {
           // filter value is not in default partition
           if (result == 0) {
             // if result is 0, the filter value is a bound value of range partition.
             if (isGreaterThan) {
               // GreaterThan(>), GreaterThanEqualTo(>=)
    -          partitionMap.set(partitionIndex + 1, partitioner.numPartitions());
    +          partitionMap.set(partitionIndex + 1, partitioner.numPartitions() - 1);
    --- End diff --
   
    partitionMap.set(partitionIndex + 2, partitioner.numPartitions() - 1);


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125301153
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/converter/ThriftWrapperSchemaConverterImpl.java ---
    @@ -219,6 +219,10 @@
         externalPartitionInfo.setList_info(wrapperPartitionInfo.getListInfo());
         externalPartitionInfo.setRange_info(wrapperPartitionInfo.getRangeInfo());
         externalPartitionInfo.setNum_partitions(wrapperPartitionInfo.getNumPartitions());
    +    externalPartitionInfo.setNumOfPartitions(wrapperPartitionInfo.getNumberOfPartitions());
    --- End diff --
   
    NumPartitions is definition for Hash partition table.
    NumOfPartitions is total partition count of partition table(according to schema, not fact data), it's defined by the length of range info/list info and hash partition number.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/295/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2879/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125319466
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/partition/RangePartitioner.java ---
    @@ -17,16 +17,17 @@
     
     package org.apache.carbondata.core.scan.partition;
     
    -import java.io.Serializable;
    -import java.math.BigDecimal;
     import java.text.SimpleDateFormat;
     import java.util.List;
     
     import org.apache.carbondata.core.constants.CarbonCommonConstants;
     import org.apache.carbondata.core.metadata.datatype.DataType;
     import org.apache.carbondata.core.metadata.schema.PartitionInfo;
     import org.apache.carbondata.core.util.ByteUtil;
    +
    --- End diff --
   
    DELETE


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/302/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2886/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2887/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/303/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125427240
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/converter/ThriftWrapperSchemaConverterImpl.java ---
    @@ -219,6 +219,10 @@
         externalPartitionInfo.setList_info(wrapperPartitionInfo.getListInfo());
         externalPartitionInfo.setRange_info(wrapperPartitionInfo.getRangeInfo());
         externalPartitionInfo.setNum_partitions(wrapperPartitionInfo.getNumPartitions());
    +    externalPartitionInfo.setNumOfPartitions(wrapperPartitionInfo.getNumberOfPartitions());
    --- End diff --
   
    I think it may be better that use Hash_numPartition. otherwise users may confused about this two num of partitions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125427623
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ---
    @@ -99,10 +98,6 @@
       private Map<String, PartitionInfo> tablePartitionMap;
     
       /**
    -   * statistic information of partition table
    -   */
    -  private PartitionStatistic partitionStatistic;
    -  /**
    --- End diff --
   
    Should keep this line.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1125#discussion_r125428800
 
    --- Diff: format/src/main/thrift/schema.thrift ---
    @@ -135,6 +135,9 @@ struct PartitionInfo{
         3: optional i32 num_partitions;  // number of partitions defined in hash partition table
    --- End diff --
   
    Same as Hash_num_parititions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/315/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1125
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2901/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---