[GitHub] incubator-carbondata pull request #551: [CARBONDATA-603] Fixed Date Filter i...

[qiuchenjian-2]

GitHub user kumarvishal09 opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/551

    [CARBONDATA-603] Fixed Date Filter issue

    **Problem**
    select query with WHERE clause for date data type is failing, date values are coming as NULL
    **Solution**
    Currently both timestamp and date has same property need to add separate property for date format

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

    $ git pull https://github.com/kumarvishal09/incubator-carbondata FixedDateFilterIssue

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

    https://github.com/apache/incubator-carbondata/pull/551.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 #551
   
----
commit 8d1ca417dfd11106295d69fd71c0d9a577c436a7
Author: kumarvishal <[hidden email]>
Date:   2017-01-11T12:29:31Z

    Date filter fix issue-CARBONDATA-603

----


---
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/incubator-carbondata/pull/551
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/666/



---
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/incubator-carbondata/pull/551
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/668/



---
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/incubator-carbondata/pull/551
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilderSpark2.1/3/



---
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/incubator-carbondata/pull/551
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/680/



---
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/incubator-carbondata/pull/551
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/685/



---
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/incubator-carbondata/pull/551
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/686/



---
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/incubator-carbondata/pull/551
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/691/



---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96883596
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java ---
    @@ -285,9 +285,17 @@ private int compareFilterKeyWithDictionaryKey(String dictionaryVal, String membe
                   .compare((Boolean.parseBoolean(dictionaryVal)), (Boolean.parseBoolean(memberVal)));
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    why if else is required? please keep in the DATE case.


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96883829
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/DirectDictionaryKeyGeneratorFactory.java ---
    @@ -57,9 +57,16 @@ public static DirectDictionaryGenerator getDirectDictionaryGenerator(DataType da
       }
     
       public static DirectDictionaryGenerator getDirectDictionaryGenerator(DataType dataType) {
    -    return getDirectDictionaryGenerator(dataType,
    -        CarbonProperties.getInstance()
    -            .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +    String format= null;
    --- End diff --
   
    Please use switch case


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96884908
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/expression/ExpressionResult.java ---
    @@ -175,9 +175,17 @@ public String getString() throws FilterIllegalMemberException {
           switch (this.getDataType()) {
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    instead of using if else, please use inside case and use break;


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96884951
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
    @@ -35,11 +37,16 @@
     
       static final int cutOffDate = Integer.MAX_VALUE >> 1;
    --- End diff --
   
    Please remove unused cutOffDate


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96885076
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/expression/ExpressionResult.java ---
    @@ -523,9 +531,17 @@ public boolean isNull() {
               return val1.compareTo(val2);
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    instead of using if else, please use inside case and use break;


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96885139
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/FilterUtil.java ---
    @@ -1078,9 +1078,17 @@ public static int compareFilterKeyBasedOnDataType(String dictionaryVal, String m
                   .compare((Boolean.parseBoolean(dictionaryVal)), (Boolean.parseBoolean(memberVal)));
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    instead of using if else, please use inside case and use break;


---
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 ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96885350
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/writer/sortindex/CarbonDictionarySortModel.java ---
    @@ -102,9 +102,17 @@ public CarbonDictionarySortModel(int key, DataType dataType, String memberValue)
             return val1.compareTo(val2);
           case DATE:
           case TIMESTAMP:
    -        SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -            .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +        String format= null;
    +        if(dataType==DataType.DATE) {
    --- End diff --
   
    instead of using if else, please use inside case and use break;


---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96887076
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/timestamp/DateDirectDictionaryGenerator.java ---
    @@ -35,11 +37,16 @@
     
       static final int cutOffDate = Integer.MAX_VALUE >> 1;
    --- End diff --
   
    ok


---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96887094
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/expression/ExpressionResult.java ---
    @@ -175,9 +175,17 @@ public String getString() throws FilterIllegalMemberException {
           switch (this.getDataType()) {
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    ok


---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96887109
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/directdictionary/DirectDictionaryKeyGeneratorFactory.java ---
    @@ -57,9 +57,16 @@ public static DirectDictionaryGenerator getDirectDictionaryGenerator(DataType da
       }
     
       public static DirectDictionaryGenerator getDirectDictionaryGenerator(DataType dataType) {
    -    return getDirectDictionaryGenerator(dataType,
    -        CarbonProperties.getInstance()
    -            .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +    String format= null;
    --- End diff --
   
    ok


---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/551#discussion_r96887126
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/cache/dictionary/ColumnDictionaryInfo.java ---
    @@ -285,9 +285,17 @@ private int compareFilterKeyWithDictionaryKey(String dictionaryVal, String membe
                   .compare((Boolean.parseBoolean(dictionaryVal)), (Boolean.parseBoolean(memberVal)));
             case DATE:
             case TIMESTAMP:
    -          SimpleDateFormat parser = new SimpleDateFormat(CarbonProperties.getInstance()
    -              .getProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT,
    -                  CarbonCommonConstants.CARBON_TIMESTAMP_DEFAULT_FORMAT));
    +          String format= null;
    +          if(dataType==DataType.DATE) {
    --- End diff --
   
    ok



---
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/incubator-carbondata/pull/551
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/708/



---
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.
---