Skip to content

Commit

Permalink
Remove AggregateFunctionDefinition::Name (#10441)
Browse files Browse the repository at this point in the history
  • Loading branch information
lewiszlw authored May 11, 2024
1 parent 76f9e2e commit 6d413a4
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 38 deletions.
8 changes: 0 additions & 8 deletions datafusion/core/src/physical_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
.collect::<Result<Vec<_>>>()?;
Ok(format!("{}({})", fun.name(), names.join(",")))
}
AggregateFunctionDefinition::Name(_) => {
internal_err!("Aggregate function `Expr` with name should be resolved.")
}
},
Expr::GroupingSet(grouping_set) => match grouping_set {
GroupingSet::Rollup(exprs) => Ok(format!(
Expand Down Expand Up @@ -1947,11 +1944,6 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
)?;
(agg_expr, filter, physical_sort_exprs)
}
AggregateFunctionDefinition::Name(_) => {
return internal_err!(
"Aggregate function name should have been resolved"
)
}
};
Ok((agg_expr, filter, order_by))
}
Expand Down
10 changes: 2 additions & 8 deletions datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,6 @@ pub enum AggregateFunctionDefinition {
BuiltIn(aggregate_function::AggregateFunction),
/// Resolved to a user defined aggregate function
UDF(Arc<crate::AggregateUDF>),
/// A aggregation function constructed with name. This variant can not be executed directly
/// and instead must be resolved to one of the other variants prior to physical planning.
Name(Arc<str>),
}

impl AggregateFunctionDefinition {
Expand All @@ -526,7 +523,6 @@ impl AggregateFunctionDefinition {
match self {
AggregateFunctionDefinition::BuiltIn(fun) => fun.name(),
AggregateFunctionDefinition::UDF(udf) => udf.name(),
AggregateFunctionDefinition::Name(func_name) => func_name.as_ref(),
}
}
}
Expand Down Expand Up @@ -1857,8 +1853,7 @@ pub(crate) fn create_name(e: &Expr) -> Result<String> {
null_treatment,
}) => {
let name = match func_def {
AggregateFunctionDefinition::BuiltIn(..)
| AggregateFunctionDefinition::Name(..) => {
AggregateFunctionDefinition::BuiltIn(..) => {
create_function_name(func_def.name(), *distinct, args)?
}
AggregateFunctionDefinition::UDF(..) => {
Expand All @@ -1878,8 +1873,7 @@ pub(crate) fn create_name(e: &Expr) -> Result<String> {
info += &format!(" {}", nt);
}
match func_def {
AggregateFunctionDefinition::BuiltIn(..)
| AggregateFunctionDefinition::Name(..) => {
AggregateFunctionDefinition::BuiltIn(..) => {
Ok(format!("{}{}", name, info))
}
AggregateFunctionDefinition::UDF(fun) => {
Expand Down
3 changes: 0 additions & 3 deletions datafusion/expr/src/expr_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,6 @@ impl ExprSchemable for Expr {
AggregateFunctionDefinition::UDF(fun) => {
Ok(fun.return_type(&data_types)?)
}
AggregateFunctionDefinition::Name(_) => {
internal_err!("Function `Expr` with name should be resolved.")
}
}
}
Expr::Not(_)
Expand Down
5 changes: 1 addition & 4 deletions datafusion/expr/src/tree_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{Expr, GetFieldAccess};
use datafusion_common::tree_node::{
Transformed, TreeNode, TreeNodeIterator, TreeNodeRecursion,
};
use datafusion_common::{internal_err, map_until_stop_and_collect, Result};
use datafusion_common::{map_until_stop_and_collect, Result};

impl TreeNode for Expr {
fn apply_children<F: FnMut(&Self) -> Result<TreeNodeRecursion>>(
Expand Down Expand Up @@ -348,9 +348,6 @@ impl TreeNode for Expr {
null_treatment,
)))
}
AggregateFunctionDefinition::Name(_) => {
internal_err!("Function `Expr` with name should be resolved.")
}
},
)?,
Expr::GroupingSet(grouping_set) => match grouping_set {
Expand Down
3 changes: 0 additions & 3 deletions datafusion/optimizer/src/analyzer/type_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,6 @@ impl<'a> TreeNodeRewriter for TypeCoercionRewriter<'a> {
),
)))
}
AggregateFunctionDefinition::Name(_) => {
internal_err!("Function `Expr` with name should be resolved.")
}
},
Expr::WindowFunction(WindowFunction {
fun,
Expand Down
3 changes: 0 additions & 3 deletions datafusion/optimizer/src/decorrelate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,6 @@ fn agg_exprs_evaluation_result_on_empty_batch(
AggregateFunctionDefinition::UDF { .. } => {
Transformed::yes(Expr::Literal(ScalarValue::Null))
}
AggregateFunctionDefinition::Name(_) => {
Transformed::yes(Expr::Literal(ScalarValue::Null))
}
},
_ => Transformed::no(expr),
};
Expand Down
6 changes: 0 additions & 6 deletions datafusion/proto/src/logical_plan/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,12 +746,6 @@ pub fn serialize_expr(
},
))),
},
AggregateFunctionDefinition::Name(_) => {
return Err(Error::NotImplemented(
"Proto serialization error: Trying to serialize a unresolved function"
.to_string(),
));
}
},

Expr::ScalarVariable(_, _) => {
Expand Down
3 changes: 0 additions & 3 deletions datafusion/substrait/src/logical_plan/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,9 +733,6 @@ pub fn to_substrait_agg_measure(
}
})
}
AggregateFunctionDefinition::Name(name) => {
internal_err!("AggregateFunctionDefinition::Name({:?}) should be resolved during `AnalyzerRule`", name)
}
}

}
Expand Down

0 comments on commit 6d413a4

Please sign in to comment.