alamb commented on code in PR #23198:
URL: https://github.com/apache/datafusion/pull/23198#discussion_r3501246541
##########
datafusion/expr/src/expr.rs:
##########
@@ -767,21 +768,112 @@ impl Alias {
}
}
+/// A `Box<Expr>` for the recursive children of [`BinaryExpr`]. A long
binary-operator chain
+/// (e.g. `a OR b OR c OR ...`) builds an `Expr` as deep as the chain, and
dropping it
+/// recursively would overflow the stack. This wrapper's `Drop` tears the
chain down iteratively.
Review Comment:
I feel like this is treating the symptom (overflow on drop) rather than the
root cause (in general processing deeply nested expressions using recursive
functions)
If we are going to change the representation of BinaryExpr and cause a bunch
of downstream churn, I think we should consider more drastic measures -- like
changing it to `BinaryExpr { .. ops:Vec<Expr> } `
I will file a ticket about this
--
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]
