timsaucer opened a new pull request, #24108: URL: https://github.com/apache/datafusion/pull/24108
> [!NOTE] > This PR is stacked on top of #24028 (`feat: Implement FFI_QueryPlanner`) and > contains that PR's commits, because a cross-repository PR cannot use a fork > branch as its base. Only the final commit, `feat(ffi): add an FFI extension > codec bundle`, is new here. Please review #24028 first; this PR will shrink to > a single commit once that one merges. ## Which issue does this PR close? - No separate issue. This is a follow-up fix to #24028, which unblocks https://github.com/apache/datafusion-python/issues/1612. ## Rationale for this change Serializing plans across an FFI boundary needs three values that must agree with one another: a task context provider, a logical extension codec, and a physical extension codec. The wrappers took them separately, and the ones that carried only a logical codec synthesized a `DefaultPhysicalExtensionCodec` whenever they had to export a session. The user-visible consequence: a foreign library that provides a query planner returning a custom physical extension node cannot get that node back across the boundary. A consumer that reaches `Session::query_planner` through an exported session gets a planner whose physical codec is the default one, and the query fails with `PhysicalExtensionCodec is not provided`. The synthesized codec also borrowed the logical codec's task context provider, which is not guaranteed to represent the exported session. ## What changes are included in this PR? Adds `FFI_ExtensionCodecBundle`, which carries the task context provider and both codecs as one unit with private fields, so the constructors are the only way to pair them. The bundle owns no private data of its own; each member already carries its own lifecycle pointers and library marker. The dependency direction is bundle → codecs → task context provider — a bundle stored inside a codec would make cloning recurse forever. The bundle is propagated through every wrapper that exports a session or builds a nested provider: `FFI_TableProvider`, `FFI_TableProviderFactory`, `FFI_TableFunction`, `FFI_CatalogProvider`, `FFI_CatalogProviderList`, `FFI_SchemaProvider`, `FFI_SessionRef`, and `FFI_QueryPlanner`. Each keeps one constructor taking the bundle; the `new_with_ffi_codec(s)` variants and the `Option<Arc<dyn LogicalExtensionCodec>>` argument whose `None` meant "use the default" are gone, so choosing the defaults is now explicit (`FFI_ExtensionCodecBundle::new_default`). The two paths inside `FFI_LogicalExtensionCodec` that rebuild a table provider receive only the codec, so they pair it with an explicit default physical codec and document what that costs a consumer. `Session::create_physical_plan` serialized its plan with no extension codec on either side of the boundary; it now uses the bundle's logical codec, matching `optimize` and `create_physical_expr`. ## Are these changes tested? Yes. - A three-library integration test where library A owns the session and a custom physical codec, installs library C's planner, and queries library B's provider, which reaches that planner through the session A handed it and returns a custom physical extension node. Restoring the old default-codec behaviour makes it fail with `PhysicalExtensionCodec is not provided`. - Unit coverage that nested catalog list → catalog → schema → table construction preserves the bundle, that cloning does not nest foreign codec wrappers, and that an expired task context provider reports a clear error rather than panicking. ## Are there any user-facing changes? Yes — this is a breaking change to the `datafusion-ffi` public API and to the `FFI_` struct layouts. FFI providers and consumers must both be rebuilt against DataFusion 55. The ABI is already evolving in DF55. Documented in `docs/source/library-user-guide/upgrading/55.0.0.md` with before/after migration examples, and in `datafusion/ffi/README.md`. The `api change` label applies. -- 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]
