Reduce repetition in try_process_group_by_unnest and try_process_unnest #11714
Conversation
There was a problem hiding this comment.
Thank you @JasonLi-cn -- this looks like an improvement to me ❤️
cc @jonahgao
| inner_projection_exprs: &mut Vec<Expr>, | ||
| original_exprs: &[Expr], | ||
| ) -> Result<Vec<Expr>> { | ||
| Ok(original_exprs |
There was a problem hiding this comment.
This does the recursion manually -- I wonder if it would be possible / useful to do the recursion using transform_up from TreeNode: https://docs.rs/datafusion/latest/datafusion/common/tree_node/trait.TreeNode.html#method.transform_up
There was a problem hiding this comment.
Thank you @alamb for your review. I'm not quite sure what you mean, maybe you can add more description, thanks again. 🙏
There was a problem hiding this comment.
I think I was confused -- sorry
There was a problem hiding this comment.
I think we can, in fact transform_up_down is needed, i'll introduce why in this PR when it's ready: #11577
|
Thanks everyone and thank you @duongcongtoai for the follow up |
Which issue does this PR close?
Closes #11498 (comment)
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?