timsaucer opened a new issue, #24762: URL: https://github.com/apache/datafusion/issues/24762
### Is your feature request related to a problem or challenge? This is a follow on to https://github.com/apache/datafusion/issues/24106. `FFI_QueryPlanner` stores `logical_codec` and `physical_codec` by value, and `create_physical_plan_with_session_runtime` uses `self.logical_codec` / `self.physical_codec` both to build the `FFI_SessionRef` it hands out and to decode the physical plan that comes back. The codecs a planner serializes with are therefore fixed at the moment it is exported. #24723 addressed one consequence of that: `new_with_ffi_codecs` now unwraps an existing `ForeignQueryPlanner` and replaces its codecs, so a host can rebind an installed planner when its codecs change. That rebind reaches exactly one layer, which is not enough once planners are layered. Concrete case, from wiring this up in datafusion-python (apache/datafusion-python#1677): 1. The host installs planner `P`, exported by library C. While exporting, `P` resolves a fallback planner `Q` from library D by calling `Q`'s own export hook, handing it the session's codecs as they are at that moment. `P` keeps the resulting `Arc<dyn QueryPlanner>` — a `ForeignQueryPlanner(Q_ffi)` — in its private data. 2. The host later installs a new extension codec, and rebuilds the installed planner through `new_with_ffi_codecs`. That unwraps `ForeignQueryPlanner(P_ffi)` and swaps `P`'s codecs. 3. `Q`'s codecs are untouched. When `P` delegates, the hop into library D serializes with whatever codecs `Q` was imported with. Neither side can repair it: - **The host cannot reach `Q`.** It sits behind `P`'s `create_physical_plan` function pointer with no handle exposed. `new_with_ffi_codecs` unwraps one layer by construction. - **Library C cannot re-derive codecs at plan time.** `create_physical_plan` receives `&dyn Session`, and `Session` exposes `task_ctx()` but no logical or physical codec accessor, so it cannot pick up the host's current codecs. This is the same observation as the "Derive the provider or codec from `Session`" alternative already listed in #24106. The stale codecs stay usable rather than dangling — each carries its own `FFI_TaskContextProvider` — so the symptom is a hop quietly serializing with an older codec rather than an error, which makes it easy to miss. Worth noting this only arises when the fallback lives in a **different** library. `impl From<&FFI_QueryPlanner> for Arc<dyn QueryPlanner + Send + Sync>` short-circuits on a matching `library_marker_id` and returns `Arc::clone(planner.inner())`, so a same-library fallback holds no codecs at all and serializes nothing — the behavior `test_ffi_query_planner_local_bypass` covers. A three-library test in datafusion-python confirmed a same-library fallback produces zero additional codec traffic, which is exactly why the problem does not reproduce there. ### Describe the solution you'd like Some way for a planner to resolve the host's *current* codecs at plan time rather than at export time. Rough options: 1. Expose the serialization environment on `Session`, or on `FFI_SessionRef`, so `create_physical_plan` can take codecs from the session it is handed and pass them down to anything it delegates to. Overlaps with #23678 and with the bundle proposed in #24106. 2. Make the rebind recursive: give `FFI_QueryPlanner` a way to hand new codecs to a planner it wraps, so `new_with_ffi_codecs` propagates rather than replacing a single layer. This needs a hook on the producing side, since the wrapped planner is opaque to the consumer. 3. Document the limitation and state the ordering rule — install codecs before a layered planner. The bundle in #24106 does not fix this on its own: a bundle held by value is still a snapshot taken at export time. It would make the thing that goes stale one object instead of two, which probably makes (1) or (2) easier to implement. ### Describe alternatives you've considered - **Have the host re-install the outer planner after installing a codec.** That re-runs the planner's export hook, which re-imports the fallback against the current codecs. It works, but it requires the fallback to be a live object rather than a previously captured capsule, requires the host to have retained the planner, and the requirement is invisible to anyone who has not hit the problem. - **Accept and document it.** This is what datafusion-python does today. ### Additional context Relevant code is all in `datafusion/ffi/src/query_planner.rs`: `FFI_QueryPlanner`, `new_with_ffi_codecs`, `create_physical_plan_with_session_runtime`, and `ForeignQueryPlanner`. Related: #24722 / #24723 (one-layer rebind), #24106 (FFI extension codec bundle), #23678 (expand `Session` to parity with `SessionState`). Downstream: apache/datafusion-python#1677, which documents the constraint under "Rebinding a planner's codecs is one level deep" in `docs/source/contributor-guide/ffi.md`. -- 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]
