You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@datafusion.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2026/09/04 13:14:25 UTC

[PR] fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue [datafusion-comet]

peterxcli opened a new pull request, #5684:
URL: https://github.com/apache/datafusion-comet/pull/5684

   ## 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^scale` and 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 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`; `CAST(16777217.0000000001 AS FLOAT)` gave `16777216` instead of `16777218`).
   
   ## What changes are included in this PR?
   
   - New `Decimal128 -> Float64/Float32` kernels in `numeric.rs` mirroring 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^53` and `scale <= 22` for f64, `<= 2^24` / `<= 10` for 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 like `BigDecimal`.
   - `cast.rs` routes `(Decimal128, Float32|Float64)` to the kernels for LEGACY, ANSI and TRY (the conversion cannot fail); `Float32/Float64` are removed from `is_df_cast_from_decimal_spark_compatible`.
   
   ## How are these changes tested?
   
   - Rust unit tests: the issue's values, ties-to-even cases (`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 carry `assert_ne!` guards showing the old `(unscaled as f64)/10^scale` formula fails on the issue's values. Reference values were cross-checked with `jshell` (`BigDecimal.doubleValue()/floatValue()`). `cargo test -p datafusion-comet-spark-expr`: 663 passed; `cargo clippy --all-targets --workspace -- -D warnings` clean.
   - `CometNativeCastSuite`: `DECIMAL(38,18)` values from the issue added to the Float/Double tests, new `DECIMAL(38,10) -> Float/Double` tests (including `16777217.0000000001`), and `DecimalType(38,18)` re-enabled in the array and nested-array cast matrices that #4278 had excluded for this mismatch. Suite passes on Spark 4.1.3 (170/170, 8 pre-existing ignores).
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.

sunchao merged PR #5684:
URL: https://github.com/apache/datafusion-comet/pull/5684


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.

sunchao commented on PR #5684:
URL: https://github.com/apache/datafusion-comet/pull/5684#issuecomment-5554794610

   Merged, thanks @peterxcli !


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue [datafusion-comet]

Posted by "sunchao (via GitHub)" <gi...@apache.org>.

sunchao commented on code in PR #5684:
URL: https://github.com/apache/datafusion-comet/pull/5684#discussion_r3936454700


##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -863,6 +865,103 @@ pub(crate) fn spark_cast_decimal_to_boolean(array: &dyn Array) -> SparkResult<Ar
     Ok(Arc::new(result.finish()))
 }
 
+/// Powers of ten that are exactly representable as `f64` (`10^n = 2^n * 5^n` and `5^22 < 2^53`);
+/// the same table as Java's `BigDecimal.DOUBLE_10_POW`.
+const F64_EXACT_POW10: [f64; 23] = [
+    1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10, 1e11, 1e12, 1e13, 1e14, 1e15, 1e16,
+    1e17, 1e18, 1e19, 1e20, 1e21, 1e22,
+];
+
+/// Powers of ten that are exactly representable as `f32` (`5^10 < 2^24`); the same table as
+/// Java's `BigDecimal.FLOAT_10_POW`.
+const F32_EXACT_POW10: [f32; 11] = [1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10];
+
+/// Largest integer magnitude that converts to `f64` without rounding.
+const F64_EXACT_INT_LIMIT: u128 = 1 << 53;
+
+/// Largest integer magnitude that converts to `f32` without rounding.
+const F32_EXACT_INT_LIMIT: u128 = 1 << 24;
+
+/// Converts a Decimal128 value to the `f64` nearest to its exact decimal value (ties to even),
+/// matching Spark's `Decimal.toDouble`, i.e. `java.math.BigDecimal.doubleValue()`.
+///
+/// DataFusion's kernel computes `(unscaled as f64) / 10^scale`, which rounds the unscaled value
+/// first and the quotient second; once `|unscaled| > 2^53` (any `DECIMAL(38,18)` value of
+/// magnitude `>= 0.01`) the two roundings can land one ulp away from the correctly rounded
+/// result, e.g. `12345.6789` becomes `12345.678899999999`.
+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 {

Review 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](https://doc.rust-lang.org/reference/expressions/operator-expr.html#numeric-cast). 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. Ple
 ase add scale-zero FLOAT/DOUBLE microbenchmark cases, including nulls, to keep this fast path covered.
   



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: correctly rounded decimal to double/float cast matching BigDecimal.doubleValue/floatValue [datafusion-comet]

Posted by "peterxcli (via GitHub)" <gi...@apache.org>.

peterxcli commented on code in PR #5684:
URL: https://github.com/apache/datafusion-comet/pull/5684#discussion_r3937522721


##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -863,6 +865,103 @@ pub(crate) fn spark_cast_decimal_to_boolean(array: &dyn Array) -> SparkResult<Ar
     Ok(Arc::new(result.finish()))
 }
 
+/// Powers of ten that are exactly representable as `f64` (`10^n = 2^n * 5^n` and `5^22 < 2^53`);
+/// the same table as Java's `BigDecimal.DOUBLE_10_POW`.
+const F64_EXACT_POW10: [f64; 23] = [
+    1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10, 1e11, 1e12, 1e13, 1e14, 1e15, 1e16,
+    1e17, 1e18, 1e19, 1e20, 1e21, 1e22,
+];
+
+/// Powers of ten that are exactly representable as `f32` (`5^10 < 2^24`); the same table as
+/// Java's `BigDecimal.FLOAT_10_POW`.
+const F32_EXACT_POW10: [f32; 11] = [1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10];
+
+/// Largest integer magnitude that converts to `f64` without rounding.
+const F64_EXACT_INT_LIMIT: u128 = 1 << 53;
+
+/// Largest integer magnitude that converts to `f32` without rounding.
+const F32_EXACT_INT_LIMIT: u128 = 1 << 24;
+
+/// Converts a Decimal128 value to the `f64` nearest to its exact decimal value (ties to even),
+/// matching Spark's `Decimal.toDouble`, i.e. `java.math.BigDecimal.doubleValue()`.
+///
+/// DataFusion's kernel computes `(unscaled as f64) / 10^scale`, which rounds the unscaled value
+/// first and the quotient second; once `|unscaled| > 2^53` (any `DECIMAL(38,18)` value of
+/// magnitude `>= 0.01`) the two roundings can land one ulp away from the correctly rounded
+/// result, e.g. `12345.6789` becomes `12345.678899999999`.
+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 {

Review Comment:
   Addressed in d410f61d8. Both helpers now handle scale zero before the magnitude guard with direct target-width casts (`i128 as f64` and `i128 as f32`). I also added `cast_numeric` Criterion cases for DECIMAL(18,0) to DOUBLE, DECIMAL(38,0) to DOUBLE, and DECIMAL(12,0) to FLOAT, each with and without nulls. A local release run measured 1.71-1.99 ns/input slot across the six cases. `cargo test -p datafusion-comet-spark-expr --lib` passed 663 tests, and package Clippy with all targets passed with warnings denied.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org