milenkovicm opened a new pull request, #22151: URL: https://github.com/apache/datafusion/pull/22151
## Which issue does this PR close? This is follow up on #20249 Closes #. (pending) ## Rationale for this change DataFusion's FFI layer allows plugins and foreign libraries to extend query processing across shared-library boundaries. However, there was no way for a foreign library to participate in the physical planning pipeline — `QueryPlanner` required a concrete &`SessionState`, which cannot cross an FFI boundary. ## What changes are included in this PR? - Adds `FFI_QueryPlanner` — an FFI-safe `repr-C` struct that wraps a foreign `QueryPlanner` implementation, serializing the `LogicalPlan` via datafusion-proto across the boundary and deserializing the resulting `ExecutionPlan`. - Adds `QueryPlannerWeak` trait — a variant of `QueryPlanner` that accepts `&dyn Session` instead of `&SessionState`, making it implementable by foreign code. It make more sense to create new interface rather than changing `QueryPlanner`. The change would introduce big backward incompatibility, also we can argue that having `SessionState` as parameter makes sense. - Extends `QueryPlanner` with an `Any` bound to allow downcasting (needed by `DynamicForeignQueryPlanner`). - Adds integration tests covering both direct use of `FFI_QueryPlanner` via `QueryPlannerWeak` and the deferred-injection pattern via `DynamicForeignQueryPlanner`. The `DynamicForeignQueryPlanner` helper (in the integration tests) shows how to break the construction-time dependency cycle: `SessionContext` needs a `QueryPlanner` at build time, but the `FFI` planner needs the codec which needs the context. The placeholder is registered at build time and swapped for the real FFI planner once both sides exist. ## Are these changes tested? Yes. Integration tests are added in `datafusion/ffi/tests/ffi_planner.rs`: - `test_ffi_query_planner` — verifies that a foreign planner loaded from a shared library can produce a valid ExecutionPlan from a LogicalPlan. - `test_ffi_dynamic_query_planner` — verifies the deferred-injection pattern where the FFI planner is installed into an already-constructed SessionContext via `DynamicForeignQueryPlanner`. ## Are there any user-facing changes? Yes, one change to a public trait: - `QueryPlanner` now requires Any as a supertrait (enables downcasting). This is not required for this implementation but it might be needed for implementors ## Open Questions - there is chicken-egg problem at the moment, `SessionContext` is needed to create a `QueryPlanner` but `QueryPlanner` is required to create a `SessionContext` (I hope I did not get that horrible wrong) - Name of `QueryPlannerWeak` is debatable, I cant come up with better name -- 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]
