Skip to content

[SPARK-59686][SQL] Clamp the rounding scale in round and bround - #58939

Open
SEPURI-SAI-KRISHNA wants to merge 2 commits into
apache:masterfrom
SEPURI-SAI-KRISHNA:SPARK-59686-round-scale-clamp
Open

SEPURI-SAI-KRISHNA wants to merge 2 commits into
apache:masterfrom
SEPURI-SAI-KRISHNA:SPARK-59686-round-scale-clamp

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

round and bround pass the user supplied scale straight to BigDecimal.setScale, which throws once the magnitude passes roughly 1e9. This PR clamps the scale to a bound past which it cannot change the result.

The clamp goes on RoundBase._scale, a single protected lazy val that every type branch reads and that codegen interpolates as a literal, so one change covers interpreted and generated code for all numeric types.

Why are the changes needed?

Neighbouring scales already return the answer, so today a well defined result becomes an error:

SELECT round(1.5, -10000000);    -- 0.0
SELECT round(1.5, -1000000000);  -- java.lang.ArithmeticException
SELECT round(1.5, 100000);       -- 1.5
SELECT round(1.5, 2147483647);   -- java.lang.ArithmeticException

The answers are not in doubt. A scale far to the left of the decimal point rounds any finite value to zero, and a scale past the digits a value carries leaves it unchanged, which is exactly what the smaller scales return.

There are two symptoms with one cause:

  • On the ANSI integral path the exception is wrapped by MathUtils.withOverflow and surfaces as ARITHMETIC_OVERFLOW. That is spurious: nothing overflowed and the result is representable.
  • Everywhere else, so FLOAT, DOUBLE, DECIMAL with a negative scale, and the integral types in non-ANSI mode, the raw java.lang.ArithmeticException reaches the user. It is not a SparkThrowable, so it carries no error condition and no SQLSTATE.

RoundBase.dataType is affected too: it computes -_scale + 1, and for Int.MinValue that negation overflows, so a DECIMAL input gets a narrower result precision than intended. Clamping fixes that as well.

Does this PR introduce any user-facing change?

Yes. Scales beyond the clamp bound now return the rounded value instead of raising. No input that previously produced a value changes, since the clamp is chosen so that it cannot alter a result.

No migration guide entry, following #58047, which likewise replaced a spurious error with the correct value. #57832 added one because it changed one error condition into another, which is not the case here.

How was this patch tested?

Added round/bround with an extreme scale to MathExpressionsSuite, covering both signs at Int.MinValue, Int.MaxValue and 1e9, across DOUBLE, FLOAT, LONG and INT, in both ANSI and non-ANSI mode.

The clamp is only correct if it preserves results, so the test also pins the two values most sensitive to it: Double.MinPositiveValue, the subnormal with the longest exact decimal expansion, and Double.MaxValue. I checked separately that clamping is bit identical to the unclamped scale for both, on either sign.

Reverting the clamp makes the new test fail, which is the behaviour being fixed.

build/sbt "catalyst/testOnly *MathExpressionsSuite"

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

round and bround passed the user supplied scale straight to
BigDecimal.setScale, which throws once the magnitude passes roughly 1e9.
Neighbouring scales return the result fine, so a well defined answer
became an error: round(1.5, -10000000) is 0.0 while round(1.5,
-1000000000) raised java.lang.ArithmeticException.

On the ANSI integral path the exception was wrapped by
MathUtils.withOverflow and surfaced as a spurious ARITHMETIC_OVERFLOW,
even though nothing overflowed and the result is representable.
Everywhere else, including the integral types in non-ANSI mode, the raw
java.lang.ArithmeticException reached the user.

Clamp the scale where it is read. _scale is a single protected lazy val
that every type branch uses and that codegen interpolates, so clamping
there covers interpreted and generated code for all numeric types. The
clamp cannot change a result: no finite value has more than 309 integral
digits, and the exact decimal expansion of a finite double needs at most
1074 fractional digits. It also stops -_scale in dataType from
overflowing for a scale of Int.MinValue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant