Skip to content

Commit

Permalink
Replace some usages of Expr::to_field with Expr::qualified_name (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jonahgao authored Sep 19, 2024
1 parent b1bdd8d commit 98e5f64
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 53 deletions.
9 changes: 3 additions & 6 deletions datafusion/expr/src/expr_rewriter/order_by.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use crate::expr::Alias;
use crate::expr_rewriter::normalize_col;
use crate::{expr::Sort, Cast, Expr, ExprSchemable, LogicalPlan, TryCast};
use crate::{expr::Sort, Cast, Expr, LogicalPlan, TryCast};

use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode};
use datafusion_common::{Column, Result};
Expand Down Expand Up @@ -77,11 +77,8 @@ fn rewrite_in_terms_of_projection(
expr.transform(|expr| {
// search for unnormalized names first such as "c1" (such as aliases)
if let Some(found) = proj_exprs.iter().find(|a| (**a) == expr) {
let col = Expr::Column(
found
.to_field(input.schema())
.map(|(qualifier, field)| Column::new(qualifier, field.name()))?,
);
let (qualifier, field_name) = found.qualified_name();
let col = Expr::Column(Column::new(qualifier, field_name));
return Ok(Transformed::yes(col));
}

Expand Down
60 changes: 13 additions & 47 deletions datafusion/optimizer/src/common_subexpr_eliminate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use datafusion_expr::logical_plan::{
Aggregate, Filter, LogicalPlan, Projection, Sort, Window,
};
use datafusion_expr::tree_node::replace_sort_expressions;
use datafusion_expr::{col, BinaryExpr, Case, Expr, ExprSchemable, Operator};
use datafusion_expr::{col, BinaryExpr, Case, Expr, Operator};
use indexmap::IndexMap;

const CSE_PREFIX: &str = "__common_expr";
Expand Down Expand Up @@ -533,14 +533,9 @@ impl CommonSubexprEliminate {
.map(|(expr, expr_alias)| expr.alias(expr_alias))
.collect::<Vec<_>>();

let new_input_schema = Arc::clone(new_input.schema());
let mut proj_exprs = vec![];
for expr in &new_group_expr {
extract_expressions(
expr,
&new_input_schema,
&mut proj_exprs,
)?
extract_expressions(expr, &mut proj_exprs)
}
for (expr_rewritten, expr_orig) in
rewritten_aggr_expr.into_iter().zip(new_aggr_expr)
Expand All @@ -555,11 +550,11 @@ impl CommonSubexprEliminate {
} else {
let expr_alias =
config.alias_generator().next(CSE_PREFIX);
let (qualifier, field) =
expr_rewritten.to_field(&new_input_schema)?;
let (qualifier, field_name) =
expr_rewritten.qualified_name();
let out_name = qualified_name(
qualifier.as_ref(),
field.name(),
&field_name,
);

agg_exprs.push(expr_rewritten.alias(&expr_alias));
Expand Down Expand Up @@ -855,24 +850,18 @@ fn build_recover_project_plan(
Projection::try_new(col_exprs, Arc::new(input)).map(LogicalPlan::Projection)
}

fn extract_expressions(
expr: &Expr,
schema: &DFSchema,
result: &mut Vec<Expr>,
) -> Result<()> {
fn extract_expressions(expr: &Expr, result: &mut Vec<Expr>) {
if let Expr::GroupingSet(groupings) = expr {
for e in groupings.distinct_expr() {
let (qualifier, field) = e.to_field(schema)?;
let col = Column::new(qualifier, field.name());
let (qualifier, field_name) = e.qualified_name();
let col = Column::new(qualifier, field_name);
result.push(Expr::Column(col))
}
} else {
let (qualifier, field) = expr.to_field(schema)?;
let col = Column::new(qualifier, field.name());
let (qualifier, field_name) = expr.qualified_name();
let col = Column::new(qualifier, field_name);
result.push(Expr::Column(col));
}

Ok(())
}

/// Which type of [expressions](Expr) should be considered for rewriting?
Expand Down Expand Up @@ -1780,16 +1769,7 @@ mod test {
fn test_extract_expressions_from_grouping_set() -> Result<()> {
let mut result = Vec::with_capacity(3);
let grouping = grouping_set(vec![vec![col("a"), col("b")], vec![col("c")]]);
let schema = DFSchema::from_unqualified_fields(
vec![
Field::new("a", DataType::Int32, false),
Field::new("b", DataType::Int32, false),
Field::new("c", DataType::Int32, false),
]
.into(),
HashMap::default(),
)?;
extract_expressions(&grouping, &schema, &mut result)?;
extract_expressions(&grouping, &mut result);

assert!(result.len() == 3);
Ok(())
Expand All @@ -1799,16 +1779,7 @@ mod test {
fn test_extract_expressions_from_grouping_set_with_identical_expr() -> Result<()> {
let mut result = Vec::with_capacity(2);
let grouping = grouping_set(vec![vec![col("a"), col("b")], vec![col("a")]]);
let schema = DFSchema::from_unqualified_fields(
vec![
Field::new("a", DataType::Int32, false),
Field::new("b", DataType::Int32, false),
]
.into(),
HashMap::default(),
)?;
extract_expressions(&grouping, &schema, &mut result)?;

extract_expressions(&grouping, &mut result);
assert!(result.len() == 2);
Ok(())
}
Expand Down Expand Up @@ -1868,12 +1839,7 @@ mod test {
#[test]
fn test_extract_expressions_from_col() -> Result<()> {
let mut result = Vec::with_capacity(1);
let schema = DFSchema::from_unqualified_fields(
vec![Field::new("a", DataType::Int32, false)].into(),
HashMap::default(),
)?;
extract_expressions(&col("a"), &schema, &mut result)?;

extract_expressions(&col("a"), &mut result);
assert!(result.len() == 1);
Ok(())
}
Expand Down

0 comments on commit 98e5f64

Please sign in to comment.