voonhous commented on code in PR #19776:
URL: https://github.com/apache/hudi/pull/19776#discussion_r3901996726
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/client/CoalescingPartitioner.java:
##########
@@ -41,7 +41,9 @@ public int getPartition(Object key) {
if (numPartitions == 1) {
return 0;
} else {
- return Math.abs(key.hashCode()) % numPartitions;
+ // Math.abs leaves Integer.MIN_VALUE negative, and a Partitioner must
answer in
+ // [0, numPartitions). floorMod is non-negative for every input.
+ return Math.floorMod(key.hashCode(), numPartitions);
Review Comment:
Body now states the reroute explicitly, with the per-parallelism numbers.
floorMod kept.
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/execution/bulkinsert/PartitionPathRDDPartitioner.java:
##########
@@ -47,6 +47,8 @@ public int numPartitions() {
@SuppressWarnings("unchecked")
@Override
public int getPartition(Object o) {
- return Math.abs(Objects.hash(partitionPathExtractor.apply(o))) %
numPartitions;
+ // Math.abs leaves Integer.MIN_VALUE negative, and a Partitioner must
answer in
+ // [0, numPartitions). floorMod is non-negative for every input.
+ return Math.floorMod(Objects.hash(partitionPathExtractor.apply(o)),
numPartitions);
Review Comment:
Impact section now covers the three bulk-insert callers and why a
deterministic reroute is safe for them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
