Skip to content

[GLUTEN-12356][VL] Preserve precision for long decimal to double cast#12605

Draft
Smallfu666 wants to merge 3 commits into
apache:mainfrom
Smallfu666:contrib/issue-12356
Draft

[GLUTEN-12356][VL] Preserve precision for long decimal to double cast#12605
Smallfu666 wants to merge 3 commits into
apache:mainfrom
Smallfu666:contrib/issue-12356

Conversation

@Smallfu666

@Smallfu666 Smallfu666 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Fixes #12356.

Velox converts a long decimal's unscaled int128 value to double before applying the decimal scale. For values whose unscaled representation exceeds the precision of double, this can produce a result that differs from Spark, which rounds the exact decimal value once (BigDecimal.doubleValue).

This patch keeps the workaround Velox-specific and:

  • converts long decimals (precision greater than 18) through their exact string representation before casting to double;
  • pins the rewrite scope with a unit test on genCastWithNewChild: precision <= 18 keeps the direct conversion, precision > 18 goes through the string representation;
  • adds a parquet-backed direct-cast regression matrix across DECIMAL(18,6), DECIMAL(19,0), DECIMAL(38,0), DECIMAL(38,19), DECIMAL(38,38), covering positive/negative/zero/NULL values, values around 2^53, and near-max 38-digit values, with expected results taken from vanilla Spark, under both ANSI on and off;
  • keeps the regression test for the reported decimal-division query (constant folding disabled, spark.sql.decimalOperations.allowPrecisionLoss=false);
  • verifies the expression remains offloaded through ProjectExecTransformer.

Performance

Micro-benchmark: 10M-row parquet DECIMAL(38,19) column, SELECT SUM(CAST(c AS DOUBLE)), local[8], 3 warmups + 5 measured trials per configuration, medians reported. The pre-patch direct path was emulated in the same binary by temporarily gating the rewrite behind a config (not part of this PR). Each run first executed the issue's query as a correctness probe to prove which path was active: patched and vanilla Spark returned 214.4, the direct path returned 214.39999999999998.

path scan-inclusive cached (cast-dominated) issue probe matches Spark
Gluten, patched (via string) 386 ms 329 ms yes (214.4)
Gluten, pre-patch (direct) 154 ms 107 ms no (214.39999999999998)
Vanilla Spark 990 ms 895 ms yes (214.4)
  • On this cast-dominated microbenchmark the rewrite costs ~2.5x end-to-end and ~3.1x cast-focused relative to the (incorrect) direct conversion.
  • The patched path remains ~2.6x faster than vanilla Spark for the same issue-probe result; Spark's own long-decimal cast is expensive for the same reason (exact decimal semantics).
  • A DECIMAL(18,6) control column is unaffected (within noise), confirming the rewrite only applies to precision > 18.
  • Correctness is asserted on the issue's probe query (a single cast), not on the 10M-row SUM aggregate: floating-point aggregation order differs between engines, so the SUM values agree only to the last ulp (patched Gluten 4.999999513013628E13 vs vanilla Spark 4.999999513013627E13) and are not expected to be bit-identical. Per-row cast equality against the vanilla Spark oracle is covered by the regression tests in this PR.

The better long-term fix is a per-row fast path in Velox's applyDecimalToFloatCast (direct conversion when the unscaled value is exactly representable in double, exact string otherwise), which would recover most of this cost; this PR keeps a narrow Spark-compatible workaround in Gluten until such a fix lands upstream. If preferred, the rewrite can also be gated behind a config for a staged rollout.

How was this patch tested?

  • Reproduced the issue on current main with Spark 3.5: vanilla Spark returned 214.4, while Gluten/Velox returned 214.39999999999998.
  • Ran MathFunctionsValidateSuite and its ANSI variant with the Velox backend: 46 tests passed, 0 failed (including the 2 new tests above).
  • The direct-cast matrix compares Gluten output against vanilla Spark as the oracle rather than recomputing expected values with the same conversion.
  • Ran Scala formatting and git diff --check.

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

Generated-by: OpenAI Codex GPT-5.6 Luna; Claude Opus 4.8

@github-actions github-actions Bot added the VELOX label Jul 23, 2026
Smallfu666 and others added 2 commits July 23, 2026 18:56
Pin the rewrite scope with a unit test on genCastWithNewChild
(precision <= 18 keeps the direct conversion, > 18 goes through the
exact string representation), and add a parquet-backed direct-cast
matrix across DECIMAL(18,6)/(19,0)/(38,0)/(38,19)/(38,38) covering
positive/negative/zero/NULL values, values around 2^53, and near-max
38-digit values, compared against vanilla Spark under both ANSI on
and off.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cast.evalMode does not exist before Spark 3.4 (it replaced the
ansiEnabled constructor arg in SPARK-40389), so the 4-arg
Cast(child, StringType, timeZoneId, evalMode) form fails to compile
on Spark 3.3. Use the 3-arg Cast instead and keep the outer cast via
withNewChildren: decimal-to-string is lossless regardless of eval
mode, so the original ANSI semantics are preserved by the outer cast.

Verified: Spark 3.3 test-compile passes; MathFunctionsValidateSuite
(Spark 3.5, Velox) 46/46.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Smallfu666

Copy link
Copy Markdown
Contributor Author

Good catch — Cast.evalMode was added in Spark 3.4 (SPARK-40389, replacing the ansiEnabled arg), so the 4-arg form broke the 3.3 build.

Fixed in 2601543 with the 3-arg Cast, keeping the outer cast via withNewChildren. decimal-to-string is lossless regardless of eval mode, so the outer double cast still carries the ANSI semantics.

Verified: Spark 3.3 test-compile passes, and MathFunctionsValidateSuite (Spark 3.5, Velox) stays 46/46.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Floating-point precision issues

1 participant