You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #22418 (the equivalent PhysicalExpr effort). This issue tracks porting the built-in ExecutionPlan implementations — and the DataSource/FileSource/DataSink families — to per-type try_to_proto / try_from_proto hooks, eliminating the central downcast_ref chain in datafusion/proto/src/physical_plan/mod.rs.
Why
Today every built-in ExecutionPlan is serialized through a large downcast_ref chain on the encode side and a symmetric match on PhysicalPlanType on the decode side. This is the same problem #21835 described for PhysicalExpr: it forces internal state to be exposed as pub for the encoder to reach, makes it easy to forget serialization when adding a plan, and splits built-in vs. third-party plans across different code paths.
Mirroring #21929 / #22418, each plan can move onto a try_to_proto hook (a trait method, default Ok(None)) plus a try_from_proto associated fn, one PR at a time, with no wire-format change.
The hook + context
Added by the base PR (see below), in datafusion-physical-plan:
<PlanType>::try_from_proto(node, ctx: &ExecutionPlanDecodeCtx) -> Result<Arc<dyn ExecutionPlan>> — inherent associated fn, wired into the matching PhysicalPlanType::… decode arm.
ExecutionPlanEncodeCtx / ExecutionPlanDecodeCtx expose everything a plan needs without naming datafusion-proto: encode_child/decode_child(+ _children), encode_expr/decode_expr (against any schema), and — for function-carrying plans — typed bytes-onlyencode_udaf/decode_udaf (+ udf/udwf). The dependency inversion (internal dispatch traits defined in physical-plan, implemented in datafusion-proto) is identical to the PhysicalExpr ctx.
The encode dispatch resolves downcast_delegate() before calling the hook, so wrapper plans serialize as their delegate.
Note on function-carrying plans (AggregateExec, window execs, ScalarSubqueryExec)
Unlike the PhysicalExpr side — where the ctx lives belowdatafusion-expr and so ScalarFunctionExpr must stay special-cased — datafusion-physical-plan sits abovedatafusion-expr, so the ctx can carry typed function serde directly. This means aggregate/window/scalar-subquery plans ride the hook via ctx.encode_udaf / decode_udaf etc.; no PhysicalExtensionCodec is exposed and no proto type crosses the boundary. (This is why the ScalarUdfCodec proposal in #23421 is not needed for plans.)
Pattern (per plan)
try_to_proto inside impl ExecutionPlan for FooExec, #[cfg(feature = "proto")]. ⚠️ It must be the trait-method override — an inherent method with the same name is silently never called through &dyn ExecutionPlan.
FooExec::try_from_proto in a separate #[cfg(feature = "proto")] impl FooExec block, wired into the PhysicalPlanType::Foo(_) => FooExec::try_from_proto(self.node(), &decode_ctx) decode arm.
In the same PR, delete the plan's old encode downcast_ref arm and repoint its decode arm — this is the only proof the hook is actually reached (roundtrip tests pass through the new path).
Keep the wire format byte-for-byte identical. Enum conversions (JoinType, NullEquality, PartitionMode, JoinSide, AggregateMode, …) inline as by-name exhaustive matches — the proto and datafusion_common enums are numbered differently, so a numeric cast silently corrupts them.
The reference implementation is ProjectionExec in the base PR. HashJoinExec is the reference for joins (two children, on-columns, JoinFilter, projection sentinel, enum conversions); AggregateExec for the function-codec path.
GenerateSeries / LazyMemoryExec — the generate-series generator types live in datafusion-functions-table, which sits abovedatafusion-physical-plan; an ExecutionPlan-keyed hook cannot name them without a crate cycle. Would require a generator-level hook (separate design).
Extension — the terminal codec.try_encode fallback for arbitrary third-party plans. This is the intended endpoint and stays.
API change
Removing the per-plan PhysicalPlanNodeExt serialization methods and the TryFromProto<&protobuf::{Json,Csv,Parquet}Sink> impls is a deliberate breaking change (these were pub-for-proto scaffolding added in #21929 and released in 54.0.0). The full end-state PR removes them; each incremental PR removes its own arms. This will be summarized in the Upgrade Guide. The load-bearing public API — AsExecutionPlan, PhysicalExtensionCodec, PhysicalProtoConverterExtension — is unchanged.
Follow-up to #22418 (the equivalent
PhysicalExpreffort). This issue tracks porting the built-inExecutionPlanimplementations — and theDataSource/FileSource/DataSinkfamilies — to per-typetry_to_proto/try_from_protohooks, eliminating the centraldowncast_refchain indatafusion/proto/src/physical_plan/mod.rs.Why
Today every built-in
ExecutionPlanis serialized through a largedowncast_refchain on the encode side and a symmetricmatchonPhysicalPlanTypeon the decode side. This is the same problem #21835 described forPhysicalExpr: it forces internal state to be exposed aspubfor the encoder to reach, makes it easy to forget serialization when adding a plan, and splits built-in vs. third-party plans across different code paths.Mirroring #21929 / #22418, each plan can move onto a
try_to_protohook (a trait method, defaultOk(None)) plus atry_from_protoassociated fn, one PR at a time, with no wire-format change.The hook + context
Added by the base PR (see below), in
datafusion-physical-plan:ExecutionPlan::try_to_proto(&self, ctx: &ExecutionPlanEncodeCtx) -> Result<Option<PhysicalPlanNode>>— trait method, defaultOk(None), feature-gated#[cfg(feature = "proto")].<PlanType>::try_from_proto(node, ctx: &ExecutionPlanDecodeCtx) -> Result<Arc<dyn ExecutionPlan>>— inherent associated fn, wired into the matchingPhysicalPlanType::…decode arm.ExecutionPlanEncodeCtx/ExecutionPlanDecodeCtxexpose everything a plan needs without namingdatafusion-proto:encode_child/decode_child(+_children),encode_expr/decode_expr(against any schema), and — for function-carrying plans — typed bytes-onlyencode_udaf/decode_udaf(+ udf/udwf). The dependency inversion (internal dispatch traits defined in physical-plan, implemented in datafusion-proto) is identical to thePhysicalExprctx.The encode dispatch resolves
downcast_delegate()before calling the hook, so wrapper plans serialize as their delegate.Note on function-carrying plans (
AggregateExec, window execs,ScalarSubqueryExec)Unlike the
PhysicalExprside — where the ctx lives belowdatafusion-exprand soScalarFunctionExprmust stay special-cased —datafusion-physical-plansits abovedatafusion-expr, so the ctx can carry typed function serde directly. This means aggregate/window/scalar-subquery plans ride the hook viactx.encode_udaf/decode_udafetc.; noPhysicalExtensionCodecis exposed and no proto type crosses the boundary. (This is why theScalarUdfCodecproposal in #23421 is not needed for plans.)Pattern (per plan)
try_to_protoinsideimpl ExecutionPlan for FooExec,#[cfg(feature = "proto")].&dyn ExecutionPlan.FooExec::try_from_protoin a separate#[cfg(feature = "proto")] impl FooExecblock, wired into thePhysicalPlanType::Foo(_) => FooExec::try_from_proto(self.node(), &decode_ctx)decode arm.downcast_refarm and repoint its decode arm — this is the only proof the hook is actually reached (roundtrip tests pass through the new path).JoinType,NullEquality,PartitionMode,JoinSide,AggregateMode, …) inline as by-name exhaustive matches — the proto anddatafusion_commonenums are numbered differently, so a numeric cast silently corrupts them.The reference implementation is
ProjectionExecin the base PR.HashJoinExecis the reference for joins (two children, on-columns,JoinFilter, projection sentinel, enum conversions);AggregateExecfor the function-codec path.Reference PRs
try_to_proto/try_from_protohooks + ctx +ProjectionExec. Must land first.DataSource/DataSinkfamilies.Scope / checklist
Each item is an independent follow-up PR. The two foundation items unblock their families.
Foundations
try_to_protohook +FileScanConfigserde porttry_to_protohook +FileSinkConfigserde portdatafusion-physical-planplansDataSource / DataSink families (blocked by the foundations)
Intentionally left as typed dispatch
GenerateSeries/LazyMemoryExec— the generate-series generator types live indatafusion-functions-table, which sits abovedatafusion-physical-plan; anExecutionPlan-keyed hook cannot name them without a crate cycle. Would require a generator-level hook (separate design).Extension— the terminalcodec.try_encodefallback for arbitrary third-party plans. This is the intended endpoint and stays.API change
Removing the per-plan
PhysicalPlanNodeExtserialization methods and theTryFromProto<&protobuf::{Json,Csv,Parquet}Sink>impls is a deliberate breaking change (these were pub-for-proto scaffolding added in #21929 and released in 54.0.0). The full end-state PR removes them; each incremental PR removes its own arms. This will be summarized in the Upgrade Guide. The load-bearing public API —AsExecutionPlan,PhysicalExtensionCodec,PhysicalProtoConverterExtension— is unchanged.