Skip to content

[branch-54] Support transparent ExecutionPlan downcasts#22565

Open
geoffreyclaude wants to merge 1 commit into
apache:branch-54from
geoffreyclaude:fix/transparent-plan-downcast-branch-54
Open

[branch-54] Support transparent ExecutionPlan downcasts#22565
geoffreyclaude wants to merge 1 commit into
apache:branch-54from
geoffreyclaude:fix/transparent-plan-downcast-branch-54

Conversation

@geoffreyclaude
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

DataFusion 54 changed ExecutionPlan downcasting to use the Any supertrait directly. That removes ExecutionPlan::as_any, which had also served as a customization point for wrapper nodes: wrappers could identify as themselves internally while exposing the wrapped plan type to normal downcast-based inspection.

This PR adds an explicit ExecutionPlan::downcast_delegate() hook for wrapper nodes that want their public ExecutionPlan downcast identity to be delegated to another plan.

The proposed behavior intentionally preserves the old as_any override semantics: when a node opts into downcast delegation, intermediate delegating wrappers are invisible to dyn ExecutionPlan::is::<T>() and downcast_ref::<T>().

What changes are included in this PR?

  • Adds ExecutionPlan::downcast_delegate() with a default implementation returning None.
  • Updates dyn ExecutionPlan::is::<T>() and downcast_ref::<T>() to delegate to downcast_delegate() when present, otherwise use the current concrete plan type.
  • Documents that downcast_delegate() is only for type introspection and is independent from children() / plan traversal.
  • Adds tests for direct and nested downcast-delegating wrappers, including that intermediate delegating wrappers remain invisible to normal downcast-based inspection.

Are these changes tested?

Yes.

  • cargo test -p datafusion-physical-plan execution_plan_downcast
  • cargo test -p datafusion-physical-plan --lib
  • cargo fmt --all -- --check
  • git diff --check

Are there any user-facing changes?

Yes. This adds a new public ExecutionPlan trait method with a default implementation, and it changes ExecutionPlan downcast helpers to honor wrappers that explicitly opt into delegating public downcast identity.

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label May 27, 2026
Copy link
Copy Markdown
Contributor

@gabotechs gabotechs left a comment

Choose a reason for hiding this comment

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

Leaving a +1, but worth merging after the original PR is merged

@geoffreyclaude geoffreyclaude force-pushed the fix/transparent-plan-downcast-branch-54 branch from 6fc9029 to 899a319 Compare May 27, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants