[branch-54] Support transparent ExecutionPlan downcasts#22565
Open
geoffreyclaude wants to merge 1 commit into
Open
[branch-54] Support transparent ExecutionPlan downcasts#22565geoffreyclaude wants to merge 1 commit into
geoffreyclaude wants to merge 1 commit into
Conversation
gabotechs
approved these changes
May 27, 2026
Contributor
gabotechs
left a comment
There was a problem hiding this comment.
Leaving a +1, but worth merging after the original PR is merged
6fc9029 to
899a319
Compare
timsaucer
approved these changes
May 27, 2026
50 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
branch-54.Rationale for this change
DataFusion 54 changed
ExecutionPlandowncasting to use theAnysupertrait directly. That removesExecutionPlan::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 publicExecutionPlandowncast identity to be delegated to another plan.The proposed behavior intentionally preserves the old
as_anyoverride semantics: when a node opts into downcast delegation, intermediate delegating wrappers are invisible todyn ExecutionPlan::is::<T>()anddowncast_ref::<T>().What changes are included in this PR?
ExecutionPlan::downcast_delegate()with a default implementation returningNone.dyn ExecutionPlan::is::<T>()anddowncast_ref::<T>()to delegate todowncast_delegate()when present, otherwise use the current concrete plan type.downcast_delegate()is only for type introspection and is independent fromchildren()/ plan traversal.Are these changes tested?
Yes.
cargo test -p datafusion-physical-plan execution_plan_downcastcargo test -p datafusion-physical-plan --libcargo fmt --all -- --checkgit diff --checkAre there any user-facing changes?
Yes. This adds a new public
ExecutionPlantrait method with a default implementation, and it changesExecutionPlandowncast helpers to honor wrappers that explicitly opt into delegating public downcast identity.