Skip to content
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

Add standalone example for OptimizerRule #11087

Merged
merged 6 commits into from
Jun 26, 2024
Merged

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jun 23, 2024

Which issue does this PR close?

Part of #10855

Rationale for this change

The current rewrite_expr.rs example has three distinct examples:

  1. Using DataFusion LogicalPlanning without execution
  2. Implementing an AnalyzerRule
  3. Implementing an OptimizerRule

Each I think should be its own example to make them easier to find and more full featured

What changes are included in this PR?

  1. Add example optimizer_rule.rs
  2. Add SessionContext::add_optimizer_rule as a convenience
  3. Change signature of newly added SessionContext::add_analyzer_rule to take &mut self to be consistent

Are these changes tested?

Yes, by CI test on examples

@github-actions github-actions bot added the core Core DataFusion crate label Jun 23, 2024
@@ -402,6 +402,16 @@ impl SessionState {
self
}

// the add_optimizer_rule takes an owned reference
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed the naming in SessionState is pretty inconsistent

/// Adds an analyzer rule to the end of the existing rules.
///
/// See [`SessionState`] for more control of when the rule is applied.
pub fn add_analyzer_rule(&self, analyzer_rule: Arc<dyn AnalyzerRule + Send + Sync>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API was added in #10849 by @pingsutw and not yet released, thus it is not an API change

@alamb alamb marked this pull request as ready for review June 23, 2024 22:34
@alamb alamb added the documentation Improvements or additions to documentation label Jun 23, 2024
// +--------+-----+
// | Andy | 11 |
// | Andrew | 22 |
// | Oleks | 33 |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @alamb this is very clear example on plan rewrite

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 24, 2024
@alamb alamb merged commit aff777b into apache:main Jun 26, 2024
23 checks passed
@alamb
Copy link
Contributor Author

alamb commented Jun 26, 2024

🚀

@alamb alamb deleted the alamb/optimizer_rule branch June 26, 2024 17:32
/// [`OptimizerRule`]s transform [`LogicalPlan`]s into an equivalent (but
/// hopefully faster) form.
///
/// See [analyzer_rule.rs] for an example of AnalyzerRules, which are for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is analyzer_rule.rs a file in the repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be once I can get someone to approve #11089

findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
* Add standalone example for `OptimizerRule`

* Fix typo

* Update datafusion-examples/examples/optimizer_rule.rs

Co-authored-by: Oleks V <[email protected]>

* fmt

---------

Co-authored-by: Oleks V <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants