PDGGK opened a new pull request, #19776: URL: https://github.com/apache/hudi/pull/19776
### Describe the issue this Pull Request addresses Two Spark `Partitioner` implementations derive the partition index as `Math.abs(hash) % numPartitions`: ```java // CoalescingPartitioner:44 return Math.abs(key.hashCode()) % numPartitions; // PartitionPathRDDPartitioner:50 return Math.abs(Objects.hash(partitionPathExtractor.apply(o))) % numPartitions; ``` `Math.abs(Integer.MIN_VALUE)` is `Integer.MIN_VALUE`, so the expression stays negative whenever `numPartitions` does not divide 2^31 — that is, for every parallelism that is not a power of two. `Partitioner#getPartition` has to answer inside `[0, numPartitions)`. Both are reachable from ordinary data: | partitioner | input | 2 | 3 | 4 | 5 | 7 | 8 | |---|---|---|---|---|---|---|---| | `CoalescingPartitioner` | key `"polygenelubricants"` (`hashCode` is `Integer.MIN_VALUE`) | 0 | **-2** | 0 | **-3** | **-2** | 0 | | `PartitionPathRDDPartitioner` | partition path `"xfjfxsf"` | 0 | **-2** | 0 | **-3** | **-2** | 0 | `Objects.hash(x)` is `31 + x.hashCode()`, so the second one needs a partition path hashing to `2147483617` for the sum to overflow to `Integer.MIN_VALUE`; `"xfjfxsf"` is such a value. ### Summary and Changelog Both now use `Math.floorMod`, which is non-negative for every input and agrees with the old expression on every hash the old one already handled correctly — only the `Integer.MIN_VALUE` case changes, and there the old answer was not a usable partition index. `BucketIndexUtil` (`(partition.hashCode() & Integer.MAX_VALUE) % parallelism`) and `JavaUpsertPartitioner` (`Math.floorMod`) already avoid `Math.abs` for the same reason. Tests: - `TestCoalescingPartitioner#testPartitionIsInRangeForMinValueHash` — added to the existing class; asserts the fixture still hashes to `Integer.MIN_VALUE` first, so it cannot silently stop exercising the case, then checks the index is in range for 1..16 partitions. - `TestPartitionPathRDDPartitioner` — new, same shape, asserting `Objects.hash` still overflows for the fixture. Reverting the change turns them red with `partition -2 out of range for numPartitions 3` and the equivalent for 5, 6 and 7; the power-of-two parallelisms stay green either way, which is why this has not been hit before. `mvn test -pl hudi-client/hudi-spark-client -Dtest='*Partitioner*'` — 59 tests, all passing. `checkstyle:check` clean. ### Impact No public API or config change. Records whose key hashes to `Integer.MIN_VALUE` now land on a valid partition instead of failing the write; every other key routes exactly as before at power-of-two parallelism, and to a different but valid partition otherwise. ### Risk Level low ### Documentation Update none ### Contributor's checklist - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [x] Enough context is provided in the sections above - [x] Adequate tests were added if applicable -- 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]
