dongjoon-hyun opened a new pull request, #57006: URL: https://github.com/apache/spark/pull/57006
### What changes were proposed in this pull request? This PR changes decimal `Divide` to perform the division directly at the result scale, instead of dividing at `DecimalType.MAX_SCALE + 1` and then rounding down to the result scale. - `Decimal` gets a new method `div(that: Decimal, scale: Int)` that divides with `RoundingMode.HALF_UP` at the given scale. - `DivModLike` codegen gains an overridable `decimalOperation` hook (default: existing `decimalMethod` behavior), and `Divide` overrides it — both the codegen and the interpreted eval now call `div(b, resultScale)` followed by the existing overflow check. `IntegralDivide`, `Remainder`, and `Pmod` are unchanged. Previously, `Decimal./` always divided at scale `39` with `ROUND_DOWN` and the subsequent `changePrecision` performed a second division to reach the result scale. This is equivalent to a single `HALF_UP` division at the result scale: truncating at 39 fractional digits cannot change a `HALF_UP` rounding decision at any scale `<= 38`, because every half-way point at scale `s <= 38` is exactly representable within 39 fractional digits. ### Why are the changes needed? The fixed scale-39 intermediate makes decimal division needlessly expensive, especially for small decimal types. For example, `decimal(7,2) / decimal(7,2)` (result `decimal(17,10)`) produces a scale-39 intermediate whose unscaled value exceeds the `Long` range, forcing two `BigInteger` (Knuth) divisions per row — one inside `BigDecimal.divide` and another in `CheckOverflow`'s rescale, visible in profiles as: ``` java.math.MutableBigInteger.divideMagnitude java.math.BigDecimal.setScale org.apache.spark.sql.types.Decimal.changePrecision org.apache.spark.sql.types.Decimal.toPrecision ``` Dividing once at the result scale keeps the computation in the compact `long` representation whenever the quotient fits. A JDK-level microbenchmark of the `decimal(7,2) / decimal(7,2)` evaluation shows `divide(39, DOWN)` + `setScale(10, HALF_UP)` at 157.7 ns/op vs `divide(10, HALF_UP)` at 15.7 ns/op (~10x), with identical results over one million random pairs. Larger types such as `decimal(38,18)` still benefit from halving the number of big divisions. ### Does this PR introduce _any_ user-facing change? Query results are unchanged. The only visible difference is that the `NUMERIC_VALUE_OUT_OF_RANGE` error message on division overflow now renders the quotient rounded at the result scale instead of the internal scale-39 intermediate, e.g. `1000000073899961059796.725866331521039185` instead of `1000000073899961059796.725866331521039184725213147467478092333`. The error class, parameters, and whether an error occurs are all unchanged. ### How was this patch tested? - New test `SPARK-57937: div with target scale` in `DecimalSuite`, checking `div` against `/` + rounding on half-way cases, non-terminating expansions, both signs, and 2,000 random `decimal(7,2)` pairs. - Existing suites: `DecimalSuite`, `ArithmeticExpressionSuite`, `DecimalPrecisionSuite`, `DecimalExpressionSuite`. - SQL golden files: `decimalArithmeticOperations.sql` (regenerated; only the error-message rendering above changed), `try_arithmetic.sql`, `cast.sql`, `interval.sql`, `sql-udf.sql`. - `DecimalDivideBenchmark` (SPARK-57935) covers this path; benchmark results should be regenerated via the GitHub Actions benchmark workflow. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Fable 5 -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
