goutamadwant opened a new pull request, #24492: URL: https://github.com/apache/datafusion/pull/24492
## Which issue does this PR close? - Closes #24065. ## Rationale for this change `ForeignSession::create_physical_plan` forwarded through the owning session. If that session had installed the calling library's query planner, the callback re-entered the same planner and could recurse until the stack was exhausted. The callback also returned an `FFI_ExecutionPlan`, which cannot reconstruct built-in nodes with the receiving library's Rust type identities for downcasting. The supported delegation boundary is `FFI_QueryPlanner`: the session owner exports its original planner before installing a foreign planner, and the foreign planner retains that handle. This boundary serializes physical plans and reconstructs them with local type identities. ## What changes are included in this PR? - Remove the unsafe `create_physical_plan` callback from `FFI_SessionRef`. - Make `ForeignSession::create_physical_plan` return an actionable `NotImplemented` error without invoking the installed planner. - Document the captured-`FFI_QueryPlanner` migration in the module documentation and DataFusion 55 upgrade guide. - Add a regression test that verifies the installed planner is not invoked through the unsupported session method. Draft PR #24108 contains broader extension-codec bundle work and currently updates the removed callback's logical-codec path. That callback-specific hunk will no longer be needed after this change; the remaining codec-bundle work is outside this PR. ## Are these changes tested? Yes. - The focused regression test passes and verifies zero planner invocations. - `cargo test -p datafusion-ffi --features integration-tests` - `cargo clippy -p datafusion-ffi --all-targets --all-features -- -D warnings` - `cargo clippy --all-targets --all-features -- -D warnings` - `RUST_BACKTRACE=1 cargo test --profile ci --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli --workspace --lib --tests --bins --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption` - `RUSTDOCFLAGS="-D warnings" cargo doc -p datafusion-ffi --all-features --no-deps` - `cargo fmt --all -- --check` - `./ci/scripts/doc_prettier_check.sh` The cross-library query-planner tests continue to verify that the captured planner route restores local, downcastable physical-plan nodes. ## Are there any user-facing changes? Yes. `ForeignSession::create_physical_plan` now returns `NotImplemented`; callers must retain and invoke the session owner's exported `FFI_QueryPlanner`. Removing the callback changes the `FFI_SessionRef` ABI layout, so FFI producers and consumers must be rebuilt against DataFusion 55. This PR targets `main`, must not be backported to a patch-release branch, and requires the `api change` label. -- 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]
