SEZ9 commented on PR #11721: URL: https://github.com/apache/seatunnel/pull/11721#issuecomment-5366233534
Thanks @SEPURI-SAI-KRISHNA for the detailed confirmation pass, and @DanielLeens for re-verifying against `7cfabc8` — closing this out on my side. **F1 (docs snippet divergence):** Agreed with your scoping. The two `Math.abs` occurrences being fixed with the `Integer.MIN_VALUE` comment is the correctness fix this PR owes; reshaping the illustrative pseudocode (`extractPrimaryKeyIfPresent`, `replicaNum`, etc.) into a faithful mirror of `MultiTableSinkWriter` is a separate docs effort and would indeed bury a 1-line fix in a rewrite. Good catch that the `zh` page uses the formula form and never carried the bug. **Remaining ask:** please open the follow-up issue for the docs-mirror rewrite as offered and drop the link in this thread so the deferral is tracked, not lost. **F2 (shared non-negative-mod helper):** Withdrawn. Your survey of 15 inline call sites across 14 modules is persuasive — a single-caller helper would make this the one inconsistent site, and the actual trap (`Math.abs(...hashCode()) %`) is now gone from production code, which was the real goal. **F3 (regression test assertion):** Your reply appears to have been truncated before covering item 3, and @DanielLeens noted the exact-index assertion lives in the sibling test. That's fine as coverage, but the original ask was for the `Integer.MIN_VALUE` test itself to pin the target queue rather than only asserting no-throw + total count. **Remaining ask:** either add the exact-queue assertion to that test (should be a couple of lines mirroring the sibling), or confirm here why the sibling test makes it redundant, and I'm happy either way. Once those two small items are settled, this is done from the review side — the remaining blocker is the write-access re-approval already discussed above, which is out of your hands. <!-- streview-comment:411 --> -- 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]
