andygrove opened a new issue, #5091: URL: https://github.com/apache/datafusion-comet/issues/5091
### What is the problem the feature request solves? A number of small hand-rolled element loops re-implement work that arrow kernels or arity helpers already provide, with identical results and null handling: - `native/spark-expr/src/conversion_funcs/numeric.rs` (`spark_cast_decimal_to_boolean`): per-row builder loop emitting `!value.is_zero()`. Replaceable with `arrow::compute::kernels::cmp::neq` against a zero-decimal `Scalar` of the same precision/scale (arrow's cast does not support Decimal-to-Boolean, but the comparison kernel is exactly equivalent and infallible). - `native/spark-expr/src/array_funcs/array_insert.rs`: builds validity masks by collecting `(0..num_rows).map(|row| arr.is_valid(row))` into `Vec<bool>` twice. Replaceable with `boolean::is_not_null` and `boolean::and` at the bitmap level. - `native/spark-expr/src/kernels/temporal.rs` (`DAYS_TO_UNIX_EPOCH` + `days_to_date`): re-derives days-since-epoch to `NaiveDate` with a hand-maintained epoch constant. Replaceable with `Date32Type::to_naive_date_opt`, already used elsewhere in the crate. - `native/spark-expr/src/math_funcs/pow.rs`: four-way scalar/array match with iterator zip/map/collect loops. The array/array arm is exactly `arity::binary` over the existing `spark_powf` closure; the dispatch can collapse via the Datum idiom already used in `modulo_expr.rs`. - `native/spark-expr/src/math_funcs/internal/make_decimal.rs`: per-row `Decimal128Builder` loop. Replaceable with `unary_opt` (note: not arrow's Int64-to-Decimal cast, which would rescale; MakeDecimal reinterprets the unscaled value). - `native/spark-expr/src/agg_funcs/covariance.rs` (`update_batch`/`retract_batch`): fragile dual-array null re-synchronization using `iter().flatten()` plus `is_valid(i)`. The sibling `correlation.rs` already shows the clean idiom: `and(is_not_null(a), is_not_null(b))` + `filter`, then iterate dense values. - `native/core/src/execution/columnar_to_row.rs` (`maybe_cast_to_schema_type`): Int32/Int64-to-Decimal128 reinterpretation via `iter().map().collect()`. Replaceable with `arity::unary`, which reuses the null buffer zero-copy (arrow's cast is not usable here since it would rescale the value). ### Describe the potential solution Apply the listed kernel/arity replacements. Each is behavior-preserving; most also remove per-row branching or allocation. ### Additional context These are grouped because each change is small; they can land as one PR or a few. Found during an audit of native code that replicates existing arrow-rs kernels. -- 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]
