wiedld commented on code in PR #9685:
URL: https://github.com/apache/arrow-datafusion/pull/9685#discussion_r1531575684
##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -571,66 +580,73 @@ enum VisitRecord {
/// `usize` is the monotone increasing series number assigned in
pre_visit().
/// Starts from 0. Is used to index the identifier array `id_array` in
post_visit().
EnterMark(usize),
+ /// the node's children were skipped => jump to f_up on same node
+ JumpMark(usize),
/// Accumulated identifier of sub expression.
ExprItem(Identifier),
}
impl ExprIdentifierVisitor<'_> {
- fn desc_expr(expr: &Expr) -> String {
+ fn node_identifier(expr: &Expr) -> String {
format!("{expr}")
}
/// Find the first `EnterMark` in the stack, and accumulates every
`ExprItem`
/// before it.
- fn pop_enter_mark(&mut self) -> Option<(usize, Identifier)> {
+ fn pop_enter_mark(&mut self) -> (usize, Identifier) {
let mut desc = String::new();
while let Some(item) = self.visit_stack.pop() {
match item {
- VisitRecord::EnterMark(idx) => {
- return Some((idx, desc));
+ VisitRecord::EnterMark(idx) | VisitRecord::JumpMark(idx) => {
+ return (idx, desc);
}
- VisitRecord::ExprItem(s) => {
- desc.push_str(&s);
+ VisitRecord::ExprItem(id) => {
+ desc.push_str(&id);
}
}
}
- None
+ unreachable!("Enter mark should paired with node number");
Review Comment:
Bring back the pop_enter_mark() contract to the original (before the
TreeNode refactor), while also adding in a new JumpMark.
--
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]