alamb commented on a change in pull request #8633:
URL: https://github.com/apache/arrow/pull/8633#discussion_r520919459



##########
File path: rust/datafusion/src/optimizer/filter_push_down.rs
##########
@@ -126,13 +126,13 @@ impl OptimizerRule for FilterPushDown {
             }
 
             // AND filter expressions that would be placed on the same depth
-            if let Some(existing_expression) = 
new_filtersnew_filters.get(&new_depth) {

Review comment:
       I am not sure what was going on with this variable name, but I figured I 
would reduce the replication ;)

##########
File path: rust/datafusion/src/optimizer/filter_push_down.rs
##########
@@ -214,6 +214,36 @@ fn analyze_plan(plan: &LogicalPlan, depth: usize) -> 
Result<AnalysisResult> {
             result.break_points.insert(depth, columns);
             Ok(result)
         }
+        LogicalPlan::Extension { node } => {
+            let inputs = node.inputs();
+
+            let mut result = match inputs.len() {
+                0 => AnalysisResult {
+                    break_points: BTreeMap::new(),
+                    filters: BTreeMap::new(),
+                    projections: BTreeMap::new(),
+                },
+                1 => analyze_plan(inputs[0], depth + 1)?,
+                _ => {
+                    return Err(DataFusionError::NotImplemented(

Review comment:
       
   More generally, I am not sure how / if this filter pushdown logic will work 
if there are multiple child inputs as no existing LogicalPlan variant his 
multiple inputs at this time. We will have to handle this when / if we want to 
support joins as well . For now I returned a "Not Yet Implemented" type error




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

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


Reply via email to