feat: allow the customization of analyzer rules #5963
Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
| pub fn with_rules( | ||
| analyzer_rules: Vec<Arc<dyn AnalyzerRule + Send + Sync>>, | ||
| rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>, | ||
| ) -> Self { | ||
| Self { | ||
| analyzer_rules, | ||
| rules, | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm unsure if this is a proper place to customize analyzer rules. But from the code structure Analyzer is part of Optimizer. And only Optimizer is exposed to the context. Please let me know what you think @alamb @jackwener
|
I think put |
Same concern. To avoid the second breaking change in the future we'd better separate them at the beginning. I'll change the config part, add |
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
waynexia
force-pushed
the
analyzer-no-default
branch
from
80d3543 to
3c58741
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thanks @waynexia -- this looks like a good idea to me.
Could you also add a "end user" example of how to use this new analyzer rule framework to datafusion-examples?
Perhaps we can extend the existing one here:
The rationale is both to document the API for potential users but also to help ensure that we don't accidentally remove / break this API in the future during a refactpor
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Great suggestion! 👍 I have expanded the example to illustrate how to create and implement a basic analyzer rule. |
Co-authored-by: jakevin <jakevingoo@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks great to me -- thank you @waynexia and @jackwener
| // run the analyzer with our custom rule | ||
| let config = OptimizerContext::default().with_skip_failing_rules(false); | ||
| let optimized_plan = optimizer.optimize(&logical_plan, &config, observe)?; | ||
| let analyzer = Analyzer::with_rules(vec![Arc::new(MyAnalyzerRule {})]); |
3 participants
Which issue does this PR close?
Closes #.
Rationale for this change
Like
Optimizerallows to customize its rules, this patch adds a similar interface toAnalyzerand makes it public.What changes are included in this PR?
Allow users to insert their customized analyzer rules into the optimizer.
Are these changes tested?
Are there any user-facing changes?
Yes but not breaking. This publics the
AnalyzerandAnalyzerRule. And add corresponding config interfaces toSessionContext