refactor(proto): migrate FilterExec serde by Phoenix500526 · Pull Request #23708 · apache/datafusion

Phoenix500526 · 2026-07-20T04:18:41Z

Which issue does this PR close?

Closes Proto: migrate FilterExec to the try_to_proto hook #23499.

Rationale for this change

The centralized protobuf dispatcher couples each execution plan to datafusion-proto. Adding plan serialization therefore requires changes outside the plan implementation.

Move FilterExec encoding and decoding behind its ExecutionPlan hooks. Preserve the existing wire format and retain the old helper methods as deprecated compatibility APIs.

What changes are included in this PR?

Are these changes tested?

Yes

Are there any user-facing changes?

github-actions · 2026-07-20T04:23:20Z

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 [  38.523s] (current)
     Parsing datafusion-physical-plan v54.0.0 (current)
      Parsed [   0.143s] (current)
    Building datafusion-physical-plan v54.0.0 (baseline)
       Built [  37.979s] (baseline)
     Parsing datafusion-physical-plan v54.0.0 (baseline)
      Parsed [   0.152s] (baseline)
    Checking datafusion-physical-plan v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.662s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  79.312s] datafusion-physical-plan
    Building datafusion-proto v54.0.0 (current)
       Built [  62.061s] (current)
     Parsing datafusion-proto v54.0.0 (current)
      Parsed [   0.019s] (current)
    Building datafusion-proto v54.0.0 (baseline)
       Built [  60.878s] (baseline)
     Parsing datafusion-proto v54.0.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-proto v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.263s] 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_filter_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:719
  method try_from_filter_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:719

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

codecov-commenter · 2026-07-20T04:43:56Z

Codecov Report

❌ Patch coverage is 67.14286% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.70%. Comparing base (774f0bc) to head (7172823).

Files with missing lines	Patch %	Lines
datafusion/proto/src/physical_plan/mod.rs	7.14%	13 Missing ⚠️
datafusion/physical-plan/src/filter.rs	82.14%	4 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #23708      +/-   ##
==========================================
- Coverage   80.71%   80.70%   -0.01%     
==========================================
  Files        1089     1089              
  Lines      368280   368293      +13     
  Branches   368280   368293      +13     
==========================================
- Hits       297250   297237      -13     
- Misses      53314    53341      +27     
+ Partials    17716    17715       -1

☔ 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:55Z

+    #[deprecated(
+        since = "55.0.0",
+        note = "unused by DataFusion; `FilterExec` serializes itself via `ExecutionPlan::try_to_proto`"
+    )]


Can we have the deprecated method delegate to the new one to avoid code duplication and drift?

The centralized protobuf dispatcher couples each execution plan to datafusion-proto. Adding plan serialization therefore requires changes outside the plan implementation. Move FilterExec encoding and decoding behind its ExecutionPlan hooks. Preserve the existing wire format and retain the old helper methods as deprecated compatibility APIs. CLOSES apache#23499 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

FilterExec serde moved to per-plan hooks, but the deprecated PhysicalPlanNodeExt helpers retained separate implementations. Keeping the wire-format logic in both paths allows the compatibility API to drift from normal dispatch. Build the existing encode and decode adapter contexts in the deprecated helpers and forward to FilterExec's canonical hooks, preserving the old API without duplicating serialization logic. Refs apache#23499 Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

adriangb

Thanks!

…w hooks (apache#23731) ## Which issue does this PR close? - Part of apache#23494. ## Rationale for this change apache#23495 landed the deprecated `ProjectionExec` compatibility shims (`try_from_projection_exec` / `try_into_projection_physical_plan`) with their original bodies fully duplicated in `datafusion-proto`, even though the same wire-format logic now lives in `ProjectionExec::try_to_proto` / `try_from_proto` (the apache#22419 hook pattern). This PR makes those deprecated shims **delegate** to the new hooks so the wire format is single-sourced in `datafusion-physical-plan` rather than duplicated. This matches the `FilterExec` delegate style in apache#23708. No wire-format change. ## What changes are included in this PR? - Replace the body of `try_from_projection_exec` with a thin shim that builds an `ExecutionPlanEncodeCtx` from the `codec` + `proto_converter` (via `ConverterPlanEncoder`) and delegates to `ProjectionExec::try_to_proto`. - Replace the body of `try_into_projection_physical_plan` with a thin shim that builds an `ExecutionPlanDecodeCtx` (via `ConverterPlanDecoder`) and delegates to `ProjectionExec::try_from_proto`; the now-unused `projection` param is renamed to `_projection`. - Drop the now-unused `ProjectionExpr` import. The `#[deprecated(...)]` attributes and signatures are otherwise unchanged, and the live dispatch arms and the `ProjectionExec` hook impls are untouched. ## Are these changes tested? Yes — the existing `roundtrip_physical_plan` projection/filter roundtrip tests exercise this path and pass (`roundtrip_projection_source`, `roundtrip_filter_with_fetch`, `roundtrip_filter_with_not_and_in_list`, and the full 203-test `roundtrip` suite). `cargo clippy -p datafusion-proto --all-features -- -D warnings` is clean. ## Are there any user-facing changes? No. The methods remain `#[deprecated]` with identical signatures and wire format; only their internals change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01U9qX6kekbJGpmTSxvrU8e5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

## Which issue does this PR close?  - Closes apache#23499. ## Rationale for this change  The centralized protobuf dispatcher couples each execution plan to datafusion-proto. Adding plan serialization therefore requires changes outside the plan implementation. Move FilterExec encoding and decoding behind its ExecutionPlan hooks. Preserve the existing wire format and retain the old helper methods as deprecated compatibility APIs. ## What changes are included in this PR?  ## Are these changes tested?  Yes ## Are there any user-facing changes?   --------- Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>

…w hooks (apache#23731) ## Which issue does this PR close? - Part of apache#23494. ## Rationale for this change apache#23495 landed the deprecated `ProjectionExec` compatibility shims (`try_from_projection_exec` / `try_into_projection_physical_plan`) with their original bodies fully duplicated in `datafusion-proto`, even though the same wire-format logic now lives in `ProjectionExec::try_to_proto` / `try_from_proto` (the apache#22419 hook pattern). This PR makes those deprecated shims **delegate** to the new hooks so the wire format is single-sourced in `datafusion-physical-plan` rather than duplicated. This matches the `FilterExec` delegate style in apache#23708. No wire-format change. ## What changes are included in this PR? - Replace the body of `try_from_projection_exec` with a thin shim that builds an `ExecutionPlanEncodeCtx` from the `codec` + `proto_converter` (via `ConverterPlanEncoder`) and delegates to `ProjectionExec::try_to_proto`. - Replace the body of `try_into_projection_physical_plan` with a thin shim that builds an `ExecutionPlanDecodeCtx` (via `ConverterPlanDecoder`) and delegates to `ProjectionExec::try_from_proto`; the now-unused `projection` param is renamed to `_projection`. - Drop the now-unused `ProjectionExpr` import. The `#[deprecated(...)]` attributes and signatures are otherwise unchanged, and the live dispatch arms and the `ProjectionExec` hook impls are untouched. ## Are these changes tested? Yes — the existing `roundtrip_physical_plan` projection/filter roundtrip tests exercise this path and pass (`roundtrip_projection_source`, `roundtrip_filter_with_fetch`, `roundtrip_filter_with_not_and_in_list`, and the full 203-test `roundtrip` suite). `cargo clippy -p datafusion-proto --all-features -- -D warnings` is clean. ## Are there any user-facing changes? No. The methods remain `#[deprecated]` with identical signatures and wire format; only their internals change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01U9qX6kekbJGpmTSxvrU8e5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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 requested changes Jul 20, 2026

View reviewed changes

Phoenix500526 added 2 commits July 21, 2026 12:05

Phoenix500526 force-pushed the issue/23499 branch from 3d8e61c to 7172823 Compare July 21, 2026 05:16

adriangb approved these changes Jul 21, 2026

View reviewed changes

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

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

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

Phoenix500526 deleted the issue/23499 branch July 21, 2026 05:59

andygrove mentioned this pull request Aug 14, 2026

Unify the ExecBuilder pattern across physical operators #24386

Open

Navigation Menu

Uh oh!

refactor(proto): migrate FilterExec serde - #23708

refactor(proto): migrate FilterExec serde#23708
adriangb merged 2 commits into
apache:mainfrom
Phoenix500526:issue/23499

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 left a comment

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 left a comment

Choose a reason for hiding this comment

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