docs: Add Expr library developer page
#7359
Conversation
|
Thanks @tshauck -- I will try and read this carefully either later today or tomorrow. I am very excited to do so |
There was a problem hiding this comment.
Thank you @tshauck -- this was a great read. I had a few minor suggestions, but otherwise I think this PR is ready to go.
I think this kind of content will help future users of DataFusion a lot. Thank you so much for writing it ❤️
| // recurse down and optimize children first | ||
| let optimized_plan = utils::optimize_children(self, plan, config)?; | ||
|
|
||
| match optimized_plan { |
There was a problem hiding this comment.
I think you can use plan.expressions() and then from_plan: https://docs.rs/datafusion/latest/datafusion/logical_expr/utils/fn.from_plan.html to rewrite expressions anywhere they appear.
It took me a while to find "from_plan" -- I will propose moving it to LogicalPlan as a method.
There was a problem hiding this comment.
Thanks for this, it seems to clean things up nicely, though please lmk if I'm not using something correctly. Also, I updated the code to use the new method proposed in #7396... if that doesn't go in I can update this back.
Co-authored-by: Andrew Lamb <[email protected]>
|
Thanks for the review! -- I've committed the quick changes and'll have a look at the rest over the next few days. I'll follow up when that's ready for review. |
|
I think this is ready for review, thanks! cc/ @alamb |
|
|
||
| let inputs = plan.inputs().into_iter().cloned().collect::<Vec<_>>(); | ||
|
|
||
| let plan = plan.with_new_exprs(&new_expressions, &inputs); |
Which issue does this PR close?
Closes #7304
Rationale for this change
This fills out the expr and UDF pages a bit. I tried to cover enough of the UDFs so that I could link to it from the expr page. For the expr page, it doesn't match the outline exactly -- I probably have some blindspots, so certainly open to feedback.
What changes are included in this PR?
Are these changes tested?
Manually built the docs to review output.
Are there any user-facing changes?
Yes