adriangb opened a new pull request, #24269:
URL: https://github.com/apache/datafusion/pull/24269
## Which issue does this PR close?
- Part of #23494. This closes out that EPIC's "Remove the
`#[deprecated(since = "55.0.0")]` `PhysicalPlanNodeExt` scaffolding" cleanup
item.
## Rationale for this change
`PhysicalPlanNodeExt` currently carries 59 `#[deprecated(since = "55.0.0")]`
methods — per-operator `try_into_*_physical_plan` / `try_from_*_exec` shims
left behind by the `ExecutionPlan::try_to_proto` / `try_from_proto` migration.
Each is a thin forwarder to the operator's own hook; none is called by
DataFusion.
**These methods have never shipped in any DataFusion release**, so no
deprecation window is owed:
- At the `54.1.0` tag they were **private inherent** methods on `impl
protobuf::PhysicalPlanNode` — not `pub`, not on a public trait:
```console
$ git show 54.1.0:datafusion/proto/src/physical_plan/mod.rs | sed -n
'700,701p'
impl protobuf::PhysicalPlanNode {
fn try_into_explain_physical_plan(
```
- #21929 (merge commit `077f08a9a6632324c95275dd15b5dd5b1f14006f`, merged
2026-05-22) is what promoted them into the public `PhysicalPlanNodeExt` trait,
and it is **not** an ancestor of `54.1.0` (54.1.0 was cut off the 54.0.x line):
```console
$ git merge-base --is-ancestor 077f08a9a6632324c95275dd15b5dd5b1f14006f
54.1.0; echo $?
1
```
- Every one of them is marked `#[deprecated(since = "55.0.0")]`, so 55.0.0
would be the very first release to expose them — already deprecated.
The [API health
policy](https://github.com/apache/datafusion/blob/main/docs/source/contributor-guide/api-health.md)
exists to protect API that users could have depended on *from a release*.
Nothing released ever exposed these. Deleting them before the 55.0.0 branch is
cut avoids shipping 59 dead-on-arrival public methods that we would then be
obliged to carry for a full deprecation cycle.
Removing them also deletes ~1350 lines from
`datafusion/proto/src/physical_plan/mod.rs`, which makes the remaining,
load-bearing surface of the trait much easier to read.
## What changes are included in this PR?
- Delete the 59 `#[deprecated(since = "55.0.0")]` methods from
`PhysicalPlanNodeExt` (29 `try_from_*_exec` encoders, 30 `try_into_*` decoders).
- Drop three `use` statements that became unused as a result:
`DataSinkExec`, `BoundedWindowAggExec`, `SortMergeJoinExecNode`.
- Delete `deprecated_projection_shim_decodes_argument_not_self`, the one
test that existed solely to pin the behaviour of the
`try_into_projection_physical_plan` shim.
Deliberately **not** changed:
- The trait itself and its 16 non-deprecated methods stay: `node()`,
`try_into_physical_plan_with_converter`, `try_into_physical_plan_with_context`,
`try_from_physical_plan_with_converter`, the scan/extension/generate-series
decoders and `try_from_data_source_exec` / `try_from_lazy_memory_exec` that the
central dispatch still calls.
- `AsExecutionPlan`, `PhysicalExtensionCodec`,
`PhysicalProtoConverterExtension` are untouched.
- **No `.proto` files and no encode/decode dispatch behaviour are touched —
the wire format is unchanged.**
- The `TryFromProto<&protobuf::{Json,Csv,Parquet}Sink>` impls in
`from_proto.rs` mentioned in #23494 are **not** removed here: they are not
marked `#[deprecated]` (there are no `deprecated` attributes anywhere in
`from_proto.rs` / `to_proto.rs`), so they don't fall under the "never-released
deprecated scaffolding" argument above and deserve their own decision.
## Are these changes tested?
Covered by the existing test suite; the change is a pure deletion of
unreferenced code.
The one deleted test only exercised the deprecated shim. The underlying
`ProjectionExec::try_to_proto` / `ProjectionExec::try_from_proto` hook keeps
full roundtrip coverage through the existing `roundtrip_test` cases in
`datafusion/proto/tests/cases/roundtrip_physical_plan.rs` (e.g.
`roundtrip_like`, `roundtrip_projection_source`, `roundtrip_empty_projection`).
Verified locally:
- `cargo fmt --all`
- `./ci/scripts/rust_clippy.sh` (CI's exact workspace + `--all-targets`
clippy, clean)
- `RUST_BACKTRACE=1 cargo test --profile ci -p datafusion-proto --features
avro,json` — 238 tests pass, 0 failures
- `cargo check -p datafusion-examples --examples` (two examples import
`PhysicalPlanNodeExt`; both only use methods that stay)
- `cargo doc -p datafusion-proto --no-deps` (no dangling intra-doc links)
## Are there any user-facing changes?
Removal of 59 public trait methods, all of which were already
`#[deprecated]` and none of which ever appeared in a published release. No
Upgrade Guide entry is needed, because there is no released version anyone
could be upgrading *from* that had this 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]
