alamb commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1585495797


##########
datafusion/core/tests/dataframe/dataframe_functions.rs:
##########
@@ -161,6 +161,181 @@ async fn test_fn_btrim_with_chars() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]

Review Comment:
   While adding a test for `coalesce` I realized the other functions in `core` 
weren't covered so I added new tests for them too (except for `get_field` which 
needs a map / struct and I was too lazy to add one to the test fixture



##########
datafusion/functions/src/core/mod.rs:
##########
@@ -39,14 +42,68 @@ make_udf_function!(getfield::GetFieldFunc, GET_FIELD, 
get_field);
 make_udf_function!(coalesce::CoalesceFunc, COALESCE, coalesce);
 
 // Export the functions out of this package, both as expr_fn as well as a list 
of functions
-export_functions!(
-    (nullif, arg_1 arg_2, "returns NULL if value1 equals value2; otherwise it 
returns value1. This can be used to perform the inverse operation of the 
COALESCE expression."),
-    (arrow_cast, arg_1 arg_2, "returns arg_1 cast to the `arrow_type` given 
the second argument. This can be used to cast to a specific `arrow_type`."),
-    (nvl, arg_1 arg_2, "returns value2 if value1 is NULL; otherwise it returns 
value1"),
-    (nvl2, arg_1 arg_2 arg_3, "Returns value2 if value1 is not NULL; 
otherwise, it returns value3."),
-    (arrow_typeof, arg_1, "Returns the Arrow type of the input expression."),
-    (r#struct, args, "Returns a struct with the given arguments"),
-    (named_struct, args, "Returns a struct with the given names and arguments 
pairs"),
-    (get_field, arg_1 arg_2, "Returns the value of the field with the given 
name from the struct"),
-    (coalesce, args, "Returns `coalesce(args...)`, which evaluates to the 
value of the first expr which is not NULL")
-);
+pub mod expr_fn {

Review Comment:
   It is not possible to create an expr_fn that takes a `Vec<Expr>` using the 
`export_functions` macro, so follow the model @Omega359  used in the math 
module 
https://github.com/apache/datafusion/blob/af3950698879a1e159fbe579bd433a0d5ce1c7cb/datafusion/functions/src/math/mod.rs#L78-L270



##########
datafusion/functions/src/core/mod.rs:
##########
@@ -39,14 +42,68 @@ make_udf_function!(getfield::GetFieldFunc, GET_FIELD, 
get_field);
 make_udf_function!(coalesce::CoalesceFunc, COALESCE, coalesce);
 
 // Export the functions out of this package, both as expr_fn as well as a list 
of functions
-export_functions!(
-    (nullif, arg_1 arg_2, "returns NULL if value1 equals value2; otherwise it 
returns value1. This can be used to perform the inverse operation of the 
COALESCE expression."),
-    (arrow_cast, arg_1 arg_2, "returns arg_1 cast to the `arrow_type` given 
the second argument. This can be used to cast to a specific `arrow_type`."),
-    (nvl, arg_1 arg_2, "returns value2 if value1 is NULL; otherwise it returns 
value1"),
-    (nvl2, arg_1 arg_2 arg_3, "Returns value2 if value1 is not NULL; 
otherwise, it returns value3."),
-    (arrow_typeof, arg_1, "Returns the Arrow type of the input expression."),
-    (r#struct, args, "Returns a struct with the given arguments"),
-    (named_struct, args, "Returns a struct with the given names and arguments 
pairs"),
-    (get_field, arg_1 arg_2, "Returns the value of the field with the given 
name from the struct"),
-    (coalesce, args, "Returns `coalesce(args...)`, which evaluates to the 
value of the first expr which is not NULL")
-);
+pub mod expr_fn {
+    use datafusion_expr::Expr;
+
+    /// returns NULL if value1 equals value2; otherwise it returns value1. This
+    /// can be used to perform the inverse operation of the COALESCE expression
+    pub fn nullif(arg1: Expr, arg2: Expr) -> Expr {
+        super::nullif().call(vec![arg1, arg2])
+    }
+
+    /// returns value1 cast to the `arrow_type` given the second argument. This
+    /// can be used to cast to a specific `arrow_type`.
+    pub fn arrow_cast(arg1: Expr, arg2: Expr) -> Expr {
+        super::arrow_cast().call(vec![arg1, arg2])
+    }
+
+    /// Returns value2 if value1 is NULL; otherwise it returns value1
+    pub fn nvl(arg1: Expr, arg2: Expr) -> Expr {
+        super::nvl().call(vec![arg1, arg2])
+    }
+
+    /// Returns value2 if value1 is not NULL; otherwise, it returns value3.
+    pub fn nvl2(arg1: Expr, arg2: Expr, arg3: Expr) -> Expr {
+        super::nvl2().call(vec![arg1, arg2, arg3])
+    }
+
+    /// Returns the Arrow type of the input expression.
+    pub fn arrow_typeof(arg1: Expr) -> Expr {
+        super::arrow_typeof().call(vec![arg1])
+    }
+
+    /// Returns a struct with the given arguments
+    pub fn r#struct(args: Vec<Expr>) -> Expr {
+        super::r#struct().call(args)
+    }
+
+    /// Returns a struct with the given names and arguments pairs
+    pub fn named_struct(args: Vec<Expr>) -> Expr {
+        super::named_struct().call(args)
+    }
+
+    /// Returns the value of the field with the given name from the struct
+    pub fn get_field(arg1: Expr, arg2: Expr) -> Expr {
+        super::get_field().call(vec![arg1, arg2])
+    }
+
+    /// Returns `coalesce(args...)`, which evaluates to the value of the first 
expr which is not NULL
+    pub fn coalesce(args: Vec<Expr>) -> Expr {

Review Comment:
   This is the actual fix, though I suspect that `struct` and `named_struct` 
are also affected



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to