fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue - #5684
Conversation
…al.doubleValue/floatValue CAST(decimal AS DOUBLE/FLOAT) was delegated to arrow-cast, which computes `(unscaled as f64) / 10^scale` and therefore rounds twice (three times for FLOAT, via the f64 intermediate). Spark's Decimal.toDouble/toFloat use BigDecimal.doubleValue()/floatValue(), which round the exact decimal once, so the results differed in the last ulp whenever |unscaled| > 2^53 — for DECIMAL(38,18) that is essentially every value of magnitude >= 0.01 (12345.6789 became 12345.678899999999). Add Decimal128 -> Float64/Float32 kernels that mirror Java's algorithm: a single IEEE division/multiplication when both the unscaled value and the power of ten are exact in the target type, otherwise a correctly rounded parse of the exact decimal (`<unscaled>e<-scale>`) straight into the target width. Route (Decimal128, Float32|Float64) to them in cast.rs for every eval mode and drop Float32/Float64 from is_df_cast_from_decimal_spark_compatible. Add Rust unit tests (issue values, ties, extremes, negative scales, nulls in every eval mode, and a 30k-value randomized fast-path vs exact-path check), extend CometNativeCastSuite with the issue's DECIMAL(38,18) and DECIMAL(38,10) values, and re-enable DecimalType(38,18) in the array cast matrices that PR apache#4278 had excluded for this mismatch. Closes apache#5670 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sunchao
left a comment
There was a problem hiding this comment.
Correctness
The prior Arrow path converts the coefficient to f64 before applying the decimal scale, and narrows through f64 for FLOAT. Those intermediate roundings can differ from Spark's decimal conversion. This change gives Decimal128-to-DOUBLE/FLOAT dedicated kernels: arithmetic when both operands are exact in the target width, otherwise direct parsing of the exact decimal value. The new dispatch also reaches decimal values inside recursive casts, and the JVM tests restore high-precision decimal coverage.
I compared the implementation with the maintained Spark 3.5 and 4.0 branches (5947fd6e74a1 and 03f28fc43180). Both interpreted and generated casts reach Decimal.toDouble / toFloat, which use BigDecimal. The separate target-width conversions avoid the FLOAT double-rounding problem. The arithmetic bounds are sufficient for one correctly rounded operation, and the parser preserves the coefficient and scale exactly. No correctness finding remained after checking signs, zero, ties, precision boundaries, nulls, empty/sliced arrays, and the scalar and recursive dispatch paths.
LEGACY, ANSI and TRY share these decimal-to-floating semantics; floating overflow produces infinity rather than a decimal overflow error. Negative scales are supported when spark.sql.legacy.allowNegativeScaleOfDecimal is enabled, so FLOAT infinity is reachable there. FLOAT subnormals are reachable at scale 38. DOUBLE overflow/subnormals are outside the Decimal128/i8-scale range. Other source/target fallback decisions are unchanged; removing FLOAT/DOUBLE from the Arrow-compatibility matrix accompanies the dedicated dispatch.
Validation and scope
Reviewed HEAD f977e8f4a2ec74bd7a598f02586d5974e8afadd5 against BASE 81d637b9bf40a5be6f4f0c65ad6f497b34746e69. An isolated harness containing the unchanged new kernels matched Java BigDecimal 26.0.1 and an independent integer-rational reference for 367,682 inputs at both widths, with zero bit mismatches; 128 Arrow array checks passed. Cases distinguish valid Spark decimals from extended i128/i8 stress inputs. This was component validation using Arrow 58.4.0 and its own lockfile, not a local full Comet or Spark build.
CI ran merge b561865f30998ca62872747195d7af25510e94de, whose parents are the exact base/head above. The changed files and native lockfile are identical to HEAD. The Rust job passed all four added tests (1,115 passed, 4 skipped overall). Successful Spark 3.5/JDK 17 and Spark 4.0/JDK 21 logs include the decimal-to-FLOAT/DOUBLE cases and array-cast suite. The review snapshot at 2026-09-04T17:24:25.103Z contains 65 successful and 9 skipped checks.
Performance
One P2 finding: handle scale zero before the exact-integer magnitude guard in both scalar helpers. Larger DECIMAL(p,0) coefficients currently incur formatting and parsing for each value even though a direct cast to the target float already performs the required single rounding. The inline comment identifies the branch and proposed change.
Matched Arrow-array benchmarks measured the following median nanoseconds per input slot, including output allocation:
| Cast, no nulls | Prior Arrow | PR kernel | Scale-zero direct-cast candidate |
|---|---|---|---|
| DECIMAL(18,0) → DOUBLE | 1.53 | 33.49 | 1.53 |
| DECIMAL(38,0) → DOUBLE | 1.51 | 59.27 | 1.52 |
| DECIMAL(12,0) → FLOAT | 1.53 | 35.01 | 1.63 |
The candidate remained about 18× and 31× faster for the two DOUBLE datasets with approximately 20% nulls. Direct target casts matched the PR, Java and the rational reference on 162,083 scale-zero inputs, including midpoint cases. FLOAT must cast directly from i128 to f32; using the old f64 intermediate can reintroduce rounding differences.
These are component timings on Apple M5 Max, rustc 1.97.1 release and Arrow 58.4.0: 8,192-element arrays, nine alternating-order rounds, varied signed inputs, black-boxed inputs/outputs, and at least 25 ms calibration per strategy. They are not end-to-end Spark speedups. The candidate keeps nonzero-scale behavior unchanged, with similar control timings. Parsing on the general nonzero-scale path also costs substantially more than the prior arithmetic, but that arithmetic is not a correctness-preserving replacement.
Design
The dedicated kernels are a sensible boundary for Spark-specific rounding semantics. Keeping FLOAT separate from DOUBLE is necessary, and the exact-arithmetic fast path plus exact-decimal fallback is straightforward to audit. The scale-zero change is a small local improvement within this design; it requires no new dependency or conversion framework.
The fixed 48-byte buffer is sufficient: an i128 sign and 39 digits, exponent separator, and four exponent characters require at most 45 bytes. Widening the scale before negation handles i8::MIN. Valid ASCII decimal literals make the private parser's error expectations defensible. Negative scales work even though one comment describes only the default nonnegative-scale case.
Abstraction & complexity
The two small Arrow wrappers share one private formatter/parser and reuse Arrow's null-preserving unary operation. This keeps the Spark-specific behavior close to the conversion dispatch without introducing wider state or indirection. The generic parser has only the two intended float instantiations, which earns the small amount of abstraction. No additional design or abstraction finding was identified.
| pub(crate) fn decimal128_to_f64(unscaled: i128, scale: i8) -> f64 { | ||
| // Fast path (also Java's): when the unscaled value and the power of ten are both exact | ||
| // doubles, a single IEEE division or multiplication is correctly rounded. | ||
| if unscaled.unsigned_abs() <= F64_EXACT_INT_LIMIT { |
There was a problem hiding this comment.
Performance
[P2] Handle scale zero before the exact-integer guard
Please handle scale == 0 before this magnitude guard in both helpers. Above 2^53 (2^24 for FLOAT), DECIMAL(p,0) currently formats and parses every value, although a direct i128 as f64 / i128 as f32 already performs the required single, correctly rounded conversion. In matched 8,192-element Arrow 58.4.0 array benchmarks (M5 Max, rustc 1.97.1 release), DECIMAL(18,0) → DOUBLE rose from 1.53 ns/input slot in Arrow to 33.49 ns here; handling scale zero first took 1.53 ns. For DECIMAL(38,0), this was 59.27 ns versus 1.52 ns, with the gain persisting with nulls. Direct target casts matched HEAD, Java BigDecimal and an exact rational reference on 162,083 scale-zero inputs, including ties. This avoids the regression while retaining the exact nonzero-scale path. FLOAT should cast directly to f32, without an f64 intermediate. These are component timings, not end-to-end Spark speedups. Please add scale-zero FLOAT/DOUBLE microbenchmark cases, including nulls, to keep this fast path covered.
Which issue does this PR close?
Closes #5670.
Rationale for this change
CAST(decimal AS DOUBLE/FLOAT)was delegated to arrow-cast, which computes(unscaled as f64) / 10^scaleand rounds twice (three times for FLOAT, via the f64 intermediate). Spark'sDecimal.toDouble/toFloatuseBigDecimal.doubleValue()/floatValue(), which round the exact decimal once, so results differed in the last ulp whenever|unscaled| > 2^53— forDECIMAL(38,18)that is essentially every value of magnitude >= 0.01 (12345.6789became12345.678899999999;CAST(16777217.0000000001 AS FLOAT)gave16777216instead of16777218).What changes are included in this PR?
Decimal128 -> Float64/Float32kernels innumeric.rsmirroring Java's algorithm: a single IEEE division/multiplication when the unscaled value and the power of ten are both exact in the target type (|unscaled| <= 2^53andscale <= 22for f64,<= 2^24/<= 10for f32), otherwise a correctly rounded parse of the exact decimal<unscaled>e<-scale>from a stack buffer straight into the target width (never via f64 for FLOAT). Negative scales and out-of-range floats (±Infinity) behave likeBigDecimal.cast.rsroutes(Decimal128, Float32|Float64)to the kernels for LEGACY, ANSI and TRY (the conversion cannot fail);Float32/Float64are removed fromis_df_cast_from_decimal_spark_compatible.How are these changes tested?
2^53+1,2^24+1), 38-digit extremes,i128::MIN/MAX, zero, negative scales, null preservation in every eval mode, and a 30k-value randomized check that the fast paths match the correctly rounded exact path; the tests carryassert_ne!guards showing the old(unscaled as f64)/10^scaleformula fails on the issue's values. Reference values were cross-checked withjshell(BigDecimal.doubleValue()/floatValue()).cargo test -p datafusion-comet-spark-expr: 663 passed;cargo clippy --all-targets --workspace -- -D warningsclean.CometNativeCastSuite:DECIMAL(38,18)values from the issue added to the Float/Double tests, newDECIMAL(38,10) -> Float/Doubletests (including16777217.0000000001), andDecimalType(38,18)re-enabled in the array and nested-array cast matrices that test: enable nested array cast coverage #4278 had excluded for this mismatch. Suite passes on Spark 4.1.3 (170/170, 8 pre-existing ignores).