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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
