From d4550840bacbafc1884b7ae760da5e116045bc7c Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Wed, 7 Oct 2026 13:37:02 +0000 Subject: [PATCH 1/2] refactor(ir): rename OperatorNode::map_children to with_new_children 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 --- crates/types/src/ir/canonicalize.rs | 2 +- crates/types/src/ir/node.rs | 7 ++++--- crates/types/tests/schema_rebuilding.rs | 8 ++++---- crates/types/tests/summary_coverage.rs | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/types/src/ir/canonicalize.rs b/crates/types/src/ir/canonicalize.rs index 797cf61bc..691ba6610 100644 --- a/crates/types/src/ir/canonicalize.rs +++ b/crates/types/src/ir/canonicalize.rs @@ -93,7 +93,7 @@ fn canon( .find(|(ptr, _)| *ptr == Rc::as_ptr(c)) .map_or_else(|| Rc::clone(c), |(_, new)| Rc::clone(new)) }; - Rc::new(node.map_children(rebuilt_child)?) + Rc::new(node.with_new_children(rebuilt_child)?) } else { Rc::clone(node) }; diff --git a/crates/types/src/ir/node.rs b/crates/types/src/ir/node.rs index 7534353b0..2ade2e5dd 100644 --- a/crates/types/src/ir/node.rs +++ b/crates/types/src/ir/node.rs @@ -199,12 +199,13 @@ impl OperatorNode { self.operator.children() } - /// Rebuild with new inputs, re-deriving all structural schema metadata. - /// Only names and qualifiers that override the old derived schema are + /// 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. Only names and qualifiers that override the old derived schema are /// retained, for either operator category. A change in output arity with /// such overrides needs an explicit new naming assignment. /// `guarantee` and `timing` depend on the inputs and are cleared. - pub fn map_children( + pub fn with_new_children( &self, f: impl FnMut(&Rc) -> Rc, ) -> Result { diff --git a/crates/types/tests/schema_rebuilding.rs b/crates/types/tests/schema_rebuilding.rs index fc7a3f145..b0fc4c666 100644 --- a/crates/types/tests/schema_rebuilding.rs +++ b/crates/types/tests/schema_rebuilding.rs @@ -77,7 +77,7 @@ fn rebuilding_rederives_schema_for_both_categories() { let original = aggregate(scan(DataType::Int64, "key"), asap); original.validate_structure().unwrap(); let replacement = scan(DataType::Utf8, "new_key"); - let rebuilt = redeclare(original.map_children(|_| replacement.clone()).unwrap()); + let rebuilt = redeclare(original.with_new_children(|_| replacement.clone()).unwrap()); assert_eq!(rebuilt.schema, rebuilt.operator.output_schema().unwrap()); rebuilt.validate_structure().unwrap(); } @@ -98,7 +98,7 @@ fn rebuilding_preserves_only_explicit_naming_overrides() { let original = Rc::new(renamed); original.validate_structure().unwrap(); let replacement = scan(DataType::Utf8, "new_key"); - let rebuilt = redeclare(original.map_children(|_| replacement.clone()).unwrap()); + let rebuilt = redeclare(original.with_new_children(|_| replacement.clone()).unwrap()); assert_eq!(rebuilt.schema.fields[0].name, "alias"); assert_eq!(rebuilt.schema.fields[0].table.as_deref(), Some("result")); assert_eq!( @@ -176,14 +176,14 @@ fn rebuilding_updates_metadata_and_requires_new_aliases_after_arity_changes() { schema: replacement_schema.clone(), })) .unwrap(); - let rebuilt = Rc::new(original.map_children(|_| replacement.clone()).unwrap()); + let rebuilt = Rc::new(original.with_new_children(|_| replacement.clone()).unwrap()); assert_eq!(rebuilt.schema, replacement_schema); rebuilt.validate_structure().unwrap(); let mut names = original.schema.clone(); names.fields[0].name = "alias".into(); let named = OperatorNode::with_schema(original.operator.clone(), names); - assert!(named.map_children(|_| replacement.clone()).is_err()); + assert!(named.with_new_children(|_| replacement.clone()).is_err()); } /// Maintaining membership and finalizing values preserve identity/time metadata. diff --git a/crates/types/tests/summary_coverage.rs b/crates/types/tests/summary_coverage.rs index 3b99e9bba..e36c3d7a4 100644 --- a/crates/types/tests/summary_coverage.rs +++ b/crates/types/tests/summary_coverage.rs @@ -124,7 +124,7 @@ fn node_coverage_is_required_checked_and_cleared_by_rewrites() { std::rc::Rc::new(state.clone()) .validate_structure() .unwrap(); - let rebuilt = state.map_children(Clone::clone).unwrap(); + let rebuilt = state.with_new_children(Clone::clone).unwrap(); assert!(rebuilt.coverage.is_none()); } From 75c03ec4ba67e09cec2948083adef47b8e6794c2 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:38:31 +0000 Subject: [PATCH 2/2] docs(ir): rewrap the with_new_children doc comment Co-Authored-By: Claude Opus 5.5 --- crates/types/src/ir/node.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/types/src/ir/node.rs b/crates/types/src/ir/node.rs index 2ade2e5dd..47ffa0ea3 100644 --- a/crates/types/src/ir/node.rs +++ b/crates/types/src/ir/node.rs @@ -201,9 +201,10 @@ 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. Only names and qualifiers that override the old derived schema are - /// retained, for either operator category. A change in output arity with - /// such overrides needs an explicit new naming assignment. + /// structural schema metadata. Only names and qualifiers that override + /// the old derived schema are retained, for either operator category. A + /// change in output arity with such overrides needs an explicit new + /// naming assignment. /// `guarantee` and `timing` depend on the inputs and are cleared. pub fn with_new_children( &self,