SEPURI-SAI-KRISHNA opened a new pull request, #11724:
URL: https://github.com/apache/seatunnel/pull/11724
### Purpose of this pull request
Closes #11723.
`ZetaSQLFunction.executeBinaryExpr` had two independent defects in its
`DECIMAL` branch.
**Operands were round-tripped through `double`.** `+`, `-`, `*` and `/` all
converted with `BigDecimal.valueOf(value.doubleValue())`, collapsing each
operand to a `double` before the arithmetic started and discarding everything
beyond ~17 significant digits. On a `DECIMAL(38,2)` money column,
`123456789012345678.99 + 0.01` returned `123456789012345680.01` — off by
`1.01`, with a spurious digit — instead of `123456789012345679.00`. A `BIGINT`
operand above 2^53 was rounded for the same reason. The fix converts exactly:
`BigDecimal` operands are used as-is, integral types go through
`BigDecimal.valueOf(longValue())`.
**Division rounded away from zero.** It used `RoundingMode.UP`, which is not
"round up" but "always away from zero", so at scale 2 `10 / 3` returned `3.34`
instead of `3.33` and `1 / 1000` returned `0.01` instead of `0.00` —
manufacturing value from a quotient that should round to zero, with a
systematic upward bias across a table. Changed to `RoundingMode.HALF_UP`.
`%` was already correct and is untouched: the `Modulo` branch delegates to
`NumericFunction.mod` instead of converting operands itself.
The conversion helper is deliberately private to `ZetaSQLFunction` rather
than shared with `NumericFunction`, so that this PR touches no file that #11697
touches and the two can be merged in either order. Once both land, the two
helpers can be unified in a follow-up.
### Does this PR introduce _any_ user-facing change?
Yes, and it is documented in `incompatible-changes.md` (en + zh).
Results of `+ - * /` on `DECIMAL` columns change wherever the old `double`
conversion was lossy, and every inexact division result changes where the old
`UP` rounding inflated it. In both cases the new value is the correct one and
the old value was wrong, but jobs reconciled against the old output will see a
difference, so it is called out as breaking with migration guidance.
No config options, defaults, or SPI contracts changed.
### How was this patch tested?
Added `SQLDecimalArithmeticTest` (4 tests) covering exact `+`/`-`/`*` on
`DECIMAL(38,2)`, an exact `DECIMAL` + `BIGINT` mix above 2^53, and both
division rounding cases. They run the real SQL path through `SQLTransform`, not
the arithmetic in isolation.
I verified each test reproduces the bug by reverting the fix and re-running:
```
[ERROR] Tests run: 4, Failures: 4, Errors: 0, Skipped: 0
[ERROR] testAddSubtractMultiplyStayExact:82 expected:
<123456789012345679.00> but was: <123456789012345680.01>
[ERROR] testMixedDecimalAndBigintStaysExact:104 expected:
<9007199254740994> but was: <9007199254740993.0>
[ERROR] testDivisionRoundsHalfUp:121 expected: <3.33> but was: <3.34>
[ERROR] testDivisionDoesNotInventValue:135 expected: <0.00> but was: <0.01>
```
With the fix restored the full module suite passes: `Tests run: 1079,
Failures: 0, Errors: 0, Skipped: 0`. `./mvnw spotless:apply` and `./mvnw -pl
seatunnel-transforms-v2 -DskipTests verify` are both clean.
I also checked every e2e config that declares a `DECIMAL` column: none
performs `+ - * /` on one (they only compare or pass through), so no e2e
assertions needed updating.
### Check list
* [ ] 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)
* [ ] 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.
* [ ] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml)
4. Add e2e testcase in
[seatunnel-e2e](https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/)
5. Update connector
[plugin_config](https://github.com/apache/seatunnel/blob/dev/config/plugin_config)
--
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]
