-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Improve documentation for registering AnalyzerRule
#9520
Conversation
pub fn add_physical_optimizer_rule( | ||
mut self, | ||
optimizer_rule: Arc<dyn PhysicalOptimizerRule + Send + Sync>, | ||
physical_optimizer_rule: Arc<dyn PhysicalOptimizerRule + Send + Sync>, |
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.
👍
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.
lgtm thanks @alamb I left some suggestion for more concise phrasing if you okay with that
@@ -145,7 +145,7 @@ | |||
//! * define your own catalogs, schemas, and table lists ([`CatalogProvider`]) | |||
//! * build your own query language or plans ([`LogicalPlanBuilder`]) | |||
//! * declare and use user-defined functions ([`ScalarUDF`], and [`AggregateUDF`], [`WindowUDF`]) | |||
//! * add custom optimizer rewrite passes ([`OptimizerRule`] and [`PhysicalOptimizerRule`]) | |||
//! * add custom plan rewrite passes ([`AnalyzerRule`], [`OptimizerRule`] and [`PhysicalOptimizerRule`]) |
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.
contribution 1: Mention AnalyzerRule
in this list
/// Use [`SessionState::add_physical_optimizer_rule`] to register additional | ||
/// `PhysicalOptimizerRule`s.. | ||
/// | ||
/// [`SessionState::add_physical_optimizer_rule`]: https://docs.rs/datafusion/latest/datafusion/execution/context/struct.SessionState.html#method.add_physical_optimizer_rule |
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.
Contrbution 2: Add links to how to register these rules with SessionContext (which was the API I didn't find initially)
Co-authored-by: comphead <comphead@users.noreply.github.com>
Which issue does this PR close?
Related to #9519
Rationale for this change
When filing #9519 I didn't think DataFusion allowed registering AnalyzerRules. However, in the middle of filing a ticket requesting we do so, I found
SessionState::add_analyzer_rule
I would like to make this clearer to find in the future
What changes are included in this PR?
Are these changes tested?
Doc tests
Are there any user-facing changes?
Documentation only (no code changes)