SEPURI-SAI-KRISHNA opened a new issue, #11723: URL: https://github.com/apache/seatunnel/issues/11723
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/seatunnel/issues?q=is%3Aissue+label%3A%22bug%22) and found no similar issues. ### What happened > **Not a duplicate of #11696.** That issue is about the *scalar functions* in `NumericFunction` (`CEIL`, `FLOOR`, `ROUND`, `MOD`, `TRUNC`), and its PR #11697 fixes them. This issue is about the *binary operators* `+ - * /` in `ZetaSQLFunction.executeBinaryExpr`, a different file and a different code path that does its own operand conversion and is untouched by that fix. `CEIL(x)` and `a + b` are corrupted by two separate pieces of code. I found this one while working on #11697 and said there that I would file it separately rather than widen that PR's scope. `ZetaSQLFunction.executeBinaryExpr` has two independent defects in its `DECIMAL` branch (`ZetaSQLFunction.java:763-787`). Both silently produce wrong numbers rather than failing. **1. Every DECIMAL operand is round-tripped through `double`** ```java if (resultType.getSqlType() == SqlType.DECIMAL) { BigDecimal bigDecimal = BigDecimal.valueOf(leftValue.doubleValue()); if (binaryExpression instanceof Addition) { return bigDecimal.add(BigDecimal.valueOf(rightValue.doubleValue())); } ... ``` `BigDecimal.valueOf(x.doubleValue())` collapses the value to a `double` first, so anything beyond ~17 significant digits is destroyed before the arithmetic even starts. `DECIMAL` exists precisely to avoid this. A `DECIMAL(38,2)` column — an ordinary money column — is comfortably inside the damage zone. | Expression on `DECIMAL(38,2)` | Correct result | SeaTunnel returns | |---|---|---| | `123456789012345678.99 + 0.01` | `123456789012345679.00` | `123456789012345680.01` | | `123456789012345678.99 - 0.01` | `123456789012345678.98` | `123456789012345680.00` | | `123456789012345678.99 * 0.01` | `1234567890123456.7899` | `1234567890123456.8` | The first row is the alarming one: the result is off by `1.01` **and** gains a spurious digit, on a value that fits `DECIMAL(38,2)` with room to spare. Nothing warns, and nothing fails. The same applies when only one side is `DECIMAL`. A `BIGINT` operand above 2^53 is rounded too — `DECIMAL(38,0) 1 + BIGINT 9007199254740993` returns `9007199254740993` instead of `9007199254740994`, because `9007199254740993` has no exact `double` representation. This affects `+`, `-`, `*` and `/`. `%` is not affected, because the `Modulo` branch delegates to `NumericFunction.mod` rather than doing its own conversion. **2. Division rounds away from zero instead of to nearest** ```java if (binaryExpression instanceof Division) { DecimalType decimalType = (DecimalType) resultType; return bigDecimal.divide( BigDecimal.valueOf(rightValue.doubleValue()), decimalType.getScale(), RoundingMode.UP); } ``` `RoundingMode.UP` always rounds away from zero — it is not "round up" in the everyday sense, and it is not what SQL division does. The result type's scale comes from `ZetaSQLType` (`max` of the operand scales, `ZetaSQLType.java:224-238`), so on the common `DECIMAL(38,2)` case the quotient is truncated to 2 decimals with this mode: | Expression at scale 2 | SQL-correct (`HALF_UP`) | SeaTunnel (`UP`) | |---|---|---| | `10 / 3` | `3.33` | `3.34` | | `1 / 1000` | `0.00` | `0.01` | The second row manufactures value out of nothing: a quotient that should round to zero becomes a non-zero amount. Summed over a table, this is a systematic upward bias, not a wash. **Suggested fix** Convert the operands exactly instead of via `double` — return the value unchanged when it is already a `BigDecimal`, and use `BigDecimal.valueOf(longValue())` for integral types — and use `RoundingMode.HALF_UP` for division. The rounding-mode change is user-visible and needs an `incompatible-changes.md` entry. ### SeaTunnel Version dev ### SeaTunnel Config ```conf env { parallelism = 1 job.mode = "BATCH" } source { FakeSource { plugin_output = "fake" schema = { fields { big = "decimal(38, 2)" small = "decimal(38, 2)" } } rows = [ {fields = ["123456789012345678.99", "0.01"], kind = INSERT} ] } } transform { Sql { plugin_input = "fake" plugin_output = "fake1" query = "select big + small as sum_val, big * small as mul_val, small / big as div_val from dual" } } sink { Console { plugin_input = "fake1" } } (The decimal values are quoted so the config parser keeps them exact — writing them as bare HOCON numbers would round them to `double` before they ever reach the transform, which would hide the bug being reported.) | Column | Expected | Actual | |---|---|---| | `sum_val` | `123456789012345679.00` | `123456789012345680.01` | | `mul_val` | `1234567890123456.7899` | `1234567890123456.8` | | `div_val` | `0.00` | `0.01` | ``` ### Running Command ```shell ./bin/seatunnel.sh --config ./config/repro.conf -e local ``` ### Error Exception ```log No exception. The values are silently wrong, which is what makes this worth fixing. ``` ### Zeta or Flink or Spark Version Zeta (engine-independent; the code is in seatunnel-transforms-v2) ### Java or Scala Version Java 11 ### Screenshots _No response_ ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
