You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/05 20:07:45 UTC

[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3692: Move optimizer init to optimizer crate

andygrove commented on code in PR #3692:
URL: https://github.com/apache/arrow-datafusion/pull/3692#discussion_r985990969


##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -107,8 +134,35 @@ pub struct Optimizer {
 }
 
 impl Optimizer {
+    /// Create a new optimizer using the recommended list of rules
+    pub fn new(config: &OptimizerConfig) -> Self {
+        let mut rules: Vec<Arc<dyn OptimizerRule + Sync + Send>> = vec![
+            Arc::new(TypeCoercion::new()),
+            Arc::new(SimplifyExpressions::new()),
+            Arc::new(UnwrapCastInComparison::new()),
+            Arc::new(DecorrelateWhereExists::new()),
+            Arc::new(DecorrelateWhereIn::new()),
+            Arc::new(ScalarSubqueryToJoin::new()),
+            Arc::new(SubqueryFilterToJoin::new()),
+            Arc::new(EliminateFilter::new()),
+            Arc::new(ReduceCrossJoin::new()),
+            Arc::new(CommonSubexprEliminate::new()),
+            Arc::new(EliminateLimit::new()),
+            Arc::new(ProjectionPushDown::new()),
+            Arc::new(RewriteDisjunctivePredicate::new()),
+        ];
+        if config.filter_null_keys {
+            rules.push(Arc::new(FilterNullJoinKeys::default()));
+        }
+        rules.push(Arc::new(ReduceOuterJoin::new()));
+        rules.push(Arc::new(FilterPushDown::new()));
+        rules.push(Arc::new(LimitPushDown::new()));
+        rules.push(Arc::new(SingleDistinctToGroupBy::new()));
+        Self::with_rules(rules)
+    }
+
     /// Create a new optimizer with the given rules
-    pub fn new(rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>) -> Self {
+    pub fn with_rules(rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>) -> Self {

Review Comment:
   One thing that is odd now is that we pass a config into the constructor and also pass it into the optimize method - I plan to create a separate PR to see if we can change that.
   
   ```rust
       let mut config = OptimizerConfig::new().with_skip_failing_rules(false);
       let optimizer = Optimizer::new(&config);
       optimizer.optimize(&plan, &mut config, &observe)
   ```



##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -107,8 +134,35 @@ pub struct Optimizer {
 }
 
 impl Optimizer {
+    /// Create a new optimizer using the recommended list of rules
+    pub fn new(config: &OptimizerConfig) -> Self {
+        let mut rules: Vec<Arc<dyn OptimizerRule + Sync + Send>> = vec![
+            Arc::new(TypeCoercion::new()),
+            Arc::new(SimplifyExpressions::new()),
+            Arc::new(UnwrapCastInComparison::new()),
+            Arc::new(DecorrelateWhereExists::new()),
+            Arc::new(DecorrelateWhereIn::new()),
+            Arc::new(ScalarSubqueryToJoin::new()),
+            Arc::new(SubqueryFilterToJoin::new()),
+            Arc::new(EliminateFilter::new()),
+            Arc::new(ReduceCrossJoin::new()),
+            Arc::new(CommonSubexprEliminate::new()),
+            Arc::new(EliminateLimit::new()),
+            Arc::new(ProjectionPushDown::new()),
+            Arc::new(RewriteDisjunctivePredicate::new()),
+        ];
+        if config.filter_null_keys {
+            rules.push(Arc::new(FilterNullJoinKeys::default()));
+        }
+        rules.push(Arc::new(ReduceOuterJoin::new()));
+        rules.push(Arc::new(FilterPushDown::new()));
+        rules.push(Arc::new(LimitPushDown::new()));
+        rules.push(Arc::new(SingleDistinctToGroupBy::new()));
+        Self::with_rules(rules)
+    }
+
     /// Create a new optimizer with the given rules
-    pub fn new(rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>) -> Self {
+    pub fn with_rules(rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>) -> Self {

Review Comment:
   Ah... we need to figure out what to do with this id generator first



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org