Amogh-2404 opened a new pull request, #22608: URL: https://github.com/apache/datafusion/pull/22608
## Which issue does this PR close? - Part of #22330. This is the first of the per-method PRs that issue describes. It plumbs `placement` only; the remaining defaulted methods follow separately, so the umbrella issue stays open. ## Rationale for this change `FFI_ScalarUDF` (`datafusion/ffi/src/udf/mod.rs`) carried no function pointer for `placement`, and `ForeignScalarUDF` did not override it, so a producer's override of `ScalarUDFImpl::placement` (default body at `datafusion/expr/src/udf.rs:1028`) was dropped on the consumer side and every foreign UDF fell back to `KeepInPlace`. A UDF loaded over FFI never delivered its leaf-pushdown hint to the optimizer. ## What changes are included in this PR? - New `FFI_ExpressionPlacement` enum bridge in `datafusion/ffi/src/placement.rs`, in the shape of `FFI_Volatility`: `#[repr(u8)]` with `From` impls both ways and a round-trip test over every variant. - A `placement` function pointer on `FFI_ScalarUDF`, populated in the `From<Arc<ScalarUDF>>` constructor, with `placement_fn_wrapper` on the producer side and a forwarding `ForeignScalarUDF::placement` on the consumer side. `placement` is infallible, so the pointer returns the enum directly rather than `FFI_Result`. Adding a field to the `#[repr(C)]` struct changes its layout, so this is an API change (label applied, base `main`). `display_name` is also on the issue's list, but it has been deprecated since 50.0.0, so it should be dropped from the gap list rather than plumbed. I have left it and the remaining methods to follow-up PRs. ## Are these changes tested? Yes. - Unit: a round-trip test over all four `ExpressionPlacement` variants, plus a forced-foreign test (`mock_foreign_marker_id`) using a UDF whose `placement` override depends on its arguments. The assertions cover ordered, reordered, and empty argument slices, so argument marshalling is checked, not just the return value. - Integration: `tests/ffi_udf.rs` loads the UDF from the real cdylib and asserts the override survives the boundary, which is the surface a layout change needs. Run with `cargo test -p datafusion-ffi` and `cargo test -p datafusion-ffi --features integration-tests`. ## Are there any user-facing changes? A `placement` override on a `ScalarUDFImpl` is now preserved across the FFI boundary instead of being silently replaced by the default. This is an ABI change to `FFI_ScalarUDF`; consumers must be recompiled against the new layout. -- 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]
