-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MINOR: Optimizer example and docs, deprecate Expr::name
#3788
Conversation
datafusion/optimizer/README.md
Outdated
/// `OptimizerRule` transforms one ['LogicalPlan'] into another which | ||
/// computes the same results, but in a potentially more efficient | ||
/// way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to emphasize that in the event of there are no transformations to apply (for the given input), the optimizer could (and should) return the input as is (e.g. instead of failing with an exception)?
/// `OptimizerRule` can transform one ['LogicalPlan'] into another which
/// computes the same results, but in a potentially more efficient way. If
/// there are no suitable transformations for the input plan, the optimizer can
/// simply return it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a very good point. I will add this.
Expr::name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice @andygrove 👌
@@ -0,0 +1,163 @@ | |||
// Licensed to the Apache Software Foundation (ASF) under one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally recommend putting this example into datafusion/examples
rather than datafusion/optimizer/examples
so that it is easier to find
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Benchmark runs are scheduled for baseline = a226587 and contender = 4cb8ac0. 4cb8ac0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
🎉 |
Which issue does this PR close?
N/A
Rationale for this change
I want to make it easier for new contributors to be able to write and review optimization rules.
You can see the rendered version of the new README here
What changes are included in this PR?
Expr::name()
toExpr::display_name()
Expr::name()
that delegates toExpr::display_name()
Are there any user-facing changes?
No