mbutrovich commented on issue #3929: URL: https://github.com/apache/datafusion-comet/issues/3929#issuecomment-5039250518
Status update now that DataFusion 54.1.0 is out and the referenced fixes landed. **`array_compact` (ready to retire the Comet UDF).** DataFusion 54.1.0 ships the `array_compact` builtin (apache/datafusion#21522) plus the NULL edge-case fix (apache/datafusion#23192, backported as #23196). I diffed our `SparkArrayCompact` (`native/spark-expr/src/array_funcs/array_compact.rs`) against DF's `functions-nested/src/array_compact.rs` and they are functionally equivalent: per-row null-element removal, null-row passthrough, and `logical_nulls()` handling so `NullArray` elements are dropped. So the reason we forked (`array_remove_all(arr, null)` nulling the whole row) no longer applies, and we can delegate to the builtin and delete our UDF plus its `jni_api` registration. One nuance to cover with a fuzz check before removing: DF's `array_compact` implements `return_type` (DataType only), while ours uses `return_field_from_args` to preserve the input field's nullability and metadata. At the shuffle boundary that difference can surface as list-element nullability drift, which `SchemaAlignExec` would then have to correct. Worth confirming `CometFuzzTestSuite` stays green across the swap. **`array_repeat` (separate, not blocked on the same fix).** DF's null handling fix (apache/datafusion#21558, and the coercion fix apache/datafusion#23071 backported as #23629) is against `datafusion-spark`'s `SparkArrayRepeat`. Comet does not use that path today: we deliberately do not register `SparkArrayRepeat` and instead call the `functions-nested` `ArrayRepeat`, wrapping it in a Scala `CaseWhen` to return NULL when the count is NULL (matching Spark). Retiring that wrapper means switching to `SparkArrayRepeat` and confirming it matches Spark on both null-element and null-count semantics. That is a distinct piece of work from `array_compact`. So `array_compact` is the clear, low-risk first step (equivalent implementations, delete the UDF and its `jni_api` registration), and `array_repeat` can be evaluated on top of it. -- 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]
