You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by ravipesala <gi...@git.apache.org> on 2018/08/07 15:54:33 UTC
[GitHub] carbondata pull request #2616: [HOTFIX]Fixed int overflow and comparison gon...
GitHub user ravipesala opened a pull request:
https://github.com/apache/carbondata/pull/2616
[HOTFIX]Fixed int overflow and comparison gone wrong during blocklet min/max
Problem: During calculating min/max for blocklet, it needs to calculate from all the pages. During that comparison, it is typecasting to int and overflows, so there is a chance the negative becomes positive and positive become negative. That's why min max of long comes wrong for bigger values.
Solution: Don't typecast directly, instead check first the negative or positive and then return.
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
- [ ] 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 blocklet-min-max-issue
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2616.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 #2616
----
commit a99d0516ee06a285f8be3880d7005bf161f1c940
Author: ravipesala <ra...@...>
Date: 2018-08-07T15:49:36Z
Fixed int overflow and comparasion gone wrong during blocklet min/max
----
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2616
LGTM
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on the issue:
https://github.com/apache/carbondata/pull/2616
LGTM
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7829/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2616
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6203/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6564/
---
[GitHub] carbondata pull request #2616: [HOTFIX]Fixed int overflow and comparison gon...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2616#discussion_r208569907
--- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java ---
@@ -368,7 +368,13 @@ private static int compareMeasureData(byte[] first, byte[] second, DataType data
secondBuffer.put(second);
firstBuffer.flip();
secondBuffer.flip();
- return (int) (firstBuffer.getDouble() - secondBuffer.getDouble());
+ long compare = firstBuffer.getLong() - secondBuffer.getLong();
--- End diff --
:)
---
[GitHub] carbondata pull request #2616: [HOTFIX]Fixed int overflow and comparison gon...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2616#discussion_r208556581
--- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java ---
@@ -368,7 +368,13 @@ private static int compareMeasureData(byte[] first, byte[] second, DataType data
secondBuffer.put(second);
firstBuffer.flip();
secondBuffer.flip();
- return (int) (firstBuffer.getDouble() - secondBuffer.getDouble());
+ long compare = firstBuffer.getLong() - secondBuffer.getLong();
--- End diff --
Yes , its a copy paste mistake :)
---
[GitHub] carbondata pull request #2616: [HOTFIX]Fixed int overflow and comparison gon...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2616#discussion_r208556245
--- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java ---
@@ -368,7 +368,13 @@ private static int compareMeasureData(byte[] first, byte[] second, DataType data
secondBuffer.put(second);
firstBuffer.flip();
secondBuffer.flip();
- return (int) (firstBuffer.getDouble() - secondBuffer.getDouble());
+ long compare = firstBuffer.getLong() - secondBuffer.getLong();
--- End diff --
For double type we should call getDouble
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2616
LGTM
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2616
LGTM
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7840/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6579/
---
[GitHub] carbondata pull request #2616: [HOTFIX]Fixed int overflow and comparison gon...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala closed the pull request at:
https://github.com/apache/carbondata/pull/2616
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2616
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6213/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2616
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6223/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7854/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2616
retest this please
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6554/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6568/
---
[GitHub] carbondata issue #2616: [HOTFIX]Fixed int overflow and comparison gone wrong...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2616
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7843/
---