Skip to content

Commit

Permalink
Move coalesce to datafusion-functions and remove BuiltInScalarFunction
Browse files Browse the repository at this point in the history
  • Loading branch information
Omega359 committed Apr 16, 2024
1 parent 0573f78 commit e298c1c
Show file tree
Hide file tree
Showing 27 changed files with 241 additions and 631 deletions.
10 changes: 0 additions & 10 deletions datafusion/core/src/datasource/listing/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,6 @@ pub fn expr_applicable_for_cols(col_names: &[String], expr: &Expr) -> bool {

Expr::ScalarFunction(scalar_function) => {
match &scalar_function.func_def {
ScalarFunctionDefinition::BuiltIn(fun) => {
match fun.volatility() {
Volatility::Immutable => Ok(TreeNodeRecursion::Continue),
// TODO: Stable functions could be `applicable`, but that would require access to the context
Volatility::Stable | Volatility::Volatile => {
is_applicable = false;
Ok(TreeNodeRecursion::Stop)
}
}
}
ScalarFunctionDefinition::UDF(fun) => {
match fun.signature().volatility {
Volatility::Immutable => Ok(TreeNodeRecursion::Continue),
Expand Down
207 changes: 0 additions & 207 deletions datafusion/expr/src/built_in_function.rs

This file was deleted.

22 changes: 3 additions & 19 deletions datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use crate::logical_plan::Subquery;
use crate::utils::expr_to_columns;
use crate::window_frame;
use crate::{
aggregate_function, built_in_function, built_in_window_function, udaf,
BuiltinScalarFunction, ExprSchemable, Operator, Signature,
aggregate_function, built_in_window_function, udaf, ExprSchemable, Operator,
Signature,
};

use arrow::datatypes::DataType;
Expand Down Expand Up @@ -356,10 +356,6 @@ impl Between {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
/// Defines which implementation of a function for DataFusion to call.
pub enum ScalarFunctionDefinition {
/// Resolved to a `BuiltinScalarFunction`
/// There is plan to migrate `BuiltinScalarFunction` to UDF-based implementation (issue#8045)
/// This variant is planned to be removed in long term
BuiltIn(BuiltinScalarFunction),
/// Resolved to a user defined function
UDF(Arc<crate::ScalarUDF>),
/// A scalar function constructed with name. This variant can not be executed directly
Expand Down Expand Up @@ -387,7 +383,6 @@ impl ScalarFunctionDefinition {
/// Function's name for display
pub fn name(&self) -> &str {
match self {
ScalarFunctionDefinition::BuiltIn(fun) => fun.name(),
ScalarFunctionDefinition::UDF(udf) => udf.name(),
ScalarFunctionDefinition::Name(func_name) => func_name.as_ref(),
}
Expand All @@ -397,9 +392,6 @@ impl ScalarFunctionDefinition {
/// when evaluated multiple times with the same input.
pub fn is_volatile(&self) -> Result<bool> {
match self {
ScalarFunctionDefinition::BuiltIn(fun) => {
Ok(fun.volatility() == crate::Volatility::Volatile)
}
ScalarFunctionDefinition::UDF(udf) => {
Ok(udf.signature().volatility == crate::Volatility::Volatile)
}
Expand All @@ -413,14 +405,6 @@ impl ScalarFunctionDefinition {
}

impl ScalarFunction {
/// Create a new ScalarFunction expression
pub fn new(fun: built_in_function::BuiltinScalarFunction, args: Vec<Expr>) -> Self {
Self {
func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}
}

/// Create a new ScalarFunction expression with a user-defined function (UDF)
pub fn new_udf(udf: Arc<crate::ScalarUDF>, args: Vec<Expr>) -> Self {
Self {
Expand Down Expand Up @@ -1276,7 +1260,7 @@ impl Expr {
pub fn short_circuits(&self) -> bool {
match self {
Expr::ScalarFunction(ScalarFunction { func_def, .. }) => {
matches!(func_def, ScalarFunctionDefinition::BuiltIn(fun) if *fun == BuiltinScalarFunction::Coalesce)
matches!(func_def, ScalarFunctionDefinition::UDF(fun) if fun.name().eq("coalesce"))
}
Expr::BinaryExpr(BinaryExpr { op, .. }) => {
matches!(op, Operator::And | Operator::Or)
Expand Down
25 changes: 4 additions & 21 deletions datafusion/expr/src/expr_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
use crate::expr::{
AggregateFunction, BinaryExpr, Cast, Exists, GroupingSet, InList, InSubquery,
Placeholder, ScalarFunction, TryCast,
Placeholder, TryCast,
};
use crate::function::{
AccumulatorArgs, AccumulatorFactoryFunction, PartitionEvaluatorFactory,
};
use crate::{
aggregate_function, built_in_function, conditional_expressions::CaseBuilder,
logical_plan::Subquery, AggregateUDF, Expr, LogicalPlan, Operator,
ScalarFunctionImplementation, ScalarUDF, Signature, Volatility,
aggregate_function, conditional_expressions::CaseBuilder, logical_plan::Subquery,
AggregateUDF, Expr, LogicalPlan, Operator, ScalarFunctionImplementation, ScalarUDF,
Signature, Volatility,
};
use crate::{AggregateUDFImpl, ColumnarValue, ScalarUDFImpl, WindowUDF, WindowUDFImpl};
use arrow::datatypes::{DataType, Field};
Expand Down Expand Up @@ -478,23 +478,6 @@ pub fn is_not_unknown(expr: Expr) -> Expr {
Expr::IsNotUnknown(Box::new(expr))
}

macro_rules! nary_scalar_expr {
($ENUM:ident, $FUNC:ident, $DOC:expr) => {
#[doc = $DOC ]
pub fn $FUNC(args: Vec<Expr>) -> Expr {
Expr::ScalarFunction(ScalarFunction::new(
built_in_function::BuiltinScalarFunction::$ENUM,
args,
))
}
};
}

// generate methods for creating the supported unary/binary expressions

// math functions
nary_scalar_expr!(Coalesce, coalesce, "returns `coalesce(args...)`, which evaluates to the value of the first [Expr] which is not NULL");

/// Create a CASE WHEN statement with literal WHEN expressions for comparison to the base expression.
pub fn case(expr: Expr) -> CaseBuilder {
CaseBuilder::new(Some(Box::new(expr)), vec![], vec![], None)
Expand Down
17 changes: 0 additions & 17 deletions datafusion/expr/src/expr_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,23 +139,6 @@ impl ExprSchemable for Expr {
.map(|e| e.get_type(schema))
.collect::<Result<Vec<_>>>()?;
match func_def {
ScalarFunctionDefinition::BuiltIn(fun) => {
// verify that function is invoked with correct number and type of arguments as defined in `TypeSignature`
data_types(&arg_data_types, &fun.signature()).map_err(|_| {
plan_datafusion_err!(
"{}",
utils::generate_signature_error_msg(
&format!("{fun}"),
fun.signature(),
&arg_data_types,
)
)
})?;

// perform additional function arguments validation (due to limited
// expressiveness of `TypeSignature`), then infer return type
fun.return_type(&arg_data_types)
}
ScalarFunctionDefinition::UDF(fun) => {
// verify that function is invoked with correct number and type of arguments as defined in `TypeSignature`
data_types(&arg_data_types, fun.signature()).map_err(|_| {
Expand Down
2 changes: 0 additions & 2 deletions datafusion/expr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
//! The [expr_fn] module contains functions for creating expressions.
mod accumulator;
mod built_in_function;
mod built_in_window_function;
mod columnar_value;
mod literal;
Expand Down Expand Up @@ -60,7 +59,6 @@ pub mod window_state;

pub use accumulator::Accumulator;
pub use aggregate_function::AggregateFunction;
pub use built_in_function::BuiltinScalarFunction;
pub use built_in_window_function::BuiltInWindowFunction;
pub use columnar_value::ColumnarValue;
pub use expr::{
Expand Down
3 changes: 0 additions & 3 deletions datafusion/expr/src/tree_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,6 @@ impl TreeNode for Expr {
.update_data(|be| Expr::Sort(Sort::new(be, asc, nulls_first))),
Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
transform_vec(args, &mut f)?.map_data(|new_args| match func_def {
ScalarFunctionDefinition::BuiltIn(fun) => {
Ok(Expr::ScalarFunction(ScalarFunction::new(fun, new_args)))
}
ScalarFunctionDefinition::UDF(fun) => {
Ok(Expr::ScalarFunction(ScalarFunction::new_udf(fun, new_args)))
}
Expand Down
Loading

0 comments on commit e298c1c

Please sign in to comment.