Pass ConfigOptions to scalar UDFs via FFI#20454
Pass ConfigOptions to scalar UDFs via FFI#20454timsaucer wants to merge 2 commits intoapache:mainfrom
Conversation
44f3a91 to
da8080d
Compare
There was a problem hiding this comment.
Pull request overview
This PR enables passing ConfigOptions to scalar UDFs via the FFI (Foreign Function Interface), resolving issue #17035. The changes allow external FFI libraries to access session configuration options when executing scalar functions, which was previously blocked by the TODO comment about passing config options.
Changes:
- Added
config_optionsparameter to FFI scalar UDF invocation signature - Switched from returning
WrappedArraytoFFI_ColumnarValuefor better scalar/array handling - Added comprehensive test validating config option passing through FFI boundary
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| datafusion/ffi/tests/ffi_udf.rs | Added test test_config_on_scalar_udf validating that config options (specifically timezone) are correctly passed to FFI UDFs |
| datafusion/ffi/src/udf/mod.rs | Updated FFI scalar UDF signatures to include config_options parameter, converted between FFI and native ConfigOptions, switched to FFI_ColumnarValue return type |
| datafusion/ffi/src/tests/udf_udaf_udwf.rs | Implemented TimeZoneUDF test helper that returns the configured timezone |
| datafusion/ffi/src/tests/mod.rs | Added create_timezone_udf function to the foreign library module exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@alamb Asking for your review since you put up the issue, but I can try to find someone who's spent more time on the FFI code if you like |
davisp
left a comment
There was a problem hiding this comment.
+1
It's a straightforward addition now that FFI_ConfigOptions exists.
Which issue does this PR close?
ConfigOptionsto scalar functions via FFI #17035Rationale for this change
Now that we have proper
FFI_ConfigOptionswe can pass these to scalar UDFs via FFI.What changes are included in this PR?
Instead of passing default options, pass in converted config options from the input.
Also did a drive by cleanup of switching to using FFI_ColumnarValue since it is now available.
Are these changes tested?
Unit test added.
Are there any user-facing changes?
This is a breaking API change, but not one that users will interact with directly. It breaks the ABI for FFI libraries, which is currently unstable.