Skip to content

Move children isolator union insertion before distribution - #565

Merged
gabotechs merged 1 commit into
datafusion-contrib:mainfrom
alexanderbianchi:codex/children-isolator-union-prepass
Jul 27, 2026
Merged

Move children isolator union insertion before distribution#565
gabotechs merged 1 commit into
datafusion-contrib:mainfrom
alexanderbianchi:codex/children-isolator-union-prepass

Conversation

@alexanderbianchi

Copy link
Copy Markdown
Contributor

What changed

  • Add a pre-pass that replaces UnionExec with ChildrenIsolatorUnionExec before annotation and network-boundary distribution.
  • Keep the pre-inserted placeholder single-node-correct by mapping all children into one default task.
  • Make annotation react to pre-inserted ChildrenIsolatorUnionExec for child task-count propagation without constructing the node there.
  • Finalize the per-task child map during distribute_plan, where final task counts are available.

Why

Fixes #539. This keeps union-isolator insertion out of the network-boundary distribution recursion while preserving the existing per-task assignment behavior.

Validation

  • cargo fmt
  • cargo test --lib
  • cargo test --lib insert_children_isolator_union
  • `cargo test --features integration

@alexanderbianchi
alexanderbianchi force-pushed the codex/children-isolator-union-prepass branch from e343ef8 to 22f631e Compare July 26, 2026 19:11
@alexanderbianchi
alexanderbianchi marked this pull request as ready for review July 26, 2026 19:15
@alexanderbianchi
alexanderbianchi force-pushed the codex/children-isolator-union-prepass branch from 22f631e to 44fe33e Compare July 26, 2026 19:18

@gabotechs gabotechs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! I was expecting this to be a bit more complicated, but it turns out it was simpler. 👍 good work.

Comment on lines +132 to +133
/// Creates a single-node union placeholder with every child assigned to the default task.
pub(crate) fn new(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd probably call this "new_single_task" or something more descriptive.

@alexanderbianchi
alexanderbianchi force-pushed the codex/children-isolator-union-prepass branch from 44fe33e to 4f63440 Compare July 27, 2026 14:48

@gabotechs gabotechs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 Looks good, thanks @alexanderbianchi!

@gabotechs
gabotechs merged commit e5f6f81 into datafusion-contrib:main Jul 27, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move ChildrenIsolatorUnionExec injection out of the inject_network_boundaries function

2 participants