ARROW-5844: [Java] Support comparison & sort for more numeric types #4799
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emkornfield
reviewed
Jul 5, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
emkornfield
reviewed
Jul 5, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
emkornfield
reviewed
Jul 5, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #4799 +/- ##
==========================================
+ Coverage 87.44% 89.59% +2.15%
==========================================
Files 997 662 -335
Lines 139728 96351 -43377
Branches 1418 0 -1418
==========================================
- Hits 122181 86327 -35854
+ Misses 17185 10024 -7161
+ Partials 362 0 -362
Continue to review full report at Codecov.
|
emkornfield
reviewed
Jul 9, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
emkornfield
reviewed
Jul 9, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
emkornfield
reviewed
Jul 9, 2019
java/algorithm/src/main/java/org/apache/arrow/algorithm/sort/DefaultVectorComparators.java
Outdated
Show resolved
Hide resolved
emkornfield
reviewed
Jul 10, 2019
| } | ||
| } | ||
|
|
||
| float result = Math.signum(value1 - value2); |
There was a problem hiding this comment.
it seems like you should either directly return the casted signum or use the if statement, not both?
There was a problem hiding this comment.
Sure. Thanks a lot.
|
+1, LGTM. |
kszucs
pushed a commit
that referenced
this pull request
Jul 22, 2019
Currently, we only support comparison & sort for 32-bit integers, in this issue, we provide support for more numeric data types: byte short long float double Author: liyafan82 <[email protected]> Closes #4799 from liyafan82/fly_0704_cmp and squashes the following commits: a921cdd <liyafan82> Remove if conditons in default float & double comparators a4b4099 <liyafan82> Replace if condition with signum function 30f946b <liyafan82> Merge branch 'master' into fly_0704_cmp bc880f1 <liyafan82> Merge branch 'master' into fly_0704_cmp 7cbe556 <liyafan82> Support NaN for float and double 3860c11 <liyafan82> Support comparison & sort for more numeric types
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, we only support comparison & sort for 32-bit integers, in this issue, we provide support for more numeric data types:
byte
short
long
float
double