PDGGK opened a new pull request, #9408:
URL: https://github.com/apache/paimon/pull/9408
### Purpose
`PARTITION_DYNAMIC` picks a fallback start channel for a partition it has no
statistics for:
```java
int startChannel = Math.abs(partitionKey.hashCode()) % numChannels;
```
`Math.abs(Integer.MIN_VALUE)` is `Integer.MIN_VALUE`, so this can be
negative, and every channel built from it inherits the sign:
```java
subtasks.add((startChannel + i) % numChannels);
```
`ChannelComputer.startChannel` already guards this — it was fixed in #5623
with a comment spelling out this exact case — and
`StatisticsOrRecordChannelComputer` implements `ChannelComputer`, so this just
calls the shared helper instead of repeating the formula without the guard.
Worth noting the failure is parallelism-dependent, which is presumably why
it has not been hit yet: `Integer.MIN_VALUE % n` is zero whenever `n` divides
2^31, so at parallelism 2, 4, 8 or 16 the bad hash happens to come out at
channel 0. At parallelism 3 the same partition returns channel `-1`.
### Tests
`testUnknownPartitionWhoseKeyHashesToMinValue` in
`StatisticsOrRecordChannelComputerTest`. Partition value `p3588823253` is a
real value whose partition key hashes to `Integer.MIN_VALUE`; the test asserts
that up front, so if the row hash ever changes the test fails rather than
quietly stopping to exercise the path. It then checks the returned channel is
in range for parallelism 1 through 16.
Without the change the test fails at parallelism 3:
```
[parallelism 3]
Expecting actual:
-1
to be between:
[0, 2]
```
Parallelism 8, which the existing tests use, passes either way.
`mvn test -pl paimon-flink/paimon-flink-common
-Dtest='org.apache.paimon.flink.sink.partition.**'` — 19 tests, and
`-Dtest='*ChannelComputer*'` — 12 tests, all passing.
--
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]
