SEPURI-SAI-KRISHNA commented on PR #12605:
URL: https://github.com/apache/seatunnel/pull/12605#issuecomment-5988898546
Thanks @SEZ9. Keeping this short given the 2500-character truncation. One of
the two is new work and is done; I think the other two points were already
closed on this thread, so pointers rather than argument.
**New: the `BigDecimal`-through-`COALESCE` test you asked for is added**, as
`ZetaSQLEngineTest.testCoalesceRejectsADecimalBeyondLongRange`. I used 2^64
deliberately, because it is the value that separates the two implementations:
```
new BigDecimal("18446744073709551616").longValue() == 0
```
Zero is inside int range, so a check done after widening would accept 2^64
and silently emit `0`. The exact path rejects it. The test asserts that
`longValue()` property first so it cannot quietly stop proving anything, then
the rejection, then an in-range decimal still converting.
Mutation check: deleting the two exact-conversion branches fails only this
test ("Expected TransformException to be thrown, but nothing was thrown").
Module: `Tests run: 1236, Failures: 0`.
**The `longValue()` contract.** I believe you closed this yourself on
2026-10-04:
> On the `numberToInt` wrapping point: at head `5eb6a8ca9bb` the
`BigDecimal` and `BigInteger` branches go through the `BigInteger` bound
comparison rather than `longValue()`, so that is resolved as far as I can see.
That is still the case at the current head. The "guaranteed not to have
wrapped" wording you quoted was real, but it was removed in `5eb6a8ca9`, the
same commit that introduced the exact conversion. `longValue()` now runs only
on the Byte/Short/Integer/Long branch, where widening is exact.
**Error code.** Also previously settled, I think: I set out the precedent on
2026-10-03 and your 2026-10-04 reply listed only two remaining asks, neither of
which was this. For the record, `UNSUPPORTED_OPERATION` is used 36 times across
the Zeta function classes and is the only code in `SystemFunction`,
`NumericFunction` and `CastFunction`. The message itself already avoids the
`CAST` keyword: `Value %s cannot be converted to %s: out of range [%d, %d]`.
If you would still like a data-oriented code, I am happy to do it as its own
change across all 36 sites rather than making this one branch inconsistent with
its neighbours.
Both asks from your 2026-10-04 list are also still in place: the CAST
wording is on both pages, and #12612 is linked from the incompatible-changes
entry at en line 40 and zh line 30.
Pushing the test now, will report back when the Build settles.
--
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]
