sunchao commented on code in PR #5367:
URL: https://github.com/apache/datafusion-comet/pull/5367#discussion_r3836655613
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -852,15 +853,13 @@ pub(crate) fn spark_cast_int_to_int(
pub(crate) fn spark_cast_decimal_to_boolean(array: &dyn Array) ->
SparkResult<ArrayRef> {
let decimal_array = array.as_primitive::<Decimal128Type>();
- let mut result = BooleanBuilder::with_capacity(decimal_array.len());
- for i in 0..decimal_array.len() {
- if decimal_array.is_null(i) {
- result.append_null()
- } else {
- result.append_value(!decimal_array.value(i).is_zero());
- }
- }
- Ok(Arc::new(result.finish()))
+ // Arrow has no Decimal-to-Boolean cast. `neq` against a zero of the same
+ // precision/scale is exactly `!value.is_zero()`, including null handling.
+ let zero = Scalar::new(
+ Decimal128Array::from(vec![0i128])
+ .with_precision_and_scale(decimal_array.precision(),
decimal_array.scale())?,
Review Comment:
[P2] Preserve non-null precision-zero results from JVM codegen
Thanks for adding the all-null fast path. There is one non-null route I
missed in the earlier report: a Java UDF returning `java.math.BigInteger.ZERO`
with declared `DecimalType(0, 0)`. Spark's converter stores that zero as a
compact decimal, and Comet's default JVM-UDF codegen writes it through
[`DecimalVector.setSafe(index,
long)`](https://github.com/apache/datafusion-comet/blob/4de30532f6e91bbc0b3a1b2988940e218c6f693c/spark/src/main/scala/org/apache/comet/codegen/CometBatchKernelCodegenOutput.scala#L233-L241).
That overload does not validate precision, so this array can reach the native
cast.
For a batch containing zero and null, the base implementation returns
`[false, null]`, but this head's zero-scalar construction throws `precision
cannot be 0, has to be between [1, 38]` in Legacy, ANSI, and TRY modes. The
all-null guard does not cover the valid zero slot.
I verified the Spark 4.1.3 converter and Arrow Java 18.3 writer, then
imported the same array layout through Arrow 58.4 FFI and compared the exact
base and head cast implementations. This is component-level reproduction, not
an end-to-end SQL run. `BigInteger.ZERO` matters here: returning
`BigDecimal.ZERO` does not take the same Spark conversion path.
Could we preserve the raw-value cast path for precision-zero arrays and add
a JVM-UDF regression? A useful case is `CAST(zero_decimal(id) AS BOOLEAN)`,
where a Java `UDF1[Long, BigInteger]`, explicitly declared as `DecimalType(0,
0)`, returns `BigInteger.ZERO` for one input row and null for another.
--
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]
