[SPARK-60119][SQL] Skip rescaling in Decimal.changePrecision when the result is known from the magnitude by viirya · Pull Request #59335 · apache/spark

viirya · 2026-10-10T05:26:45Z

What changes were proposed in this pull request?

In the BigDecimal branch of Decimal.changePrecision, when the scale change exceeds DecimalType.MAX_PRECISION, the value's number of integral digits (precision - scale, computed in Long) is checked before calling setScale:

If the value is nonzero and has more integral digits than the target type allows, return false (overflow) directly. Rounding cannot bring it back into range.
If the value is zero or below 0.01 ulp of the target scale, the result depends only on the sign and the rounding mode, so a same-sign value of 0.01 ulp is rescaled instead. This gives the same result for every rounding mode.

Otherwise the remaining scale change is bounded by the number of digits of the value, so setScale is unchanged. The extra check costs one Int comparison when the scale change is small, which is the common case.

NUMERIC_VALUE_OUT_OF_RANGE.WITH_SUGGESTION now renders a decimal whose scale is below -1000 (only possible under spark.sql.legacy.allowNegativeScaleOfDecimal) with toString instead of toPlainString, since the plain form of e.g. 1E+2147483647 does not fit in a String. Other values are rendered as before. QueryExecutionErrors.cannotChangeDecimalPrecisionError, an identical copy, is removed and CastUtils calls DataTypeErrors.cannotChangeDecimalPrecisionError directly.

Why are the changes needed?

setScale takes time proportional to the scale change and throws once it exceeds the Int range:

SELECT CAST('1e-2147483647' AS DECIMAL(10,2));
-- java.lang.ArithmeticException: BigInteger would overflow supported range  (LEGACY, ANSI and TRY)
SELECT CAST('1e-100000000' AS DECIMAL(10,2));
-- 0.00, after ~40s

SET spark.sql.legacy.allowNegativeScaleOfDecimal=true;
SELECT CAST('1e2147483647' AS DECIMAL(10,2));
-- java.lang.ArithmeticException: Underflow

This is the small-side counterpart of the fast-fail added in SPARK-35841/SPARK-37451. With negative scales allowed, Decimal.fromString intentionally skips that fast-fail, so huge values reach changePrecision too.

Does this PR introduce any user-facing change?

Yes.

Casting a string whose value is far below the target scale's ulp now returns the rounded value (e.g. 0.00) instead of throwing ArithmeticException, and returns quickly.
With spark.sql.legacy.allowNegativeScaleOfDecimal=true, casting a string with a huge exponent returns NULL (non-ANSI, try_cast) or throws NUMERIC_VALUE_OUT_OF_RANGE (ANSI) instead of ArithmeticException: Underflow. In that error, a value with more than 1000 trailing zeros (e.g. 1E+2147483647) is shown in scientific notation, since its plain form is too long to build. Other values are shown as before.

No input that previously produced a value changes its result.

How was this patch tested?

New tests in DecimalSuite, CastSuiteBase (so they run in CastWithAnsiOffSuite, CastWithAnsiOnSuite and TryCastSuite), CastWithAnsiOffSuite and CastWithAnsiOnSuite, covering interpreted and codegen evaluation. All of them fail without the fix. The ANSI test also checks the value parameter of the error, in scientific notation for 1e2147483647 and in plain notation for 1e40. DecimalSuite also checks that values around the bounds of both shortcuts, with short and 81-digit mantissas, under every supported rounding mode and with and without negative scales, round exactly like BigDecimal.setScale.

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

Generated-by: Claude Code (Claude Opus 5.5)

This pull request and its description were written by Isaac.

… result is known from the magnitude In the BigDecimal branch of `Decimal.changePrecision`, when the scale change exceeds `DecimalType.MAX_PRECISION`, check the value's number of integral digits before calling `setScale`: a nonzero value with too many integral digits overflows, and a zero or a value below 0.01 ulp of the target scale is replaced by a same-sign value of 0.01 ulp, which rounds the same under every rounding mode. This avoids `ArithmeticException` and very slow rescaling for inputs like '1e-2147483647' and, with negative scales allowed, '1e2147483647'. `NUMERIC_VALUE_OUT_OF_RANGE.WITH_SUGGESTION` now renders a decimal with a negative scale with `toString`, since its plain form can be too long to fit in a String. Co-authored-by: Isaac <no-reply@databricks.com>

dongjoon-hyun

Thank you for the fix, @viirya. The magnitude shortcut in changePrecision itself looks correct to me: a nonzero value with more integral digits than precision - scale overflows under every rounding mode (including ROUND_DOWN, since 10^(p-s) is a multiple of the target ulp), and the remaining setScale work is bounded by the number of digits of the value. I left 6 inline comments. Summary:

Decimal.fromString/fromStringANSI still crash on '1e2147483647' with the default (non-legacy) config, because numDigitsInIntegralPart overflows Int and the existing fast-fail is skipped. try_cast throws ArithmeticException instead of returning NULL.
Decimal.set(decimal, precision, scale), used by the JSON/CSV/XML parsers, still calls setScale without a bound, so from_json with 1e-2147483647 still fails and 1e-100000000 is still slow. A shared rescale helper would cover all entry points.
The error message now uses scientific notation for every negative-scale value (e.g. 1E+2 instead of 100), not only for huge ones.
The ANSI test checks only a message substring, not the error condition or the new value rendering.
A test comment says 0.1 ulp, while the code uses 0.01 ulp.
QueryExecutionErrors.cannotChangeDecimalPrecisionError is now a pure passthrough and can be removed.

dongjoon-hyun · 2026-10-10T07:26:33Z

    if (dv.ne(null)) {
      // We get here if either we started with a BigDecimal, or we switched to one because we would
      // have overflowed our Long; in either case we must rescale dv to the new scale.
+      if (math.abs(dv.scale.toLong - scale) > DecimalType.MAX_PRECISION) {


This uses Long arithmetic for the scale difference and the integral digits, but the existing fast-fail in Decimal.fromString/fromStringANSI still uses numDigitsInIntegralPart, which computes bigDecimal.precision - bigDecimal.scale in Int. For 1e2147483647 (precision 1, scale -2147483647), the result overflows to -2147483648, so the > DecimalType.MAX_PRECISION fast-fail is skipped. With the default spark.sql.legacy.allowNegativeScaleOfDecimal=false, Decimal(bigDecimal) then goes to set(decimal), which calls decimal.setScale(0):

jshell> var b = new java.math.BigDecimal("1e2147483647"); jshell> b.precision() - b.scale() $2 ==> -2147483648 jshell> b.setScale(0) | Exception java.lang.ArithmeticException: BigInteger would overflow supported range

So SELECT try_cast('1e2147483647' AS DECIMAL(10,2)) (and the non-ANSI cast) throws instead of returning NULL, and the ANSI cast throws a raw ArithmeticException instead of NUMERIC_VALUE_OUT_OF_RANGE. '12e2147483646' hits the same overflow. The new tests cover these huge exponents only with the legacy config on. Could you compute numDigitsInIntegralPart in Long as well and add the non-legacy cases to CastSuiteBase?

Thanks. This is the Int overflow in numDigitsInIntegralPart, which is fixed separately in #59334 (SPARK-60118) together with non-legacy CastWithAnsiOffSuite/CastWithAnsiOnSuite cases for 1e2147483647, 12e2147483647, etc. This PR covers the remaining cases that reach changePrecision.

dongjoon-hyun · 2026-10-10T07:26:33Z

+          dv = BigDecimal(dv.signum, scale + 2)
+        }
+      }
      dv = dv.setScale(scale, roundMode)


This shortcut protects only changePrecision. Decimal.set(decimal: BigDecimal, precision: Int, scale: Int) still calls decimal.setScale(scale, ROUND_HALF_UP) without a bound, and it is used by the JSON, CSV and XML parsers (JacksonParser.scala:441/444, UnivocityParser.scala:226, StaxXmlParser.scala:788). For example, from_json('{"a": 1e-2147483647}', 'a DECIMAL(10,2)'), or a CSV/XML column 1e-2147483647 read with a DECIMAL(10,2) schema, still fails with BigInteger would overflow supported range, and 1e-100000000 still takes ~40s per value. set(decimal)'s decimal.setScale(0) has the same issue.

Would it make sense to move this magnitude check into a small shared rescale helper used by changePrecision and both set overloads, so every entry point is covered? If you prefer to keep this PR focused on casts, a separate JIRA would be fine too.

Agreed that set(decimal, precision, scale) and set(decimal) have the same issue. I'd like to keep this PR focused on changePrecision. The parser path also needs a decision on how to report overflow there (NUMERIC_VALUE_OUT_OF_RANGE.WITHOUT_SUGGESTION carries a roundedValue). Filed SPARK-60133 for it.

dongjoon-hyun · 2026-10-10T07:26:33Z

-        "value" -> value.toPlainString,
+        // A negative scale (legacy mode only) can make the plain string arbitrarily long,
+        // e.g. 1E+2147483647.
+        "value" -> (if (value.scale < 0) value.toString else value.toPlainString),


This changes the message for every negative-scale value, not only for the huge ones. With spark.sql.legacy.allowNegativeScaleOfDecimal=true, an overflow of a value stored as unscaled 1 with scale -2 used to show 100 and now shows 1E+2, and the same applies to ordinary values like 1E+40. Since only very large -scale values make the plain string too long, would it be better to switch to toString only above some threshold (e.g. when -value.scale exceeds a reasonable number of digits), so the existing message stays the same for normal values?

Good point. It now uses toString only when the scale is below -1000, so the message for ordinary values like 1E+2 and 1E+40 is unchanged.

dongjoon-hyun · 2026-10-10T07:26:33Z

+      Seq("1e2147483647", "-1e2147483646", "12e2147483647", "1e100000000").foreach { str =>
+        checkExceptionInExpression[ArithmeticException](
+          cast(str, DecimalType(10, 2)),
+          "cannot be represented as Decimal(10, 2)")


This checks only the message substring, so the DataTypeErrors change (rendering a negative-scale value like 1E+2147483647 with toString) is not asserted directly. If that branch were changed later, e.g. to print a truncated plain string or the wrong value, this test would still pass. Could we add a checkError on NUMERIC_VALUE_OUT_OF_RANGE.WITH_SUGGESTION with the expected value parameter for at least one of these inputs?

Added a checkError with the expected value for 1e2147483647 (1E+2147483647) and for 1e40 (plain notation).

dongjoon-hyun · 2026-10-10T07:26:33Z

  }
+
+  test("SPARK-60119: changePrecision with a source scale far from the target scale") {
+    // A value below 0.1 ulp of the target scale rounds to 0 or +/-1 ulp depending only on its


nit. This comment says below 0.1 ulp, while the shortcut in changePrecision applies below 0.01 ulp (numIntegralDigits < -scale.toLong - 1), and its comment says 0.01 ulp. The statement is mathematically true, but it can make readers think values in [0.01, 0.1) ulp take the shortcut. Could you align it with the main code?

dongjoon-hyun · 2026-10-10T07:26:33Z

-        "config" -> toSQLConf(SQLConf.ANSI_ENABLED.key)),
-      context = getQueryContext(context),
-      summary = getSummary(context))
+    DataTypeErrors.cannotChangeDecimalPrecisionError(


nit. Now this method only forwards to DataTypeErrors.cannotChangeDecimalPrecisionError with the same signature. Since its only main-code caller is CastUtils.java:110, we can call DataTypeErrors there directly and remove this wrapper, so that the two copies cannot drift apart again.

…rt it Co-authored-by: Isaac <no-reply@databricks.com>

dongjoon-hyun reviewed Oct 10, 2026

View reviewed changes

viirya mentioned this pull request Oct 10, 2026

[SPARK-60118][SQL] Fix Int overflow in the fast-fail check of casting string to decimal #59334

Open

Keep plain notation for ordinary values in the error message and asse…

d14ad06

…rt it Co-authored-by: Isaac <no-reply@databricks.com>

Conversation

viirya commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

viirya commented Oct 10, 2026 •

edited

Loading