timsaucer opened a new pull request, #24690:
URL: https://github.com/apache/datafusion/pull/24690
## Which issue does this PR close?
- Closes #24688.
## Rationale for this change
A foreign library that holds an FFI session reference cannot call
`Session::create_physical_plan` for any plan that references a custom table
provider or other extension node. Planning fails with:
> This feature is not implemented: LogicalExtensionCodec is not provided
`FFI_SessionRef` already carries a `logical_codec`, and every neighboring
method consults it — `optimize`, `create_physical_expr`, `catalog_list`, and
`query_planner` all do. `create_physical_plan` was the only one that did not,
so both sides of the boundary serialized the logical plan with the codec-less
helpers: `ForeignSession::create_physical_plan` called
`logical_plan_to_bytes`
and `create_physical_plan_fn_wrapper` called `logical_plan_from_bytes`.
This affects query planners, table providers, UDTFs, and table provider
factories that receive the session reference over FFI. It was found while
reviewing the FFI query planner work in datafusion-python, where codecs were
confirmed to reach foreign planners correctly through `optimize()` but not
through `create_physical_plan()` — an oversight rather than a design choice.
## What changes are included in this PR?
In `datafusion/ffi/src/session/mod.rs`:
- `ForeignSession::create_physical_plan` now serializes with
`logical_plan_to_bytes_with_extension_codec`, using the session's
`logical_codec`.
- `create_physical_plan_fn_wrapper` now deserializes with
`logical_plan_from_bytes_with_extension_codec`. The codec is extracted from
`session.logical_codec` before `session.inner()` shadows the binding.
- Removed the now-unused `logical_plan_to_bytes` / `logical_plan_from_bytes`
imports.
This matches what `optimize_fn_wrapper` and `ForeignSession::optimize`
already do.
## Are these changes tested?
Yes. A new unit test,
`session::tests::test_create_physical_plan_uses_logical_codec`, registers a
`MemTable` that can only be serialized by a custom codec, wraps the session
in
an `FFI_SessionRef` carrying that codec, and plans a scan of it through
`ForeignSession::create_physical_plan`.
The test was verified to reproduce the bug: with either half of the fix
reverted it fails with `NotImplemented("LogicalExtensionCodec is not
provided")`.
`cargo test -p datafusion-ffi` passes (114 tests), and `./dev/rust_lint.sh`
is clean.
## Are there any user-facing changes?
No API changes. `Session::create_physical_plan` over FFI now works for plans
that require the session's `LogicalExtensionCodec` instead of returning a
not-implemented error. No breaking changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
