apache / apache/datafusion-comet

Add a forward-flexible property slot to the Comet UDF C ABI

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

Description

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

The C ABI today exposes exactly `function_name`, `new_impl`, `init`, `execute`, `get_last_error` and `release`. Anything else a kernel might want to tell the host, volatility, `Display`/`Debug` renderings, documentation, aliases, needs a new struct field and a `COMET_UDF_ABI_VERSION` bump, which breaks every existing cdylib.

@paleolimbot suggested a generic property getter instead, of the shape they are already using for table providers, exec plans and expressions:

```rust
pub get_property: Option<
unsafe extern "C" fn(
*mut CometCScalarKernelImpl,
property: *const c_char,
args: *const c_char,
out: *mut FFI_ArrowArray,
) -> c_int,
>,
```

New properties then become new strings rather than new ABI, and an older cdylib answers "unsupported" for a property it does not know.

The awkward part, as noted in the thread, is returning a variable-length string: using an `FFI_ArrowArray` for it works and is what the suggestion does, but is arguably overkill.

Adding the field itself is an ABI change, so it wants to land before anyone depends on the current layout, or ride along with another version bump. Volatility (#5249) is the first concrete consumer.

Contributor guide

Open the contributing guide

Research direction

Start with the existing Comet UDF C ABI and compare the property getter shape used for table providers, exec plans, and expressions, following the review thread from #4459. Confirm how unsupported properties and variable-length string results should be represented, then account for the ABI versioning concern and the volatility consumer in #5249.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, rust
Domain
api, backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.