SEZ9 commented on PR #12605: URL: https://github.com/apache/seatunnel/pull/12605#issuecomment-6007970744
Thanks @SEPURI-SAI-KRISHNA, and sorry for the delay here. **COALESCE / `BigDecimal` test.** Using `18446744073709551616` is a good choice: since `longValue()` on it yields `0`, it is the input that separates an exact bound check from a check-after-widening, and asserting that property up front keeps the test from silently becoming a no-op. Once the push is up, please leave a one-line note here and I'll review the test in the diff. **`longValue()` contract.** I did note at `5eb6a8ca9bb` that the `BigDecimal`/`BigInteger` branches go through the `BigInteger` bound comparison. I'll confirm against the current head that `longValue()` is still confined to the Byte/Short/Integer/Long branch and that the old "guaranteed not to have wrapped" wording is gone, then resolve it. **Error code.** The consistency argument is reasonable; if `UNSUPPORTED_OPERATION` is the only code used by the neighbouring function classes, I would not want this one branch to differ. I'm fine deferring a data-oriented code to a separate change. I'll check the message emitted on the COALESCE/IFNULL path in the diff to confirm it no longer refers to `CAST`. **Docs.** The reworded CAST note you quoted (naming the four rejecting targets, omitting `BIGINT`, with `CAST(3000000000 AS INT)` as the example) plus the follow-up link in the incompatible-changes entry is what I asked for at `c5b287cc5`. I'll verify the text on both pages in the diff and resolve. **Javadoc.** Could you point me to the commit where the `numberToInt` Javadoc was trimmed to describe the current contract (inputs, range check, exception) rather than the before/after history? If it was `5eb6a8ca9`, I'll check there. After the test push I'll do a final pass over the diff at the new head; I expect this to be close to ready. <!-- streview-comment:1541 --> -- 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]
