alamb commented on code in PR #2729:
URL: https://github.com/apache/arrow-datafusion/pull/2729#discussion_r897292665


##########
datafusion/optimizer/src/filter_push_down.rs:
##########
@@ -1831,4 +1870,111 @@ mod tests {
 
         Ok(())
     }
+

Review Comment:
   When I remove the code changes in this PR the tests still pass 🤔 
   
   So when I apply this diff:
   
   ```diff
   diff --git a/datafusion/optimizer/src/filter_push_down.rs 
b/datafusion/optimizer/src/filter_push_down.rs
   index 1629e95c5..2668eaaf7 100644
   --- a/datafusion/optimizer/src/filter_push_down.rs
   +++ b/datafusion/optimizer/src/filter_push_down.rs
   @@ -122,27 +122,6 @@ fn remove_filters(
            .collect::<Vec<_>>()
    }
    
   -// rename all filter columns which have alias name
   -fn rename_filters_column_name(
   -    filters: &mut [Predicate],
   -    alias_cols_expr_and_name: &HashMap<&Expr, &String>,
   -) {
   -    for (expr, columns) in filters {
   -        if alias_cols_expr_and_name.contains_key(expr) {
   -            let col_string = <&std::string::String>::clone(
   -                alias_cols_expr_and_name.get(expr).unwrap(),
   -            );
   -            let column = Column::from_qualified_name(col_string);
   -            if let Expr::Column(col) = expr {
   -                columns.remove(col);
   -            } else {
   -                unreachable!()
   -            }
   -            columns.insert(column);
   -        }
   -    }
   -}
   -
    /// builds a new [LogicalPlan] from `plan` by issuing new 
[LogicalPlan::Filter] if any of the filters
    /// in `state` depend on the columns `used_columns`.
    fn issue_filters(
   @@ -357,18 +336,6 @@ fn optimize(plan: &LogicalPlan, mut state: State) -> 
Result<LogicalPlan> {
            LogicalPlan::Analyze { .. } => push_down(&state, plan),
            LogicalPlan::Filter(Filter { input, predicate }) => {
                let mut predicates = vec![];
   -            let mut alias_cols_expr_and_name = HashMap::new();
   -            //Need rewrite column name before push down
   -            let input_plan = &*input.clone();
   -            if let LogicalPlan::Projection(projection) = input_plan {
   -                let exprs = &projection.expr;
   -                for e in exprs {
   -                    if let Expr::Alias(col_expr, alias_name) = e {
   -                        alias_cols_expr_and_name.insert(col_expr.as_ref(), 
alias_name);
   -                    }
   -                }
   -            }
   -
                utils::split_conjunction(predicate, &mut predicates);
    
                // Predicates without referencing columns (WHERE FALSE, WHERE 
1=1, etc.)
   @@ -397,12 +364,6 @@ fn optimize(plan: &LogicalPlan, mut state: State) -> 
Result<LogicalPlan> {
                        &no_col_predicates,
                    ))
                } else {
   -                if !alias_cols_expr_and_name.is_empty() {
   -                    rename_filters_column_name(
   -                        &mut state.filters,
   -                        &alias_cols_expr_and_name,
   -                    )
   -                }
                    optimize(input, state)
                }
            }
   ```
   
   The tests all pass:
   
   ```
   
   -*- mode: compilation; default-directory: "~/Software/arrow-datafusion/" -*-
   Compilation started at Tue Jun 14 16:27:52
   
   cd /Users/alamb/Software/arrow-datafusion && RUST_BACKTRACE=1 
CARGO_TARGET_DIR=/Users/alamb/Software/df-target cargo test -p 
datafusion-optimizer -- filter_push_down
      Compiling datafusion-optimizer v9.0.0 
(/Users/alamb/Software/arrow-datafusion/datafusion/optimizer)
       Finished test [unoptimized + debuginfo] target(s) in 4.38s
        Running unittests src/lib.rs 
(/Users/alamb/Software/df-target/debug/deps/datafusion_optimizer-ad133d04102e7238)
   
   running 39 tests
   test filter_push_down::tests::filter_no_columns ... ok
   test filter_push_down::tests::filter_before_projection ... ok
   test filter_push_down::tests::alias ... ok
   test filter_push_down::tests::filter_jump_2_plans ... ok
   test filter_push_down::tests::filter_after_limit ... ok
   test filter_push_down::tests::complex_expression ... ok
   test filter_push_down::tests::filter_keep_agg ... ok
   test filter_push_down::tests::filter_move_agg ... ok
   test filter_push_down::tests::complex_plan ... ok
   test filter_push_down::tests::filter_2_breaks_limits ... ok
   test filter_push_down::tests::double_limit ... ok
   test filter_push_down::tests::filter_using_left_join ... ok
   test filter_push_down::tests::filter_on_join_on_common_independent ... ok
   test filter_push_down::tests::filter_join_on_one_side ... ok
   test filter_push_down::tests::filter_join_on_common_dependent ... ok
   test filter_push_down::tests::filter_with_table_provider_exact ... ok
   test filter_push_down::tests::filter_using_join_on_common_independent ... ok
   test filter_push_down::tests::filter_with_table_provider_inexact ... ok
   test filter_push_down::tests::filter_using_left_join_on_common ... ok
   test filter_push_down::tests::filter_with_table_provider_unsupported ... ok
   test filter_push_down::tests::filters_user_defined_node ... ok
   test 
filter_push_down::tests::filter_with_table_provider_multiple_invocations ... ok
   test filter_push_down::tests::filter_using_right_join ... ok
   test filter_push_down::tests::filter_using_right_join_on_common ... ok
   test filter_push_down::tests::full_join_on_with_filter ... ok
   test filter_push_down::tests::multi_combined_filter ... ok
   test filter_push_down::tests::join_filter_on_common ... ok
   test filter_push_down::tests::join_filter_with_alias ... ok
   test filter_push_down::tests::join_filter_removed ... ok
   test filter_push_down::tests::left_join_on_with_filter ... ok
   test filter_push_down::tests::join_on_with_filter ... ok
   test filter_push_down::tests::test_filter_with_alias ... ok
   test filter_push_down::tests::test_filter_with_multi_alias ... ok
   test filter_push_down::tests::two_filters_on_same_depth ... ok
   test filter_push_down::tests::union_all ... ok
   test filter_push_down::tests::union_all_with_alias ... ok
   test filter_push_down::tests::multi_filter ... ok
   test filter_push_down::tests::split_filter ... ok
   test filter_push_down::tests::right_join_on_with_filter ... ok
   
   test result: ok. 39 passed; 0 failed; 0 ignored; 0 measured; 118 filtered 
out; finished in 0.07s
   
      Doc-tests datafusion_optimizer
   
   running 0 tests
   
   test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; 
finished in 0.00s
   
   
   Compilation finished at Tue Jun 14 16:27:56
   ```



-- 
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-unsubscr...@arrow.apache.org

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

Reply via email to