Skip to content

use array_compact and array_repeat from Datafusion #3929

Closed

Description

@comphead

What is the problem the feature request solves?

Followup on #3574 where array_compact and array_repeat implemented in Comet(details in #3629)

Once functions are fixed in DataFusion reuse the implementation from DF

Ref PRs:
apache/datafusion#21558
apache/datafusion#21522

which would be available in DF 54.0.0

Describe the potential solution

No response

Additional context

No response

Activity

  1. self-assigned this
    on Apr 11, 2026
  2. comphead commented on Jun 25, 2026

    @comphead
    ContributorAuthor
  3. mbutrovich commented on Jul 21, 2026

    @mbutrovich
    Contributor

    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.

  4. comphead commented on Jul 22, 2026

    @comphead
    ContributorAuthor

    array_compact gets rewritten in runtime to lambda expression

    lazy val lv = NamedLambdaVariable("arg",
        child.dataType.asInstanceOf[ArrayType].elementType, true)
      lazy val lambda = LambdaFunction(isNotNull(lv), Seq(lv))
    
      override lazy val replacement: Expression = KnownNotContainsNull(ArrayFilter(child, lambda))
    

    So for array_compact we may want to fallback to codegen for now

  5. comphead commented on Jul 22, 2026

    @comphead
    ContributorAuthor

    for native execution it should wait for #4744

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions