apache / apache/datafusion-comet
Replace small hand-rolled element loops with arrow kernels and arity helpers
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 190
Description
### 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` 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.
Contributor guide
Assessment
This issue has not been assessed yet.