This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 513fd052bd Minor: Remove uncessary name field in
ScalarFunctionDefintion (#8365)
513fd052bd is described below
commit 513fd052bdbf5c7a73de544a876961f780b90a92
Author: Andrew Lamb <[email protected]>
AuthorDate: Thu Nov 30 17:11:19 2023 -0500
Minor: Remove uncessary name field in ScalarFunctionDefintion (#8365)
---
datafusion/core/src/datasource/listing/helpers.rs | 2 +-
datafusion/expr/src/built_in_function.rs | 9 ++++++--
datafusion/expr/src/expr.rs | 14 ++++-------
datafusion/expr/src/expr_fn.rs | 14 +++++------
datafusion/expr/src/expr_schema.rs | 2 +-
datafusion/expr/src/tree_node/expr.rs | 2 +-
datafusion/optimizer/src/analyzer/type_coercion.rs | 2 +-
datafusion/optimizer/src/optimize_projections.rs | 4 +---
datafusion/optimizer/src/push_down_filter.rs | 2 +-
.../src/simplify_expressions/expr_simplifier.rs | 27 ++++++----------------
.../optimizer/src/simplify_expressions/utils.rs | 12 ++--------
datafusion/physical-expr/src/planner.rs | 2 +-
datafusion/proto/src/logical_plan/to_proto.rs | 2 +-
datafusion/sql/src/expr/value.rs | 2 +-
14 files changed, 36 insertions(+), 60 deletions(-)
diff --git a/datafusion/core/src/datasource/listing/helpers.rs
b/datafusion/core/src/datasource/listing/helpers.rs
index 0c39877cd1..a4505cf62d 100644
--- a/datafusion/core/src/datasource/listing/helpers.rs
+++ b/datafusion/core/src/datasource/listing/helpers.rs
@@ -92,7 +92,7 @@ pub fn expr_applicable_for_cols(col_names: &[String], expr:
&Expr) -> bool {
Expr::ScalarFunction(scalar_function) => {
match &scalar_function.func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
match fun.volatility() {
Volatility::Immutable =>
Ok(VisitRecursion::Continue),
// TODO: Stable functions could be `applicable`,
but that would require access to the context
diff --git a/datafusion/expr/src/built_in_function.rs
b/datafusion/expr/src/built_in_function.rs
index 2f67783201..a51941fdee 100644
--- a/datafusion/expr/src/built_in_function.rs
+++ b/datafusion/expr/src/built_in_function.rs
@@ -348,6 +348,12 @@ impl BuiltinScalarFunction {
self.signature().type_signature.supports_zero_argument()
}
+ /// Returns the name of this function
+ pub fn name(&self) -> &str {
+ // .unwrap is safe here because compiler makes sure the map will have
matches for each BuiltinScalarFunction
+ function_to_name().get(self).unwrap()
+ }
+
/// Returns the [Volatility] of the builtin function.
pub fn volatility(&self) -> Volatility {
match self {
@@ -1627,8 +1633,7 @@ impl BuiltinScalarFunction {
impl fmt::Display for BuiltinScalarFunction {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- // .unwrap is safe here because compiler makes sure the map will have
matches for each BuiltinScalarFunction
- write!(f, "{}", function_to_name().get(self).unwrap())
+ write!(f, "{}", self.name())
}
}
diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs
index 256f5b210e..ee9b0ad6f9 100644
--- a/datafusion/expr/src/expr.rs
+++ b/datafusion/expr/src/expr.rs
@@ -17,7 +17,6 @@
//! Expr module contains core type definition for `Expr`.
-use crate::built_in_function;
use crate::expr_fn::binary_expr;
use crate::logical_plan::Subquery;
use crate::udaf;
@@ -26,6 +25,7 @@ use crate::window_frame;
use crate::window_function;
use crate::Operator;
use crate::{aggregate_function, ExprSchemable};
+use crate::{built_in_function, BuiltinScalarFunction};
use arrow::datatypes::DataType;
use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_common::{internal_err, DFSchema, OwnedTableReference};
@@ -340,10 +340,7 @@ 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 {
- fun: built_in_function::BuiltinScalarFunction,
- name: Arc<str>,
- },
+ 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
@@ -371,7 +368,7 @@ impl ScalarFunctionDefinition {
/// Function's name for display
pub fn name(&self) -> &str {
match self {
- ScalarFunctionDefinition::BuiltIn { name, .. } => name.as_ref(),
+ ScalarFunctionDefinition::BuiltIn(fun) => fun.name(),
ScalarFunctionDefinition::UDF(udf) => udf.name(),
ScalarFunctionDefinition::Name(func_name) => func_name.as_ref(),
}
@@ -382,10 +379,7 @@ 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,
- name: Arc::from(fun.to_string()),
- },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}
}
diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs
index 1f4ab7bb4a..6148226f6b 100644
--- a/datafusion/expr/src/expr_fn.rs
+++ b/datafusion/expr/src/expr_fn.rs
@@ -1032,7 +1032,7 @@ mod test {
macro_rules! test_unary_scalar_expr {
($ENUM:ident, $FUNC:ident) => {{
if let Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}) = $FUNC(col("tableA.a"))
{
@@ -1053,7 +1053,7 @@ mod test {
col(stringify!($arg.to_string()))
),*
);
- if let Expr::ScalarFunction(ScalarFunction { func_def:
ScalarFunctionDefinition::BuiltIn{fun, ..}, args }) = result {
+ if let Expr::ScalarFunction(ScalarFunction { func_def:
ScalarFunctionDefinition::BuiltIn(fun), args }) = result {
let name = built_in_function::BuiltinScalarFunction::$ENUM;
assert_eq!(name, fun);
assert_eq!(expected.len(), args.len());
@@ -1073,7 +1073,7 @@ mod test {
),*
]
);
- if let Expr::ScalarFunction(ScalarFunction { func_def:
ScalarFunctionDefinition::BuiltIn{fun, ..}, args }) = result {
+ if let Expr::ScalarFunction(ScalarFunction { func_def:
ScalarFunctionDefinition::BuiltIn(fun), args }) = result {
let name = built_in_function::BuiltinScalarFunction::$ENUM;
assert_eq!(name, fun);
assert_eq!(expected.len(), args.len());
@@ -1214,7 +1214,7 @@ mod test {
#[test]
fn uuid_function_definitions() {
if let Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}) = uuid()
{
@@ -1229,7 +1229,7 @@ mod test {
#[test]
fn digest_function_definitions() {
if let Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}) = digest(col("tableA.a"), lit("md5"))
{
@@ -1244,7 +1244,7 @@ mod test {
#[test]
fn encode_function_definitions() {
if let Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}) = encode(col("tableA.a"), lit("base64"))
{
@@ -1259,7 +1259,7 @@ mod test {
#[test]
fn decode_function_definitions() {
if let Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
args,
}) = decode(col("tableA.a"), lit("hex"))
{
diff --git a/datafusion/expr/src/expr_schema.rs
b/datafusion/expr/src/expr_schema.rs
index 99b27e8912..2795ac5f09 100644
--- a/datafusion/expr/src/expr_schema.rs
+++ b/datafusion/expr/src/expr_schema.rs
@@ -84,7 +84,7 @@ impl ExprSchemable for Expr {
| Expr::TryCast(TryCast { data_type, .. }) =>
Ok(data_type.clone()),
Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
match func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
let arg_data_types = args
.iter()
.map(|e| e.get_type(schema))
diff --git a/datafusion/expr/src/tree_node/expr.rs
b/datafusion/expr/src/tree_node/expr.rs
index fcb0a4cd93..1098842716 100644
--- a/datafusion/expr/src/tree_node/expr.rs
+++ b/datafusion/expr/src/tree_node/expr.rs
@@ -277,7 +277,7 @@ impl TreeNode for Expr {
nulls_first,
)),
Expr::ScalarFunction(ScalarFunction { func_def, args }) => match
func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } =>
Expr::ScalarFunction(
+ ScalarFunctionDefinition::BuiltIn(fun) => Expr::ScalarFunction(
ScalarFunction::new(fun, transform_vec(args, &mut
transform)?),
),
ScalarFunctionDefinition::UDF(fun) => Expr::ScalarFunction(
diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs
b/datafusion/optimizer/src/analyzer/type_coercion.rs
index bedc86e2f4..e3b86f5db7 100644
--- a/datafusion/optimizer/src/analyzer/type_coercion.rs
+++ b/datafusion/optimizer/src/analyzer/type_coercion.rs
@@ -320,7 +320,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
Ok(Expr::Case(case))
}
Expr::ScalarFunction(ScalarFunction { func_def, args }) => match
func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
let new_args = coerce_arguments_for_signature(
args.as_slice(),
&self.schema,
diff --git a/datafusion/optimizer/src/optimize_projections.rs
b/datafusion/optimizer/src/optimize_projections.rs
index b6d026279a..bbf704a83c 100644
--- a/datafusion/optimizer/src/optimize_projections.rs
+++ b/datafusion/optimizer/src/optimize_projections.rs
@@ -510,9 +510,7 @@ fn rewrite_expr(expr: &Expr, input: &Projection) ->
Result<Option<Expr>> {
)))
}
Expr::ScalarFunction(scalar_fn) => {
- let fun = if let ScalarFunctionDefinition::BuiltIn { fun, .. } =
- scalar_fn.func_def
- {
+ let fun = if let ScalarFunctionDefinition::BuiltIn(fun) =
scalar_fn.func_def {
fun
} else {
return Ok(None);
diff --git a/datafusion/optimizer/src/push_down_filter.rs
b/datafusion/optimizer/src/push_down_filter.rs
index bad6e24715..e8f116d894 100644
--- a/datafusion/optimizer/src/push_down_filter.rs
+++ b/datafusion/optimizer/src/push_down_filter.rs
@@ -979,7 +979,7 @@ fn is_volatile_expression(e: &Expr) -> bool {
e.apply(&mut |expr| {
Ok(match expr {
Expr::ScalarFunction(f) => match &f.func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. }
+ ScalarFunctionDefinition::BuiltIn(fun)
if fun.volatility() == Volatility::Volatile =>
{
is_volatile = true;
diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
index c7366e1761..41c71c9d9a 100644
--- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
+++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
@@ -344,7 +344,7 @@ impl<'a> ConstEvaluator<'a> {
| Expr::Wildcard { .. }
| Expr::Placeholder(_) => false,
Expr::ScalarFunction(ScalarFunction { func_def, .. }) => match
func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
Self::volatility_ok(fun.volatility())
}
ScalarFunctionDefinition::UDF(fun) => {
@@ -1202,41 +1202,28 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for
Simplifier<'a, S> {
// log
Expr::ScalarFunction(ScalarFunction {
- func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::Log,
- ..
- },
+ func_def:
ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Log),
args,
}) => simpl_log(args, <&S>::clone(&info))?,
// power
Expr::ScalarFunction(ScalarFunction {
- func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::Power,
- ..
- },
+ func_def:
ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Power),
args,
}) => simpl_power(args, <&S>::clone(&info))?,
// concat
Expr::ScalarFunction(ScalarFunction {
- func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::Concat,
- ..
- },
+ func_def:
ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Concat),
args,
}) => simpl_concat(args)?,
// concat_ws
Expr::ScalarFunction(ScalarFunction {
func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::ConcatWithSeparator,
- ..
- },
+ ScalarFunctionDefinition::BuiltIn(
+ BuiltinScalarFunction::ConcatWithSeparator,
+ ),
args,
}) => match &args[..] {
[delimiter, vals @ ..] => simpl_concat_ws(delimiter, vals)?,
diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs
b/datafusion/optimizer/src/simplify_expressions/utils.rs
index e69207b688..fa91a3ace2 100644
--- a/datafusion/optimizer/src/simplify_expressions/utils.rs
+++ b/datafusion/optimizer/src/simplify_expressions/utils.rs
@@ -365,11 +365,7 @@ pub fn simpl_log(current_args: Vec<Expr>, info: &dyn
SimplifyInfo) -> Result<Exp
)?))
}
Expr::ScalarFunction(ScalarFunction {
- func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::Power,
- ..
- },
+ func_def:
ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Power),
args,
}) if base == &args[0] => Ok(args[1].clone()),
_ => {
@@ -409,11 +405,7 @@ pub fn simpl_power(current_args: Vec<Expr>, info: &dyn
SimplifyInfo) -> Result<E
Ok(base.clone())
}
Expr::ScalarFunction(ScalarFunction {
- func_def:
- ScalarFunctionDefinition::BuiltIn {
- fun: BuiltinScalarFunction::Log,
- ..
- },
+ func_def:
ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Log),
args,
}) if base == &args[0] => Ok(args[1].clone()),
_ => Ok(Expr::ScalarFunction(ScalarFunction::new(
diff --git a/datafusion/physical-expr/src/planner.rs
b/datafusion/physical-expr/src/planner.rs
index 5c5cc8e36f..5501647da2 100644
--- a/datafusion/physical-expr/src/planner.rs
+++ b/datafusion/physical-expr/src/planner.rs
@@ -349,7 +349,7 @@ pub fn create_physical_expr(
}
Expr::ScalarFunction(ScalarFunction { func_def, args }) => match
func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
let physical_args = args
.iter()
.map(|e| {
diff --git a/datafusion/proto/src/logical_plan/to_proto.rs
b/datafusion/proto/src/logical_plan/to_proto.rs
index b619339674..ab8e850014 100644
--- a/datafusion/proto/src/logical_plan/to_proto.rs
+++ b/datafusion/proto/src/logical_plan/to_proto.rs
@@ -793,7 +793,7 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode {
))
}
Expr::ScalarFunction(ScalarFunction { func_def, args }) => match
func_def {
- ScalarFunctionDefinition::BuiltIn { fun, .. } => {
+ ScalarFunctionDefinition::BuiltIn(fun) => {
let fun: protobuf::ScalarFunction = fun.try_into()?;
let args: Vec<Self> = args
.iter()
diff --git a/datafusion/sql/src/expr/value.rs b/datafusion/sql/src/expr/value.rs
index f33e9e8ddf..a3f29da488 100644
--- a/datafusion/sql/src/expr/value.rs
+++ b/datafusion/sql/src/expr/value.rs
@@ -144,7 +144,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
values.push(value);
}
Expr::ScalarFunction(ScalarFunction {
- func_def: ScalarFunctionDefinition::BuiltIn { fun, .. },
+ func_def: ScalarFunctionDefinition::BuiltIn(fun),
..
}) => {
if fun == BuiltinScalarFunction::MakeArray {
