adriangb opened a new pull request, #23495:
URL: https://github.com/apache/datafusion/pull/23495

   ## Which issue does this PR close?
   
   - Part of #23494.
   
   ## Rationale for this change
   
   This is the foundation for porting `ExecutionPlan` serialization off the 
central `downcast_ref` chain in `datafusion/proto/src/physical_plan/mod.rs`, 
mirroring what #21929 did for `PhysicalExpr` (tracked in #22418). Everything 
else in #23494 depends on this PR; it can't be parallelized because it defines 
the ctx API and wiring that every per-plan follow-up uses.
   
   ## What changes are included in this PR?
   
   In `datafusion-physical-plan` (feature-gated `#[cfg(feature = "proto")]`):
   
   - `ExecutionPlan::try_to_proto(&self, ctx: &ExecutionPlanEncodeCtx) -> 
Result<Option<PhysicalPlanNode>>` — a new trait method defaulting to `Ok(None)` 
(so un-migrated plans are unchanged).
   - `ExecutionPlanEncodeCtx` / `ExecutionPlanDecodeCtx` plus the internal 
`ExecutionPlanEncode` / `ExecutionPlanDecode` dispatch traits (defined here, 
implemented in `datafusion-proto`) — the same dependency-inversion the 
`PhysicalExpr` ctx uses. The ctx exposes `encode_child`/`decode_child` (+ 
`_children`), `encode_expr`/`decode_expr`, and typed **bytes-only** 
`encode_udaf`/`decode_udaf` (+ udf/udwf) for function-carrying plans. No proto 
type is ever named by physical-plan beyond the pure prost types in 
`datafusion-proto-models`.
   - An `expect_plan_variant!` macro mirroring `expect_expr_variant!`.
   
   In `datafusion-proto`:
   
   - `ConverterPlanEncoder` / `ConverterPlanDecoder` implement the dispatch 
traits over the existing `PhysicalExtensionCodec` + converter (the function 
serde reuses today's `fun_definition` byte semantics exactly). The encode 
dispatch calls the hook first (resolving `downcast_delegate()` beforehand so 
wrapper plans serialize as their delegate), then falls back to the existing 
downcast chain.
   
   `ProjectionExec` is migrated as the reference implementation: its old encode 
arm is deleted and its decode arm reduced to `ProjectionExec::try_from_proto`, 
so a green roundtrip proves the hook is the only path.
   
   ## Are these changes tested?
   
   Yes — the existing `proto_integration` roundtrip suite (including TPC-H) 
passes unchanged (189 tests), which exercises the `ProjectionExec` migration 
end-to-end. `cargo clippy --all-targets -- -D warnings` and `cargo fmt` are 
clean.
   
   ## Are there any user-facing changes?
   
   Additive only in this PR (new trait method with a default, new pub ctx 
types). No wire-format change. The broader effort (#23494) will remove the 
pub-for-proto `PhysicalPlanNodeExt` scaffolding as plans migrate — that is a 
documented `api change`, but this base PR does not remove any public API.
   


-- 
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]

Reply via email to