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 2020/11/10 22:40:59 UTC

[GitHub] [arrow] alamb commented on a change in pull request #8633: ARROW-10547: Do not lose Filters with UserDefined plan nodes

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:
users@infra.apache.org