Skip to content

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
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
@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