SEPURI-SAI-KRISHNA opened a new pull request, #12605:
URL: https://github.com/apache/seatunnel/pull/12605
## Purpose of this pull request
`CAST(bigint_col AS INT)` silently wrapped a value `INT` could not hold, but
only when the source was numeric. The same expression with a string source
reported the overflow. So it either failed or corrupted the value depending on
nothing but the source column's type.
Closes #12571
Scoped as agreed on that issue: planner acceptance unchanged, `CAST` failing
so `TRY_CAST` can return null, and `FLOAT` / `Infinity` left out as a separate
compatibility decision.
## What changes
One method, `SystemFunction.castAs`, numeric branch of the `INT` | `INTEGER`
case: `Number.intValue()` is replaced with a range-checked conversion that
throws a `TransformException` naming the accepted range.
**`TINYINT`, `SMALLINT` and `BYTE` are deliberately untouched.** I had
originally routed all four integral targets through one shared helper, as the
issue discussion implied, then measured what that actually did and backed it
out. Those three convert with `Byte.parseByte` and `Short.parseShort`, which
already reject an out-of-range value, so they never had the defect. Worse,
routing them through a numeric helper would have *loosened* them:
`Byte.parseByte("5.7")` throws today, whereas `Number.longValue()` would have
truncated a fractional source to `5`. The `INT` numeric path is the only thing
that was ever broken, and it is now the only thing changed.
`TRY_CAST` needs no change. `ZetaSQLFunction.executeTryCastExpr` already
catches `Exception` and returns null, so making the cast fail is exactly what
lets `TRY_CAST` report null here.
`CastFunction.getCastType` is not modified, so planner acceptance is
identical.
## Why it is worth a behaviour change
The wraparound flips signs. `Integer.MIN_VALUE - 1` came back as
`2147483647`: a negative input surfacing as the largest positive int, with
nothing reported.
| `BIGINT` input | before | after |
| --- | --- | --- |
| `-2147483649` | `2147483647` | error |
| `-2147483648` | `-2147483648` | `-2147483648` |
| `0` | `0` | `0` |
| `2147483647` | `2147483647` | `2147483647` |
| `2147483648` | `-2147483648` | error |
| `3000000000` | `-1294967296` | error |
`TRY_CAST` returns null for the rows that now error, and is unchanged for
the rest.
## One path beyond `CAST` that this also reaches
Surfacing this rather than leaving it to review. `COALESCE` and `IFNULL`
reach the same `castAs` boundary, and `ZetaSQLType` infers their result type
from the **first non-null argument**, not the widest. So `COALESCE(int_col,
bigint_col)` targets `INT`, and an out-of-range value from the `BIGINT`
argument was truncated in exactly the same silent way:
```
COALESCE(c_int, c_bigint), c_int null, c_bigint = 3000000000
before: -1294967296
after: TransformException
```
Measured on pristine `dev` and again with the patch. It is the same defect
in another expression, so it is fixed rather than special-cased around, and
pinned with a test.
`CASE` also reaches `castAs` but infers the widest branch type, so it cannot
narrow. I checked both branch orders; both widen to `BIGINT`. There is a test
pinning that too, so a later change to that inference does not quietly start
failing `CASE`.
## Tests
Seven cases added to the existing `ZetaSQLEngineTest`:
- `testCastBigintToIntAcceptsTheExactBoundaries` pins `MIN`, `0`, `MAX`.
- `testCastBigintToIntRejectsJustOutsideTheBoundaries` pins `MIN-1`,
`MAX+1`, `3000000000`.
- `testTryCastReturnsNullWhereCastNowFails` pins null for those, and an
in-range value still returning.
- `testStringSourceBoundariesAreUnchangedForEveryIntegralTarget` walks
`MIN-1` / `MIN` / `MAX` / `MAX+1` for `TINYINT`, `SMALLINT` and `INT` from a
string source, asserting both the `CAST` outcome and the `TRY_CAST` null.
- `testWideningAndIdentityCastsAreUnchanged` pins ten widening and identity
conversions by exact value and type.
- `testCoalesceAndIfnullAlsoRejectAnOutOfRangeNarrowing` pins the `COALESCE`
/ `IFNULL` path.
- `testCaseExpressionWidensAndIsUnaffected` pins that `CASE` widens and is
untouched.
`seatunnel-transforms-v2` is green: `Tests run: 1165, Failures: 0, Errors:
0, Skipped: 0`, reproduced across repeat runs. `spotless:check` passes.
## Mutation check
Reverting the one changed line back to `Number.intValue()` fails three of
the new tests, for the right reasons:
```
testCastBigintToIntRejectsJustOutsideTheBoundaries
expected CAST to reject -2147483649 ==> Expected TransformException, but
nothing was thrown.
testTryCastReturnsNullWhereCastNowFails
expected TRY_CAST to yield null for -2147483649 ==> expected: <null> but
was: <2147483647>
testCoalesceAndIfnullAlsoRejectAnOutOfRangeNarrowing
coalesce(c_int, c_bigint) should reject an out-of-range bigint ==> nothing
was thrown.
```
The second is the sign flip caught directly.
## Compatibility
Documented in `docs/en/transforms/sql-functions.md`,
`docs/zh/transforms/sql-functions.md`, and the incompatible-changes record in
both languages, including the `COALESCE` / `IFNULL` effect, since a job relying
on the wraparound now fails at the overflowing row instead of writing a wrong
number.
In-range values, widening casts, identity casts and fractional handling are
all unaffected, and no option, default or signature changes. `FLOAT` and
`DOUBLE` are deliberately out of scope and still return `Infinity`; I will file
that separately.
--
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]
