SEPURI-SAI-KRISHNA commented on issue #12612:
URL: https://github.com/apache/seatunnel/issues/12612#issuecomment-6399989904
@SEZ9 closing the loop here, late: #12605 merged on 2026-10-18 and the
BIGINT PR went up the next day as #12899, which closes this issue. I never came
back to say so.
**On reusing the helpers rather than duplicating them.** The PR does not add
a long-specific `outOfLongRange`. It unifies instead: the INT and LONG paths
now share one implementation selected by bounds.
```java
private enum IntegralBounds {
INT(Integer.MIN_VALUE, Integer.MAX_VALUE),
LONG(Long.MIN_VALUE, Long.MAX_VALUE);
}
// numberToInt -> return (int) narrowToIntegral(value, targetType,
IntegralBounds.INT);
// numberToLong -> return narrowToIntegral(value, targetType,
IntegralBounds.LONG);
```
So `numberToInt` keeps its behaviour but now delegates, and there is a
single range check and a single failure builder for both targets rather than
two near-copies in front of `castAs(List<Object>)`. Truncate-then-range-check
order and fraction handling are unchanged.
**On the test list.** All four values are covered under both `CAST` and
`TRY_CAST`:
| case | where |
| --- | --- |
| 2^64+5, 2^64, `NaN`, 1e30 |
`testCastAsBigintRejectsWideAndNonFiniteNumericSources` |
| exact `Long.MIN_VALUE` / `Long.MAX_VALUE` and one step beyond each |
`testCastAsBigintAcceptsTheExactBoundariesAndRejectsOneStepBeyond` |
| `TRY_CAST` returning null for the rejected values |
`testStringSourceBoundariesForABigintTargetIncludingTryCast` |
| in-range decimal and in-range double still convert | controls inside the
same tests |
Both infinities are included alongside `NaN`, and the error message test
pins that the failure names the target and its bounds.
One thing worth flagging that I found while writing the tests: a `Double`
has to be range checked as the double it is, not after narrowing, or 1e30
passes. That is
`testCastAsBigintChecksTheDoubleItselfRatherThanItsNarrowedValue`.
As agreed, the `COALESCE` / `IFNULL` result-type inference question stayed
out of this PR.
Current state of #12899: no conflicts, and it still needs a write-access
review. Its last CI run is from 2026-10-23 and failed only on
`BackpressureSlowSinkIT`, an engine e2e timing assertion unrelated to this
change, which touches no engine files. I have just rerun that job.
--
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]
