xudong963 commented on code in PR #21749:
URL: https://github.com/apache/datafusion/pull/21749#discussion_r3115367445


##########
datafusion/common/src/tree_node.rs:
##########
@@ -2431,4 +2450,46 @@ pub(crate) mod tests {
 
         item.visit(&mut visitor).unwrap();
     }
+
+    #[test]
+    fn box_map_elements_reuses_allocation() {
+        let boxed = Box::new(TestTreeNode::new_leaf(42i32));
+        let before: *const TestTreeNode<i32> = &*boxed;
+        let out = boxed.map_elements(|n| Ok(Transformed::no(n))).unwrap();
+        let after: *const TestTreeNode<i32> = &*out.data;
+        assert_eq!(after, before);
+    }
+
+    #[test]
+    fn arc_map_elements_reuses_allocation_when_unique() {
+        let arc = Arc::new(TestTreeNode::new_leaf(42i32));
+        let before = Arc::as_ptr(&arc);
+        let out = arc.map_elements(|n| Ok(Transformed::no(n))).unwrap();
+        assert_eq!(Arc::as_ptr(&out.data), before);
+    }
+
+    #[test]
+    fn arc_map_elements_clones_when_shared() {
+        // When the input `Arc` is shared, `make_mut` clones into a fresh
+        // allocation, so the reuse optimization does not apply.
+        let arc = Arc::new(TestTreeNode::new_leaf(42i32));
+        let _keepalive = Arc::clone(&arc);
+        let before = Arc::as_ptr(&arc);
+        let out = arc.map_elements(|n| Ok(Transformed::no(n))).unwrap();
+        assert_ne!(Arc::as_ptr(&out.data), before);
+    }
+
+    #[test]
+    fn box_map_elements_panic() {
+        use std::panic::{AssertUnwindSafe, catch_unwind};
+        let boxed = Box::new(TestTreeNode::new_leaf(42i32));
+        let result = catch_unwind(AssertUnwindSafe(|| {
+            boxed
+                .map_elements(|_: TestTreeNode<i32>| -> Result<_> {
+                    panic!("simulated panic during rewrite")
+                })
+                .ok()
+        }));
+        assert!(result.is_err());

Review Comment:
   It doesn't verify the actual panic-safety invariant — that dropping an 
unwound `Box` doesn't double-free or access garbage



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to