Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor: Use Expr::apply() instead of inspect_expr_pre() #9984

Merged
merged 1 commit into from
Apr 7, 2024
Merged

Minor: Use Expr::apply() instead of inspect_expr_pre() #9984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions datafusion/expr/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub fn grouping_set_to_exprlist(group_expr: &[Expr]) -> Result<Vec<Expr>> {
/// Recursively walk an expression tree, collecting the unique set of columns
/// referenced in the expression
pub fn expr_to_columns(expr: &Expr, accum: &mut HashSet<Column>) -> Result<()> {
inspect_expr_pre(expr, |expr| {
expr.apply(&mut |expr| {
match expr {
Expr::Column(qc) => {
accum.insert(qc.clone());
Expand Down Expand Up @@ -307,8 +307,9 @@ pub fn expr_to_columns(expr: &Expr, accum: &mut HashSet<Column>) -> Result<()> {
| Expr::Placeholder(_)
| Expr::OuterReferenceColumn { .. } => {}
}
Ok(())
Ok(TreeNodeRecursion::Continue)
})
.map(|_| ())
}

/// Find excluded columns in the schema, if any
Expand Down Expand Up @@ -838,11 +839,11 @@ pub fn find_column_exprs(exprs: &[Expr]) -> Vec<Expr> {

pub(crate) fn find_columns_referenced_by_expr(e: &Expr) -> Vec<Column> {
let mut exprs = vec![];
inspect_expr_pre(e, |expr| {
e.apply(&mut |expr| {
if let Expr::Column(c) = expr {
exprs.push(c.clone())
}
Ok(()) as Result<()>
Ok(TreeNodeRecursion::Continue)
})
// As the closure always returns Ok, this "can't" error
.expect("Unexpected error");
Expand All @@ -867,7 +868,7 @@ pub(crate) fn find_column_indexes_referenced_by_expr(
schema: &DFSchemaRef,
) -> Vec<usize> {
let mut indexes = vec![];
inspect_expr_pre(e, |expr| {
e.apply(&mut |expr| {
match expr {
Expr::Column(qc) => {
if let Ok(idx) = schema.index_of_column(qc) {
Expand All @@ -879,7 +880,7 @@ pub(crate) fn find_column_indexes_referenced_by_expr(
}
_ => {}
}
Ok(()) as Result<()>
Ok(TreeNodeRecursion::Continue)
})
.unwrap();
indexes
Expand Down
6 changes: 3 additions & 3 deletions datafusion/optimizer/src/optimize_projections.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use datafusion_expr::{
Expr, Projection, TableScan, Window,
};

use datafusion_expr::utils::inspect_expr_pre;
use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion};
use hashbrown::HashMap;
use itertools::{izip, Itertools};

Expand Down Expand Up @@ -613,7 +613,7 @@ fn rewrite_expr(expr: &Expr, input: &Projection) -> Result<Option<Expr>> {
/// columns are collected.
fn outer_columns(expr: &Expr, columns: &mut HashSet<Column>) {
// inspect_expr_pre doesn't handle subquery references, so find them explicitly
inspect_expr_pre(expr, |expr| {
expr.apply(&mut |expr| {
match expr {
Expr::OuterReferenceColumn(_, col) => {
columns.insert(col.clone());
Expand All @@ -632,7 +632,7 @@ fn outer_columns(expr: &Expr, columns: &mut HashSet<Column>) {
}
_ => {}
};
Ok(()) as Result<()>
Ok(TreeNodeRecursion::Continue)
})
// unwrap: closure above never returns Err, so can not be Err here
.unwrap();
Expand Down
Loading