SEPURI-SAI-KRISHNA opened a new pull request, #11987:
URL: https://github.com/apache/seatunnel/pull/11987
### Purpose of this pull request
Closes #11976.
Hash-to-bucket routing is currently spelled three different ways across the
codebase. This PR introduces a single helper and migrates the call sites that
genuinely perform hash-to-bucket routing.
Before this PR:
| Spelling | Sites |
| --- | --- |
| `(h & Integer.MAX_VALUE) % n` | 9 |
| `((h * 31) & 0x7FFFFFFF) % n` | 3 |
| `Math.abs(h) % n` | 1 (fixed separately in #11721) |
The new helper lives in `seatunnel-common`:
```java
public static int nonNegativeMod(int hash, int bucketCount) {
if (bucketCount <= 0) {
throw new IllegalArgumentException(
"bucketCount must be greater than zero, but was " +
bucketCount);
}
return (hash & Integer.MAX_VALUE) % bucketCount;
}
```
`seatunnel-common` is already on the compile classpath of every module
touched here, so no `pom.xml` changes were needed.
### Behaviour is bit-for-bit unchanged
Every migrated site keeps the exact masking semantics it already had.
The three `((h * 31) & 0x7FFFFFFF) % n` sites become `nonNegativeMod(h * 31,
n)`. `0x7FFFFFFF` and `Integer.MAX_VALUE` are the same constant, and the `* 31`
is part of the hash rather than part of the bucketing, so the multiply moves
inside the call unchanged.
Deliberately **not** switched to `Math.floorMod`. Masking and `floorMod`
agree only when `bucketCount` is a power of two, because masking adds `2^31` to
a negative hash instead of preserving its magnitude. For `hash = -5` and
`bucketCount = 3`, masking yields `0` and `floorMod` yields `1`. Standardising
on `floorMod` would look like a rename but would silently reassign split
ownership across an upgrade at the source enumerators.
`HashUtilsTest#testNotEquivalentToFloorModForNonPowerOfTwo` pins this invariant
so a future refactor cannot quietly cross the two.
### Scope
Following the boundaries agreed on #11976:
- **One helper**, with an explicit `bucketCount > 0` contract.
- **#11721 stays separate.** `MultiTableSinkWriter` is the one live bug
(`Math.abs(hash) % n` returns a negative index for `Integer.MIN_VALUE`) and is
fixed in #11721. It is untouched here to avoid a conflict; migrating it onto
the helper is a trailing follow-up once #11721 merges.
- **Only genuine hash-to-bucket routing sites** were migrated, not a
mechanical sweep of every `%` in the tree.
Two sites were considered and deliberately left alone:
- `FileSourceDocumentRouting.routeBucket` already uses `Math.floorMod` over
a SHA-256 digest. It is correct as written, and converting it to masking would
change routing for negative hashes. Left as is.
- `ShardRouter` uses a `long` variant (`& Long.MAX_VALUE`). Out of scope for
an `int` helper.
### Does this PR introduce a user-facing change?
No. Refactor only; routing decisions are identical for all inputs.
### How was this patch tested?
New `HashUtilsTest` covering `Integer.MIN_VALUE` (the overflow case that
motivated #11721), negative hashes across power-of-two and non-power-of-two
bucket counts, `bucketCount == 1`, rejection of `bucketCount <= 0`, and an
exhaustive equivalence check against the previous `(hash & Integer.MAX_VALUE) %
bucketCount` spelling over `hash` in `[-1000, 1000]`.
The two existing tests that mirrored the masking expression inline,
`BigtableSourceSplitEnumeratorTest` and `JdbcSourceSplitEnumeratorTest`, were
updated to call the helper so they still assert against a single source of
truth.
```
HashUtilsTest ................................ Tests run: 6, Failures: 0,
Errors: 0
JdbcSourceSplitEnumeratorTest ................ Tests run: 3, Failures: 0,
Errors: 0
BigtableSourceSplitEnumeratorTest ............ Tests run: 14, Failures: 0,
Errors: 0
```
`spotless:check` passes on all twelve touched modules, and all of them
compile.
### Check list
* [x] Code changed are covered with tests, or it does not need tests
* [x] If any new Jar binary package adding in your PR, please add License
Notice according [New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If you are contributing the connector code, please check that the
following files are updated:
1. Update change log that in connector document. For more details you can
refer to
[connector-v2](https://github.com/apache/seatunnel/tree/dev/docs/en/connector-v2)
2. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
3. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
* [x] Update the
[`release-note`](https://github.com/apache/seatunnel/blob/dev/release-note.md).
--
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]
