Ensure partition function never return negative partition #8221
Conversation
| @@ -57,4 +56,8 @@ public int getNumPartitions() { | |||
| public String toString() { | |||
| return NAME; | |||
| } | |||
|
|
|||
| private int abs(int n) { | |||
| return (n == Integer.MIN_VALUE) ? 0 : Math.abs(n); | |||
There was a problem hiding this comment.
this is equivalent to Math.abs(n) & Integer.MAX_VALUE but the callers could just move the modulo inside the Math.abs to avoid dealing with Integer.MIN_VALUE
There was a problem hiding this comment.
Sounds good. This is the same as ByteArrayPartitionFunction, so I'll also change that
Jackie-Jiang
force-pushed
the
partition_function_negative_handling
branch
from
e843754 to
b316b73
Compare
Codecov Report
@@ Coverage Diff @@
## master #8221 +/- ##
============================================
+ Coverage 71.01% 71.04% +0.02%
+ Complexity 4320 4319 -1
============================================
Files 1626 1626
Lines 85067 85067
Branches 12799 12799
============================================
+ Hits 60408 60433 +25
+ Misses 20505 20474 -31
- Partials 4154 4160 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
...ent-spi/src/main/java/org/apache/pinot/segment/spi/partition/ByteArrayPartitionFunction.java
Outdated
Show resolved
Hide resolved
Jackie-Jiang
force-pushed
the
partition_function_negative_handling
branch
from
b316b73 to
3ca6b92
Compare
|
If this is a backward incompatible change (especially for |
|
@snleee This is not. For modulo, it only fixes the partition if it is negative before, which is not working anyway. |
There are some corner cases not handled in partition function which can lead to negative partition: - In HashCodePartitionFunction when hashcode is Integer.MIN_VALUE - In ModuloPartitionFunction when value is negative Also enhance the test to ensure passing number and string returns the same result
There are some corner cases not handled in partition function which can lead to negative partition:
Also enhance the test to ensure passing number and string returns the same result