SEPURI-SAI-KRISHNA opened a new pull request, #11927:
URL: https://github.com/apache/seatunnel/pull/11927
### Purpose of this pull request
Closes #11926.
Two Zeta SQL numeric functions narrowed an integral result without checking
it fits, so a value
that overflowed came back **wrapped around to a wrong — usually negative —
number** instead of
failing. Nothing logged, nothing thrown; the bad value was written
downstream as an ordinary row.
**1. `ABS` on the minimum negative value** — `Math.abs(Integer.MIN_VALUE) ==
Integer.MIN_VALUE`, and
the same for `Long`. `NumericFunction.abs` returned that straight back, so
`ABS(-2147483648)`
produced `-2147483648`.
This one contradicts documentation that has been in the tree all along
(`docs/en/transforms/sql-functions.md`):
> `ABS(-2147483648)` should be `2147483648`, but this value is not allowed
for this data type.
> **It leads to an exception.** To avoid it cast argument of this function
to a higher data type.
The implementation simply never did it.
**2. `ROUND` / `CEIL` / `CEILING` / `FLOOR` with a negative digit count** —
the shared
`round(Number, Number, RoundingMode)` helper narrowed the rounded value
*twice* without a range
check:
```java
long scaled = BigDecimal.valueOf(original)
.setScale(scale, roundingMode)
.longValue(); // (1) keeps only the low-order 64 bits ->
wrong for BIGINT
if (original != scaled) {
v1 = convertTo(t, scaled); // (2) intValue()/shortValue() -> wrong for
INT / SMALLINT
}
```
`BigDecimal.longValue()` is documented to discard everything above bit 63,
and `intValue()` /
`shortValue()` truncate the same way.
Observed on `dev` at `4ba2895`:
| Expression | Argument type | Result before | Result after |
|------------|---------------|---------------|--------------|
| `ABS(-2147483648)` | `INT` | `-2147483648` | `TransformException` |
| `ABS(-9223372036854775808)` | `BIGINT` | `-9223372036854775808` |
`TransformException` |
| `ROUND(2147483647, -1)` | `INT` | `-2147483646` | `TransformException` |
| `ROUND(9223372036854775807, -1)` | `BIGINT` | `-9223372036854775806` |
`TransformException` |
| `CEIL(32767, -1)` | `SMALLINT` | `-32766` | `TransformException` |
| `FLOOR(-2147483648, -1)` | `INT` | `2147483646` | `TransformException` |
`TRUNC` / `TRUNCATE` uses `RoundingMode.DOWN`, which always moves toward
zero and therefore can
never grow a value out of its own range — it is unaffected and covered by a
test that pins that.
`FLOAT`, `DOUBLE` and `DECIMAL` arguments take different branches and are
untouched.
### The fix
Round in `BigDecimal` and narrow only *after* checking the result is
representable:
```java
BigDecimal rounded = BigDecimal.valueOf(v1.longValue()).setScale(scale,
roundingMode);
v1 = convertTo(t, rounded, function);
```
`convertTo` now takes the exact `BigDecimal` and the SQL function name, and
routes through
`checkIntegralRange`, which raises a `TransformException` naming the
function, the value it
produced and the range that value has to fit:
```
ErrorCode:[COMMON-05], ErrorDescription:[Unsupported operation] - Function
ROUND produced
2147483650, which is out of range for INT (-2147483648 to 2147483647). Cast
the argument to a
wider data type to avoid this.
```
`abs()` gets the matching guard for `Integer.MIN_VALUE` and
`Long.MIN_VALUE`, with an error that
mirrors the wording already in the docs:
```
ErrorCode:[COMMON-05], ErrorDescription:[Unsupported operation] - Function
ABS cannot represent
the absolute value of -2147483648: INT allows -2147483648 to 2147483647, so
the result does not
fit. Cast the argument to a wider data type to avoid this.
```
### Does this PR introduce _any_ user-facing change?
Yes — a job that used to emit a wrapped value now fails on the row that
overflows. That is the
point of the change, but it is a behaviour change, so it is recorded as
`**[BREAKING]**` with a
migration guide in both
`docs/en/introduction/concepts/incompatible-changes.md` and
`docs/zh/introduction/concepts/incompatible-changes.md`.
Migration is a cast — `ABS(CAST(int_col AS BIGINT))`, `ROUND(CAST(int_col AS
BIGINT), -1)`.
Docs updated:
- `docs/en/transforms/sql-functions.md` and
`docs/zh/transforms/sql-functions.md` — negative
`digitsInt` overflow note on `ROUND`. The `ABS` sections already described
this behaviour
correctly and needed no change; this PR makes the code match them.
- `docs/en/introduction/concepts/incompatible-changes.md` and the `zh`
mirror — new `**[BREAKING]**`
entry with a before/after table and a **Migration Guide**.
### How was this patch tested?
`seatunnel-transforms-v2` unit tests, **1094 run, 0 failures** on JDK 17.
Three new tests plus one
existing-behaviour guard:
- `NumericFunctionTest.testAbsRejectsMinValueInsteadOfReturningItUnchanged`
— both integral
`MIN_VALUE`s throw and the message names the SQL type; `MIN_VALUE + 1`
still returns `MAX_VALUE`,
so the guard is pinned to the exact boundary rather than a range.
- `NumericFunctionTest.testNegativeScaleRoundingRejectsResultsThatDoNotFit`
— `ROUND`/`CEIL`/`FLOOR`
overflow at `INT`, `SMALLINT` and `BIGINT`, each asserting the function
name and target type in
the message; `TRUNC(2147483647, -1)` still returns `2147483640`.
-
`NumericFunctionTest.testNegativeScaleRoundingKeepsArgumentTypeWhenResultFits`
— regression guard
for the restructured path: `ROUND(x, -2)` still returns
`Integer`/`Short`/`Long` matching the
argument, and rounding away every significant digit yields `0` rather than
an error.
-
`SQLNumericFunctionsTest.testIntegralOverflowFailsInsteadOfProducingNegativeValues`
— same two
defects through the real SQL engine. The engine wraps expression failures,
so this asserts the
outer message names the expression and the detail is on the cause.
I mutation-checked the new assertions rather than trusting that they pass:
disabling the range
check (`if (false)`) and both `MIN_VALUE` guards fails exactly the three
overflow tests and nothing
else in the module, so they are not vacuous.
```
./mvnw -pl seatunnel-transforms-v2 test # 1094 run, 0 failures
./mvnw -pl seatunnel-transforms-v2 spotless:check
```
### Known related gap, not in this PR
`round()`'s `switch` handles `INTEGER`, `SHORT` and `LONG` but has no `BYTE`
case and no `default`,
so a `TINYINT` argument falls through and comes back **unrounded** —
`ROUND(tinyint_col, -1)` is a
silent no-op. `ABS` and `SIGN` have the mirror-image problem: neither
accepts `Byte` or `Short`, so
they throw `UNSUPPORTED_OPERATION` on a `TINYINT` column even though
`NumericFunction.toBigDecimal`
in the same file already handles both.
That is a distinct defect with a different fix, and folding it in here would
widen this PR past the
one it closes. I am filing it separately and will open the follow-up PR
against it — same as the
`toBigDecimal` follow-up from #11724, which became #11918.
### Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
— no new dependencies
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [x] If you are contributing the connector code, please check that the
following files are updated — not a connector change
--
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]
