alamb commented on code in PR #4701:
URL: https://github.com/apache/arrow-datafusion/pull/4701#discussion_r1054829813


##########
datafusion/sql/src/planner.rs:
##########
@@ -970,6 +970,45 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         }
     }
 
+    fn infer_placeholder_types(&self, expr: &mut Expr, schema: &DFSchema) -> 
Result<()> {
+        match expr {
+            Expr::BinaryExpr(BinaryExpr {
+                ref mut left,
+                ref mut right,
+                ..
+            }) => {
+                self.infer_placeholder_types(left, schema)?;
+                self.infer_placeholder_types(right, schema)?;
+                let lt = left.get_type(schema);
+                let rt = right.get_type(schema);
+                match (left, rt) {
+                    (
+                        box Expr::Placeholder {

Review Comment:
   I think using an `ExprRewriter` 
https://docs.rs/datafusion-expr/15.0.0/datafusion_expr/expr_rewriter/trait.ExprRewriter.html#
 would let you do what you want -- rather than getting a `mut &` you get the 
entire `expr` by value (and can then rewrite it as you like).
   
   Bonus points is that it will handle the recursion for you
   
   Here is an example of using it:
   
https://github.com/apache/arrow-datafusion/blob/bfef10541f2a989f76f1b98532a8aa0bcde04617/datafusion/expr/src/logical_plan/plan.rs#L615-L648
   
   
   You could perhaps destructure / rewrite the BinaryExpr  then
   
   



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to