[GitHub] carbondata pull request #2708: [CARBONDATA-2886] Select Filter Comppatibilit...

[qiuchenjian-2]

GitHub user manishnalla1994 opened a pull request:

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

    [CARBONDATA-2886] Select Filter Comppatibility Fixed

    Problem : Select Filter Query with INT data type was showing incorrect result in case of table creation and loading on old version and queried on new version. The min max values in case of legacy table were not being updated properly so the check inside the blocklet was not happening.
    Solution : Correctly updated the min max values.
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [x] Testing done
           Manually tested.
     - [ ] 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/manishnalla1994/carbondata compatibilty_fix

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

    https://github.com/apache/carbondata/pull/2708.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 #2708
   
----
commit 234d159aea1ada889fec0424a8370f4564f90f6e
Author: Manish Nalla <manishnalla1994@...>
Date:   2018-09-11T08:38:33Z

    Select Filter Comppatibility Fixed

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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


---

[qiuchenjian-2]

Github user brijoobopanna commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2708
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/246/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2708#discussion_r216889572
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/AbstractQueryExecutor.java ---
    @@ -383,8 +383,9 @@ private void fillBlockletInfoToTableBlock(List<TableBlockInfo> tableBlockInfos,
               blockletInfo.getBlockletIndex().getMinMaxIndex().getMaxValues());
           // update min and max values in case of old store for measures as min and max is written
           // opposite for measures in old store ( store <= 1.1 version)
    +      byte[][] tempMaxValues = maxValues;
           maxValues = CarbonUtil.updateMinMaxValues(fileFooter, maxValues, minValues, false);
    -      minValues = CarbonUtil.updateMinMaxValues(fileFooter, maxValues, minValues, true);
    +      minValues = CarbonUtil.updateMinMaxValues(fileFooter, tempMaxValues, minValues, true);
    --- End diff --
   
    Is there any difference?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2708#discussion_r216908025
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/AbstractQueryExecutor.java ---
    @@ -383,8 +383,9 @@ private void fillBlockletInfoToTableBlock(List<TableBlockInfo> tableBlockInfos,
               blockletInfo.getBlockletIndex().getMinMaxIndex().getMaxValues());
           // update min and max values in case of old store for measures as min and max is written
           // opposite for measures in old store ( store <= 1.1 version)
    +      byte[][] tempMaxValues = maxValues;
           maxValues = CarbonUtil.updateMinMaxValues(fileFooter, maxValues, minValues, false);
    -      minValues = CarbonUtil.updateMinMaxValues(fileFooter, maxValues, minValues, true);
    +      minValues = CarbonUtil.updateMinMaxValues(fileFooter, tempMaxValues, minValues, true);
    --- End diff --
   
    The maxValues are changed in line 386 and we need the old values of maxValues to compute the correct minValues. So stored it in a temporary variable first.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2708
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/415/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2708
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8485/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---