SEPURI-SAI-KRISHNA opened a new pull request, #11937:
URL: https://github.com/apache/seatunnel/pull/11937
### Purpose of this pull request
Closes #11935.
> [!IMPORTANT]
> **This PR stacks on #11927.** GitHub can only target a branch in
`apache/seatunnel`, so the base here is `dev` and the diff shows **two**
commits — `3de3d9c` is #11927's, already reviewed and approved there. **Only
the second commit, `bfeaa65`, belongs to this PR.** Once #11927 merges, this
diff collapses to that commit on its own.
>
> The dependency is real, not cosmetic: `ROUND(TINYINT 127, -1)` is `130`,
which does not fit a `TINYINT`. Adding the `BYTE` case without #11927's range
check would trade a silent no-op for a silently wrapped `-126`.
Three Zeta SQL numeric functions dispatch on the runtime type of their
argument with an incomplete set of branches.
**1. `ROUND` / `CEIL` / `CEILING` / `FLOOR` / `TRUNC` / `TRUNCATE` silently
ignored `TINYINT`.**
The shared `round(Number, Number, RoundingMode, String)` helper switches on
the argument's class simple name with cases `INTEGER`, `SHORT`, `LONG`,
`BIGDECIMAL`, `DOUBLE`, `FLOAT` — no `BYTE`, and no `default`. A `Byte` fell
straight through and the method returned its input unchanged.
Driven through the real `SQLTransform`:
| Expression | Column type | Before | After |
|---|---|---|---|
| `ROUND(tiny_v, -1)`, `tiny_v = 44` | `TINYINT` | **`44`** | `40` |
| `CEIL(tiny_v, -1)`, `tiny_v = 44` | `TINYINT` | **`44`** | `50` |
| `ROUND(small_v, -2)`, `small_v = 1234` | `SMALLINT` | `1200` | `1200` |
The `SMALLINT` row is the control: the identical expression one type up
always worked. That is what makes this a missing branch rather than a
deliberate carve-out. `44` is chosen because it rounds to `40`, well inside a
`TINYINT`, so the dispatch bug is isolated from any overflow concern.
**2. `ABS` and `SIGN` rejected `TINYINT` and `SMALLINT` outright.**
Both use an `instanceof` chain over `Integer`, `Long`, `Float`, `Double`,
`BigDecimal` and then throw:
```
ErrorCode:[TRANSFORM_COMMON-06], ErrorDescription:[The expression
'ABS(tiny_v)' of SQL transform execute failed]
caused by: ErrorCode:[COMMON-05], ErrorDescription:[Unsupported operation]
- Unsupported arg type java.lang.Byte of function ABS
```
This contradicted the documentation. `docs/en/transforms/sql-functions.md`
ABS section: *"Note that TINYINT, SMALLINT, INT, and BIGINT data types cannot
represent absolute values of their minimum negative values…"* — the page
specifies overflow semantics for `ABS(TINYINT)` while the code refused the type
as unsupported.
@DanielLeens found the `SMALLINT` half of this independently while reviewing
#11927 (minor observation 2) and flagged it for a separate follow-up. This is
that follow-up.
**Why these types are reachable.** `ZetaSQLType.isNumberType` accepts
everything from `SqlType.TINYINT` to `SqlType.DECIMAL` inclusive
(`ZetaSQLType.java:254-256`); `BasicType.BYTE_TYPE` maps `Byte` to
`SqlType.TINYINT` (`BasicType.java:30`); and for these functions
`getFunctionType` returns the first argument's type
(`ZetaSQLType.java:463-473`), so a `TINYINT` column stays `TINYINT` into the
function. The same class already handles both deliberately —
`NumericFunction.toBigDecimal` has an explicit `Byte`/`Short` branch. They were
simply omitted from these three functions.
**3. `SIGN` lost the sign of a `BigDecimal` too small for `double`
(minor).** `sign` converted via `doubleValue()`, so any magnitude below
`Double.MIN_VALUE` underflowed to `0.0`. In fairness this is not reachable
through a declared column — `DECIMAL(p, s)` caps precision at 38 — so it is
included as a correctness cleanup in the same `instanceof` chain, not as a
user-facing bug. `BigDecimal.signum()` is exact, cheaper, and allocation-free.
**The fix.** Add the `BYTE` case to the rounding family, routed through
#11927's `checkIntegralRange` so a result that no longer fits `TINYINT` fails
loudly. Add a `default` to that switch so an unhandled numeric type throws
instead of being returned unrounded. Add `Byte`/`Short` branches to `ABS` and
`SIGN`, guarding `MIN_VALUE` in `ABS` exactly as `INT`/`BIGINT` are guarded —
`Math.abs` promotes to `int`, so `(byte) -128` would come back as `128` and not
fit. Switch `SIGN`'s `BigDecimal` branch to `signum()`.
### Does this PR introduce _any_ user-facing change?
Yes, and `incompatible-changes.md` is updated in both languages with a
before/after table and a migration guide.
A `TINYINT` column that silently skipped rounding now receives the rounded
value; if a rounded `TINYINT` no longer fits, the row now fails loudly instead
of wrapping. `ABS` and `SIGN` on `TINYINT`/`SMALLINT` now succeed where they
previously threw, which is strictly additive — existing casts like
`ABS(CAST(tiny_col AS INT))` keep working unchanged.
**No change to `sql-functions.md`.** Its `ROUND` overflow note is already
written for "an integral type", and its `ABS` section already names `TINYINT`
and `SMALLINT`. This PR brings the code up to documentation that was already
correct, which is why there is nothing to correct there.
### How was this patch tested?
`./mvnw -pl seatunnel-transforms-v2 test` on JDK 17 — **1101 run, 0
failures, 0 errors** (1094 before, so +7). `spotless:check` clean.
Six unit tests in `NumericFunctionTest` and one end-to-end test in
`SQLNumericFunctionsTest` that drives the real `SQLTransform` over
`TINYINT`/`SMALLINT` columns, including the `SMALLINT` control row.
I mutation-tested each new guard to confirm the assertions are not vacuous —
disabling one and checking that exactly the intended tests fail and nothing
else does, across all 1101:
| Mutation | Tests killed | Collateral |
|---|---|---|
| remove `case "BYTE"` from `round()` | `testRoundingFamilyHandlesTinyInt`,
`testTinyIntRoundingRejectsResultsThatDoNotFit`, SQL end-to-end | none |
| remove the new `default:` | `testRoundingRejectsUnhandledNumericTypes` |
none |
| remove `abs()` Byte/Short branches |
`testAbsAndSignAcceptTinyIntAndSmallInt`,
`testAbsRejectsTinyIntAndSmallIntMinValue`, SQL end-to-end | none |
| revert `signum()` to `doubleValue()` |
`testSignIsExactForDecimalsBelowDoubleRange` | none |
The source was restored byte-identical afterwards and the suite re-run green.
`TRUNC` is pinned as unaffected: it rounds toward zero and so can never grow
a value out of its own range, and
`testTinyIntRoundingRejectsResultsThatDoNotFit` asserts `TRUNC(127, -1) == 120`
rather than throwing.
### 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 — `sql-functions.md`
already documents the intended behavior; see above
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR. — updated, `docs/en` and `docs/zh`
* [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]
