refactor(proto): migrate single-child plans by Phoenix500526 · Pull Request #23710 · apache/datafusion

Phoenix500526 · 2026-07-20T05:57:00Z

Which issue does this PR close?

Closes Proto: migrate trivial single-child plans (CoalesceBatches, CoalescePartitions, Cooperative, Buffer) #23500.

Rationale for this change

Physical plan serialization currently relies on centralized type
dispatch in datafusion-proto. This separates serialization from the
plans that own the relevant state and requires every built-in plan to
be handled specially.

Moving serialization into each plan keeps that logic with its owner and
allows the central dispatch chain to be removed incrementally.

CoalesceBatchesExec is deprecated but remains a supported public type.
Migrating its existing serialization keeps the dispatch model
consistent without extending its lifetime or changing its behavior.

What changes are included in this PR?

Add per-plan protobuf encoding and decoding for:
- CoalesceBatchesExec
- CoalescePartitionsExec
- CooperativeExec
- BufferExec
Route decoding through each plan's try_from_proto implementation.
Remove the corresponding centralized encoding branches.
Retain the old public conversion helpers as deprecated compatibility
APIs.
Preserve the existing protobuf wire representation.
Add roundtrip coverage for CooperativeExec and BufferExec.

Are these changes tested?

Yes

Are there any user-facing changes?

There are no runtime behavior or protobuf wire-format changes.

The old PhysicalPlanNodeExt conversion helpers remain available but
are deprecated in favor of the per-plan serialization hooks.

CoalesceBatches serialization still depends on centralized plan downcasts, keeping protobuf ownership outside the execution plan. Move encoding and decoding behind its ExecutionPlan hooks while preserving the existing wire fields. Keep the legacy helpers as deprecated compatibility APIs. Refs apache#23500 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

CoalescePartitions serialization still depends on the central Merge dispatch, keeping protobuf ownership outside the execution plan. Move encoding and decoding behind its ExecutionPlan hooks while preserving the historical Merge wire variant and optional fetch field. Keep the legacy helpers as deprecated compatibility APIs. Refs apache#23500 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

CooperativeExec serialization still depends on centralized plan dispatch despite carrying only one child. Move encoding and decoding behind its ExecutionPlan hooks, retain the legacy helpers as deprecated compatibility APIs, and add a direct round-trip test that proves the hooks are reached. Refs apache#23500 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

BufferExec serialization still depends on centralized plan dispatch, keeping its protobuf capacity representation outside the execution plan. Move encoding and decoding behind its ExecutionPlan hooks, retain the legacy helpers as deprecated compatibility APIs, and verify capacity preservation with a direct round-trip test. CLOSES apache#23500 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

github-actions · 2026-07-20T06:01:27Z

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion-physical-plan v54.0.0 (current)
       Built [  45.493s] (current)
     Parsing datafusion-physical-plan v54.0.0 (current)
      Parsed [   0.146s] (current)
    Building datafusion-physical-plan v54.0.0 (baseline)
       Built [  41.008s] (baseline)
     Parsing datafusion-physical-plan v54.0.0 (baseline)
      Parsed [   0.148s] (baseline)
    Checking datafusion-physical-plan v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.656s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  89.417s] datafusion-physical-plan
    Building datafusion-proto v54.0.0 (current)
       Built [  66.004s] (current)
     Parsing datafusion-proto v54.0.0 (current)
      Parsed [   0.020s] (current)
    Building datafusion-proto v54.0.0 (baseline)
       Built [  65.797s] (baseline)
     Parsing datafusion-proto v54.0.0 (baseline)
      Parsed [   0.020s] (baseline)
    Checking datafusion-proto v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.242s] 223 checks: 222 pass, 1 fail, 0 warn, 30 skip

--- failure trait_method_marked_deprecated: trait method #[deprecated] added ---

Description:
A trait method is now #[deprecated]. Downstream crates will get a compiler warning when using this method.
        ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_method_marked_deprecated.ron

Failed in:
  method try_into_coalesce_batches_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_into_merge_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_into_cooperative_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_into_buffer_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_from_coalesce_batches_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_from_coalesce_partitions_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_from_cooperative_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722
  method try_from_buffer_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:722

     Summary semver requires new minor version: 0 major and 1 minor checks failed
    Finished [ 133.657s] datafusion-proto

codecov-commenter · 2026-07-20T06:21:11Z

Codecov Report

❌ Patch coverage is 58.95954% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.70%. Comparing base (a8d1af6) to head (1c33ac2).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/proto/src/physical_plan/mod.rs	14.86%	63 Missing ⚠️
datafusion/physical-plan/src/buffer.rs	92.00%	0 Missing and 2 partials ⚠️
datafusion/physical-plan/src/coalesce_batches.rs	91.66%	1 Missing and 1 partial ⚠️
...atafusion/physical-plan/src/coalesce_partitions.rs	91.30%	1 Missing and 1 partial ⚠️
datafusion/physical-plan/src/coop.rs	92.59%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #23710    +/-   ##
========================================
  Coverage   80.69%   80.70%            
========================================
  Files        1088     1089     +1     
  Lines      367741   368362   +621     
  Branches   367741   368362   +621     
========================================
+ Hits       296761   297280   +519     
- Misses      53272    53364    +92     
- Partials    17708    17718    +10

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

adriangb · 2026-07-20T23:47:08Z

+    #[deprecated(
+        since = "55.0.0",
+        note = "unused by DataFusion; `CoalesceBatchesExec` deserializes itself via `CoalesceBatchesExec::try_from_proto`"
+    )]


Can we have the deprecated methods delegate to the originals?

adriangb · 2026-07-20T23:47:23Z

-                ),
-            PhysicalPlanType::Merge(merge) => {
-                self.try_into_merge_physical_plan(merge, ctx, proto_converter)
+            #[expect(deprecated)]


Why expect deprecated here? What deprecated method are we calling?

try_from_proto itself is not deprecated. CoalesceBatchesExec is the deprecated item, and referring to it in CoalesceBatchesExec::try_from_proto triggers the deprecation lint. The attribute is scoped to this match arm to allow decoding the legacy protobuf variant.

FYI：#19622

The migrated PhysicalPlanNodeExt helpers retained independent encode and decode implementations after per-plan protobuf hooks became canonical. This duplicated wire-format logic and allowed the deprecated compatibility paths to drift from normal dispatch. Make the CoalesceBatches, CoalescePartitions, Cooperative, and Buffer helpers delegate through the execution-plan encode and decode contexts. Add coverage that compares legacy helper output with the canonical hooks for both serialization and deserialization. Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

adriangb

Thanks!

Phoenix500526 added 4 commits July 20, 2026 13:02

github-actions Bot added proto Related to proto crate physical-plan Changes to the physical-plan crate labels Jul 20, 2026

github-actions Bot added the auto detected api change Auto detected API change label Jul 20, 2026

adriangb reviewed Jul 20, 2026

View reviewed changes

Phoenix500526 force-pushed the issue/23500 branch from e0cbf55 to 1c33ac2 Compare July 21, 2026 05:20

adriangb approved these changes Jul 21, 2026

View reviewed changes

adriangb enabled auto-merge July 21, 2026 05:33

adriangb added this pull request to the merge queue Jul 21, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 21, 2026

adriangb added this pull request to the merge queue Jul 21, 2026

Merged via the queue into apache:main with commit f3f381c Jul 21, 2026
40 checks passed

Phoenix500526 deleted the issue/23500 branch July 21, 2026 06:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(proto): migrate single-child plans#23710

refactor(proto): migrate single-child plans#23710
adriangb merged 5 commits into
apache:mainfrom
Phoenix500526:issue/23500

Phoenix500526 commented Jul 20, 2026

Uh oh!

github-actions Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Jul 20, 2026 •

edited

Loading

Uh oh!

adriangb Jul 20, 2026

Uh oh!

Phoenix500526 Jul 21, 2026

Uh oh!

adriangb Jul 20, 2026

Uh oh!

Phoenix500526 Jul 21, 2026

Uh oh!

adriangb left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Phoenix500526 commented Jul 20, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

adriangb Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Phoenix500526 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Phoenix500526 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 20, 2026 •

edited

Loading

codecov-commenter commented Jul 20, 2026 •

edited

Loading