nzw921rx commented on PR #11987:
URL: https://github.com/apache/seatunnel/pull/11987#issuecomment-5439185996
Thank you for your response. I would like to clarify my perspective.
1. Readability is being traded for a lower risk of repeating bugs that the
project has encountered before
The original problem was caused by an oversight during code review, and
introducing a utility method alone cannot prevent the same issue from
recurring. New connectors may still choose not to use this utility unless we
establish it as a project-wide convention, which we currently have not done.
More importantly, the current `HashUtils` does not cover all existing
hash-partitioning variants. As long as exceptions remain, we cannot require
contributors to use this utility as the single correct implementation for
hash-based partitioning. The codebase would still contain fragmented approaches.
2. This is not a matter of personal preference
We are discussing how to reach a more reasonable implementation together. My
PR comments are intended to explore a better direction, not to reject an
otherwise correct and bug-free implementation based on personal preference.
The next question is how this PR can address the fragmentation described in
point 1.
I think the utility can support multiple explicit strategies to preserve the
behavior of existing implementations. For special legacy algorithms, we could
keep the scope limited and mark the corresponding methods with `@Deprecated` to
indicate that they may be removed in the future.
With this approach, the utility could cover the remaining cases without
silently changing existing partition assignments.
examples:
```java
public final class HashUtils {
private HashUtils() {}
public static int bucketIndex(Object key, int bucketCount) {
return bucketIndex(Objects.requireNonNull(key, "key").hashCode(),
bucketCount);
}
public static int bucketIndex(int hash, int bucketCount) {
checkBucketCount(bucketCount);
return (hash & Integer.MAX_VALUE) % bucketCount;
}
public static int bucketIndex(long hash, int bucketCount) {
checkBucketCount(bucketCount);
return (int) ((hash & Long.MAX_VALUE) % bucketCount);
}
private static void checkBucketCount(int bucketCount) {
if (bucketCount <= 0) {
throw new IllegalArgumentException(
"bucketCount must be greater than zero, but was " +
bucketCount);
}
}
}
```
I look forward to your reply, thank you🚀
--
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]
