Skip to content

refactor(proto): migrate AsyncFuncExec to self-serializing proto - #23825

Merged
adriangb merged 1 commit into
apache:mainfrom
mattp5657:refactor/async-func-exec-proto-migration
Jul 24, 2026
Merged

refactor(proto): migrate AsyncFuncExec to self-serializing proto#23825
adriangb merged 1 commit into
apache:mainfrom
mattp5657:refactor/async-func-exec-proto-migration

Conversation

@mattp5657

Copy link
Copy Markdown
Contributor

Closes #23514. Part of #23494.

Migrate AsyncFuncExec proto encode/decode into the plan itself via try_to_proto / try_from_proto, removing its central-arm handling in physical_plan/mod.rs.

Rationale

datafusion-proto currently downcasts AsyncFuncExec in the central encode match and rebuilds it inline on decode. #23495 introduced self-serializing hooks so each plan owns its own wire format. This moves AsyncFuncExec onto that pattern.

Changes

  • Added try_to_proto / AsyncFuncExec::try_from_proto, wired into the decode dispatch
  • Removed the central encode downcast branch
  • Old helper methods kept as #[deprecated] stubs, per existing convention
  • Wire format unchanged

Testing

Existing roundtrip_async_func_exec integration test now exercises the new hooks (old path deleted, so it's the only path left). cargo fmt + cargo clippy --all-targets --features proto -- -D warnings clean.

User-facing changes

No

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

Copy link
Copy Markdown

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.1.0 (current)
       Built [  36.879s] (current)
     Parsing datafusion-physical-plan v54.1.0 (current)
      Parsed [   0.148s] (current)
    Building datafusion-physical-plan v54.1.0 (baseline)
       Built [  35.803s] (baseline)
     Parsing datafusion-physical-plan v54.1.0 (baseline)
      Parsed [   0.149s] (baseline)
    Checking datafusion-physical-plan v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.938s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  75.409s] datafusion-physical-plan
    Building datafusion-proto v54.1.0 (current)
       Built [  59.376s] (current)
     Parsing datafusion-proto v54.1.0 (current)
      Parsed [   0.021s] (current)
    Building datafusion-proto v54.1.0 (baseline)
       Built [  64.887s] (baseline)
     Parsing datafusion-proto v54.1.0 (baseline)
      Parsed [   0.023s] (baseline)
    Checking datafusion-proto v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.330s] 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.49.0/src/lints/trait_method_marked_deprecated.ron

Failed in:
  method try_into_async_func_physical_plan in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:721
  method try_from_async_func_exec in trait datafusion_proto::physical_plan::PhysicalPlanNodeExt in /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:721

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

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Jul 22, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.27778% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.70%. Comparing base (c118002) to head (63b1850).

Files with missing lines Patch % Lines
datafusion/proto/src/physical_plan/mod.rs 5.26% 18 Missing ⚠️
datafusion/physical-plan/src/async_func.rs 86.79% 2 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23825      +/-   ##
==========================================
- Coverage   80.72%   80.70%   -0.02%     
==========================================
  Files        1089     1089              
  Lines      368911   368932      +21     
  Branches   368911   368932      +21     
==========================================
- Hits       297785   297762      -23     
- Misses      53374    53409      +35     
- Partials    17752    17761       +9     

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@adriangb
adriangb added this pull request to the merge queue Jul 24, 2026
Merged via the queue into apache:main with commit 16471ee Jul 24, 2026
40 checks passed
kosiew pushed a commit to kosiew/datafusion that referenced this pull request Aug 12, 2026
…che#23825)

Closes apache#23514. Part of apache#23494.

Migrate `AsyncFuncExec` proto encode/decode into the plan itself via
`try_to_proto` / `try_from_proto`, removing its central-arm handling in
`physical_plan/mod.rs`.

## Rationale

`datafusion-proto` currently downcasts `AsyncFuncExec` in the central
encode match and rebuilds it inline on decode. apache#23495 introduced
self-serializing hooks so each plan owns its own wire format. This moves
`AsyncFuncExec` onto that pattern.

## Changes

- Added `try_to_proto` / `AsyncFuncExec::try_from_proto`, wired into the
decode dispatch
- Removed the central encode downcast branch
- Old helper methods kept as `#[deprecated]` stubs, per existing
convention
- Wire format unchanged

## Testing

Existing `roundtrip_async_func_exec` integration test now exercises the
new hooks (old path deleted, so it's the only path left). `cargo fmt` +
`cargo clippy --all-targets --features proto -- -D warnings` clean.

## User-facing changes

No

Co-authored-by: Matthew Patton <matthewpatton@macbookpro.mynetworksettings.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api change Auto detected API change physical-plan Changes to the physical-plan crate proto Related to proto crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proto: migrate AsyncFuncExec

3 participants