You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/04/30 20:33:34 UTC

[PR] Fix coalesce expr_fn function to take multiple arguments [datafusion]

alamb opened a new pull request, #10321:
URL: https://github.com/apache/datafusion/pull/10321

   ## Which issue does this PR close?
   
   Closes https://github.com/apache/datafusion/issues/10320
   
   ## Rationale for this change
   
   There was a regression in the `coalesce` function's signature due to lack of testing
   
   ## What changes are included in this PR?
   
   1. Fix signature
   2. Add test coverage
   
   ## Are these changes tested?
   Yes, by existing CI and new tests
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   3. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   -->
   
   ## Are there any user-facing changes?
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix coalesce expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1587802909


##########
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:
   The difference is that in this case there is a single macro call to `export_functions!` that has a single (variadic) argument
   
   In the array cases there are multiple invocations of the macro, and the different invocation has different parameters



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1585700474


##########
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:
   I think it is possible to take `Vec` like what functions-array macro does
   
   ```rust
   ($UDF:ty, $EXPR_FN:ident, $DOC:expr , $SCALAR_UDF_FN:ident) => {
           paste::paste! {
               // "fluent expr_fn" style function
               #[doc = $DOC]
               pub fn $EXPR_FN(arg: Vec<Expr>) -> Expr {
                   Expr::ScalarFunction(ScalarFunction::new_udf(
                       $SCALAR_UDF_FN(),
                       arg,
                   ))
               }
   ```



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1588855295


##########
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:
   https://users.rust-lang.org/t/macro-repetition-with-multiple-rules/110816/2?u=jayzhan
   
   I got a probably better solution (I had not tried it) from rust forum. To anyone that is interested in



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1591681406


##########
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:
   I think we can merge this first and release 38. 



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10321:
URL: https://github.com/apache/datafusion/pull/10321


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1586856895


##########
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:
   What I could't figure out how to do was make the macro take both syntaxes
   
   It would have to look something like 
   
   ```rust
   export_functions!(
       // create a function with arg_1 and arg_2 Expr arguments
       (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`."),
      /// create a function with a single  Vec<Expr> arg
       (coalesce, args, "Returns `coalesce(args...)`, which evaluates to the value of the first expr which is not NULL")
   ...
   }
   ```
   
   the macro definition is https://github.com/apache/datafusion/blob/7c1c7941a078153173a6c82bf2c8742bbcbdefa8/datafusion/functions/src/macros.rs#L39-L60
   
   there may be some way to do this in rust, but I could't figure it out



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1588855295


##########
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:
   https://users.rust-lang.org/t/macro-repetition-with-multiple-rules/110816/2?u=jayzhan
   
   I got a slight better solution from rust forum. To anyone that is interested in



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1591477380


##########
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:
   I couldn't make this work (though I didn't try very hard) so I filed https://github.com/apache/datafusion/issues/10397 as a follow on task for this



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1586925126


##########
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:
   https://github.com/apache/datafusion/blob/e3487eee1365a37b8651bf5a393ae8d2cc16e653/datafusion/functions-array/src/macros.rs#L47-L109
   
   In array macro, we support two syntaxes, which is surprisingly easy



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#issuecomment-2097148278

   Thanks @jayzhan211  


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1589968630


##########
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:
   Thank you @jayzhan211  -- I will give it a try



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1588554179


##########
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:
   I was thinking of https://github.com/alamb/datafusion/pull/19.
   
   However, it does not reuse the macro `export_functions` that accepts multiple functions with one macro call.
   
   `export_functions_single` accepts a single function with each macro call.
   
   Since this requires introducing another macro, it is also not an ideal solution.
   
   I'm fine to move on with the current PR.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] Fix `coalesce`, `struct` and `named_strct` expr_fn function to take multiple arguments [datafusion]

Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #10321:
URL: https://github.com/apache/datafusion/pull/10321#discussion_r1586921438


##########
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:
   I spent a whole day trying to make it work as well and got lost in the depths of macro programming. I'm not sure it's worth the effort of trying to get it to work tbh - the alternative is not exactly hard work.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org