alamb commented on code in PR #10473:
URL: https://github.com/apache/datafusion/pull/10473#discussion_r1600030182


##########
datafusion/common/src/tree_node.rs:
##########
@@ -204,6 +214,24 @@ pub trait TreeNode: Sized {
         apply_impl(self, &mut f)
     }
 
+    fn apply_ref<'n, F: FnMut(&'n Self) -> Result<TreeNodeRecursion>>(

Review Comment:
   This API would be helpful in other areas to avoid cloning -- I am very much 
in favor of adding it



##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -1169,21 +1281,73 @@ mod test {
     #[test]
     fn redundant_project_fields() {
         let table_scan = test_table_scan().unwrap();
+        let c_plus_a = col("c") + col("a");
+        let b_plus_a = col("b") + col("a");
         let expr_stats_1 = ExprStats::from([
-            ("c+a".to_string(), (1, DataType::UInt32)),
-            ("b+a".to_string(), (1, DataType::UInt32)),
+            (
+                Identifier {
+                    hash: 0,
+                    expr: &c_plus_a,
+                },
+                (1, DataType::UInt32),
+            ),

Review Comment:
   I wonder if we could make these tests more readable with something like this 
or similar
   
   ```suggestion
               (
                   Identifier::from(&c_plus_a).with_hash(0)
                   (1, DataType::UInt32),
               ),
   ```
   
   or
   
   ```suggestion
               (
                   make_identifier(&c_plus_a, 0),
                   (1, DataType::UInt32),
               ),
   ```



-- 
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