timsaucer opened a new issue, #24106: URL: https://github.com/apache/datafusion/issues/24106
### Is your feature request related to a problem or challenge? This is a follow on to https://github.com/apache/datafusion/pull/24028 `datafusion-ffi` passes logical extension codecs, physical extension codecs, and task context providers across library boundaries. These values describe one serialization environment and must remain consistent, but the current API carries them separately. `FFI_LogicalExtensionCodec` and `FFI_PhysicalExtensionCodec` each contain an `FFI_TaskContextProvider`. Several FFI wrappers carry only an `FFI_LogicalExtensionCodec` because they historically needed to serialize only logical expressions and plans. Now that `FFI_SessionRef` exposes `Session::query_planner`, those wrappers can also initiate physical-plan serialization through a session callback. `FFI_SessionRef::new` has no physical codec available, so it creates a `DefaultPhysicalExtensionCodec` and gives it a clone of the logical codec's task context provider. This has two limitations: 1. A query planner obtained through that session reference cannot round-trip custom physical extension nodes. The failure occurs later during physical-plan encoding or decoding as `PhysicalExtensionCodec is not provided`. 2. The cloned task context provider is not guaranteed to represent the session passed to `FFI_SessionRef::new`. It may have arrived with a codec exported by another library, and its weak reference may expire. Callers can avoid these limitations with `FFI_SessionRef::new_with_ffi_codecs`, but wrappers such as `FFI_TableProvider`, `FFI_TableProviderFactory`, and `FFI_TableFunction` do not currently carry a matching physical codec to supply. Passing codecs independently also makes it easy to combine codecs and a provider that were not configured together. ### Describe the solution you'd like Add an ABI-stable bundle that represents the complete serialization environment used at an FFI boundary. The exact representation should be determined during implementation, but its public abstraction should contain: - one `FFI_TaskContextProvider`; - one `FFI_LogicalExtensionCodec`; - one `FFI_PhysicalExtensionCodec`. For example: ```rust #[repr(C)] pub struct FFI_ExtensionCodecBundle { task_ctx_provider: FFI_TaskContextProvider, logical_codec: FFI_LogicalExtensionCodec, physical_codec: FFI_PhysicalExtensionCodec, } ``` The fields should remain private so constructors can enforce that both codecs use the intended provider. Constructors should support: - creating a bundle from one task context provider and explicit native logical and physical codecs; - explicitly creating a bundle with both default codecs; - safely cloning or re-exporting an existing FFI bundle without nesting foreign wrappers; - preserving the live task context behavior needed when functions or other session state change after construction. Individual codec wrappers currently need direct access to a task context provider because their function pointers receive only the codec. The initial implementation may retain provider clones inside each codec while the bundle acts as the canonical pairing mechanism. It should not introduce a codec-to-bundle reference or a self-referential structure. The dependency direction should remain bundle to codecs to task context provider. Propagate the bundle through FFI wrappers that create nested providers or export a session, including the relevant catalog, schema, table provider, table provider factory, and table function paths. Change `FFI_SessionRef` and `FFI_QueryPlanner` construction to consume the bundle, or otherwise require codecs obtained from the same bundle. Default physical codec use should always be explicit. Add cross-library integration coverage for this scenario: 1. Library A owns a session, task context provider, and custom physical codec. 2. Library A installs a query planner owned by library C. 3. Library A calls a table provider owned by library B. 4. Library B accesses the planner through the received session. 5. The planner returns a custom physical extension node. 6. The node is serialized through the configured physical codec and reconstructed with the expected local type identity. The tests should also verify that nested catalog/schema/table-provider construction preserves the bundle and that an expired provider produces a clear error. ### Describe alternatives you've considered - **Document the current limitation.** This prevents accidental misuse but does not allow custom physical nodes through session callbacks made by wrappers that carry only a logical codec. - **Add a physical codec field independently to each affected wrapper.** This is smaller locally but repeats propagation logic and still permits mismatched logical codecs, physical codecs, and task context providers. - **Derive the provider or codec from `Session`.** `Session` returns an `Arc<TaskContext>`, but it does not expose the reference-counted `TaskContextProvider` required for live FFI callbacks or a physical extension codec registry. - **Preserve codecs only when re-exporting an existing `ForeignQueryPlanner`.** This fixes one planner-swap topology but does not support local planners that produce custom physical nodes. - **Continue synthesizing a default physical codec.** This preserves current behavior but causes custom-node support to fail after the query planner has already crossed the boundary. ### Additional context The relevant code is in: - `datafusion/ffi/src/session/mod.rs` - `datafusion/ffi/src/query_planner.rs` - `datafusion/ffi/src/proto/logical_extension_codec.rs` - `datafusion/ffi/src/proto/physical_extension_codec.rs` - the catalog, schema, table provider, table provider factory, and table function FFI wrappers -- 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]
