apache / apache/datafusion-comet

Implement return_field() for imported Rust UDFs

Open
#5,251 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

Follow-up from review of #4459 ([thread](https://github.com/apache/datafusion-comet/pull/4459#discussion_r3712524816)).

`ImportedCScalarUdf::return_type` already calls the kernel through the C ABI, decodes an `FFI_ArrowSchema` into a full `Field`, and then throws everything but the `DataType` away. The planner separately hardcodes the output field as nullable:

```rust
let return_field = Arc::new(Field::new(&call.name, kernel_return_type, true));
```

So a kernel that reports a non-nullable output, or attaches field metadata, has both discarded.

Implementing `ScalarUDFImpl::return_field` instead would carry the kernel's own `Field` through to the plan, and would let the planner stop fabricating one.

Worth checking before doing it:

- Spark UDF results are nullable in Spark's own schema, so promising DataFusion a non-nullable field needs to not trip a schema comparison at the Comet boundary or in the shuffle writer.
- The declared-vs-actual return type check in `planner.rs` deliberately erases nested nullability, because Spark carries `containsNull` in the type while the delivered array normalizes children to nullable. Whatever this does with top-level nullability should be consistent with that.
- A test for a kernel returning a non-nullable field, since nothing exercises that today.

Contributor guide

Open the contributing guide

Research direction

Start in planner.rs at the declared-versus-actual return type check and the planner code that constructs the nullable output Field. Trace ImportedCScalarUdf::return_type through the C ABI and review the Spark schema and shuffle-writer boundary behavior described in the issue. Done means the kernel's complete Field, including top-level nullability and metadata, reaches the plan without breaking schema checks, with a test for a non-nullable kernel result.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.