Skip to content

refactor(ir): rename OperatorNode::map_children to with_new_children - #648

Merged
zzylol merged 2 commits into
mainfrom
refactor/with-new-children
Oct 7, 2026
Merged

zzylol merged 2 commits into
mainfrom
refactor/with-new-children

Conversation

@zzylol

@zzylol zzylol commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Why

OperatorNode::map_children(f) does more than map over the children: it replaces the children and rebuilds the node. Each child is replaced by f(child), then the node is rebuilt over the new children with OperatorNode::new, re-deriving its schema and result kind. Output names and qualifiers the caller overrode are kept, and guarantee and timing are cleared. The name map_children hides that rebuild, and it is shared with Operator::map_children, which is a plain map over child references (it can change the reference type, e.g. Rc<OperatorNode> → node id, and derives nothing).

This PR renames the node-level method to with_new_children, the name DataFusion uses for the same "replace the children and rebuild" operation, and says so in its doc comment.

impl OperatorNode {
    /// Replace the children and rebuild: each child is replaced by `f(child)`
    /// and the node is rebuilt over the new children, re-deriving all
    /// structural schema metadata. ...
    pub fn with_new_children(
        &self,
        f: impl FnMut(&Rc<OperatorNode>) -> Rc<OperatorNode>,
    ) -> Result<Self, SchemaDerivationError>;
}

Scope

  • Renamed: OperatorNode::map_children → OperatorNode::with_new_children. Callers updated: canonicalize.rs and the schema_rebuilding / summary_coverage tests.
  • Unchanged: Operator::map_children, ASAPOp::map_children, NonASAPOp::map_children (plain maps over child references, used by CSE and flattening).
  • No behavior change.

Validation: workspace tests (1,625 passed), cargo fmt --check, workspace/all-target Clippy with warnings denied.

Raised in review of #646.

🤖 Generated with Claude Code

zzylol and others added 2 commits October 7, 2026 13:37
The node-level method replaces each child and rebuilds the node,
re-deriving its schema; it is not a plain map over children. The
generic Operator/ASAPOp/NonASAPOp::map_children, which only maps child
references, keeps its name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol merged commit 0ab650e into main Oct 7, 2026
3 checks passed
@zzylol
zzylol deleted the refactor/with-new-children branch October 7, 2026 14:49
zzylol added a commit that referenced this pull request Oct 7, 2026
Planned summary states no longer declare coverage: it is derived from the
node (#646). Node copies use clone + field updates (the coverage cache is
private), physical DAG fixtures drop the removed coverage field, and the
design example uses with_new_children (#648).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W7qG9aFyPij5uWsyAJCxDW
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.

1 participant