You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org> on 2024/05/28 16:32:45 UTC
[PR] Reduce repetition in math and functions modules with macros [datafusion]
MohamedAbdeen21 opened a new pull request, #10700:
URL: https://github.com/apache/datafusion/pull/10700
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Possibly closes #10397.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
Extending the `export_function!` macro to be used in math and functions modules, reducing code repetition.
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Extending the `export_function!` macro to accept both variadic functions and functions that take single `Vec<Expr>` argument.
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. 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)?
-->
Passes 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.
-->
No
<!--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1618201733
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
Thought about that, the only problem is how can we split the definition of `pub fn functions()` between two different macros, without pulling it into it's own macro?
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1621195045
##########
datafusion/functions/src/macros.rs:
##########
@@ -36,25 +36,31 @@
/// ]
/// }
/// ```
+///
+/// Exported functions accept:
+/// - `Vec<Expr>` argument (single argument followed by a comma)
+/// - Variable number of `Expr` arguments (zero or more arguments, must be without commas)
macro_rules! export_functions {
- ($(($FUNC:ident, $($arg:ident)*, $DOC:expr)),*) => {
- pub mod expr_fn {
- $(
- #[doc = $DOC]
- /// Return $name(arg)
- pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
- super::$FUNC().call(vec![$($arg),*],)
- }
- )*
+ ($(($FUNC:ident, $DOC:expr, $($arg:tt)*)),*) => {
+ $(
+ // switch to single-function cases below
+ export_functions!(single $FUNC, $DOC, $($arg)*);
Review Comment:
A little hack to avoid making a separate macro.
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1618187445
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
I don't agree to rewrite the second argument to Expr.
I think we can remove it out of macro and define it specially given it's specialty
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
I don't agree to rewrite the second argument to Expr.
I think we can remove it out of macro and define it specifically given it's specialty
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1617618779
##########
datafusion/functions/src/core/mod.rs:
##########
@@ -41,70 +38,40 @@ make_udf_function!(r#struct::StructFunc, STRUCT, r#struct);
make_udf_function!(named_struct::NamedStructFunc, NAMED_STRUCT, named_struct);
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
-pub mod expr_fn {
- use datafusion_expr::{Expr, Literal};
-
- /// 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, field_name: impl Literal) -> Expr {
- super::get_field().call(vec![arg1, field_name.lit()])
- }
-
- /// Returns `coalesce(args...)`, which evaluates to the value of the first expr which is not NULL
- pub fn coalesce(args: Vec<Expr>) -> Expr {
- super::coalesce().call(args)
- }
-}
-
-/// Return a list of all functions in this package
-pub fn functions() -> Vec<Arc<ScalarUDF>> {
- vec![
- nullif(),
- arrow_cast(),
- nvl(),
- nvl2(),
- arrow_typeof(),
- r#struct(),
- named_struct(),
- get_field(),
- coalesce(),
- ]
-}
+export_functions!((
+ nullif,
+ "Returns NULL if value1 equals value2; otherwise it returns value1. This can be used to perform the inverse operation of the COALESCE expression",
+ arg1 arg2
+),(
+ arrow_cast,
+ "Returns value2 if value1 is NULL; otherwise it returns value1",
+ arg1 arg2
+),(
+ nvl,
+ "Returns value2 if value1 is NULL; otherwise it returns value1",
+ arg1 arg2
+),(
+ nvl2,
+ "Returns value2 if value1 is not NULL; otherwise, it returns value3.",
+ arg1 arg2 arg3
+),(
+ arrow_typeof,
+ "Returns the Arrow type of the input expression.",
+ arg1
+),(
+ r#struct,
+ "Returns a struct with the given arguments",
+ args,
+),(
+ named_struct,
+ "Returns a struct with the given names and arguments pairs",
+ args,
+),(
+ get_field,
+ "Returns the value of the field with the given name from the struct",
+ arg1, field_name
+),(
+ coalesce,
+ "Returns `coalesce(args...)`, which evaluates to the value of the first expr which is not NULL",
+ args,
Review Comment:
Single arguments followed by a single comma indicate a vector argument. The only downside here is that rust-analyzer is unable to identify the type of `args` and the dev is expected to read the doc of `export_functions!` to notice that `args` is a vector and not an Expr.
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1618774900
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
Maybe move `functions` out of macro?
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1624896673
##########
datafusion/functions/src/macros.rs:
##########
@@ -36,25 +36,31 @@
/// ]
/// }
/// ```
+///
+/// Exported functions accept:
+/// - `Vec<Expr>` argument (single argument followed by a comma)
+/// - Variable number of `Expr` arguments (zero or more arguments, must be without commas)
macro_rules! export_functions {
- ($(($FUNC:ident, $($arg:ident)*, $DOC:expr)),*) => {
- pub mod expr_fn {
- $(
- #[doc = $DOC]
- /// Return $name(arg)
- pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
- super::$FUNC().call(vec![$($arg),*],)
- }
- )*
+ ($(($FUNC:ident, $DOC:expr, $($arg:tt)*)),*) => {
+ $(
+ // switch to single-function cases below
+ export_functions!(single $FUNC, $DOC, $($arg)*);
+ )*
+ };
+
+ // single vector argument (a single argument followed by a comma)
Review Comment:
that is pretty fancy 👍
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1621415355
##########
datafusion/functions/src/unicode/mod.rs:
##########
@@ -47,22 +47,64 @@ make_udf_function!(reverse::ReverseFunc, REVERSE, reverse);
make_udf_function!(rpad::RPadFunc, RPAD, rpad);
make_udf_function!(strpos::StrposFunc, STRPOS, strpos);
make_udf_function!(substr::SubstrFunc, SUBSTR, substr);
+make_udf_function!(substr::SubstrFunc, SUBSTRING, substring);
make_udf_function!(substrindex::SubstrIndexFunc, SUBSTR_INDEX, substr_index);
make_udf_function!(translate::TranslateFunc, TRANSLATE, translate);
pub mod expr_fn {
use datafusion_expr::Expr;
+ export_functions!((
+ character_length,
+ "the number of characters in the `string`",
+ string
+ ),(
+ lpad,
+ "fill up a string to the length by prepending the characters",
+ args,
+ ),(
+ rpad,
+ "fill up a string to the length by appending the characters",
+ args,
+ ),(
+ reverse,
+ "reverses the `string`",
+ string
+ ),(
+ substr,
+ "substring from the `position` to the end",
+ string position
+ ),(
+ substr_index,
+ "Returns the substring from str before count occurrences of the delimiter",
+ string delimiter count
+ ),(
+ strpos,
+ "finds the position from where the `substring` matches the `string`",
+ string substring
+ ),(
+ substring,
+ "substring from the `position` with `length` characters",
+ string position length
+ ),(
+ translate,
+ "replaces the characters in `from` with the counterpart in `to`",
+ string from to
+ ),(
+ right,
+ "returns the last `n` characters in the `string`",
+ string n
+ ),(
+ left,
+ "returns the first `n` characters in the `string`",
+ string n
+ ));
+
#[doc = "the number of characters in the `string`"]
pub fn char_length(string: Expr) -> Expr {
character_length(string)
}
- #[doc = "the number of characters in the `string`"]
- pub fn character_length(string: Expr) -> Expr {
- super::character_length().call(vec![string])
- }
-
#[doc = "Returns a value in the range of 1 to N if the string str is in the string list strlist consisting of N substrings"]
Review Comment:
Oh wow I missed the `find_in_set`. The others are aliases, so the macro doesn't apply
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1621408653
##########
datafusion/functions/src/unicode/mod.rs:
##########
@@ -47,22 +47,64 @@ make_udf_function!(reverse::ReverseFunc, REVERSE, reverse);
make_udf_function!(rpad::RPadFunc, RPAD, rpad);
make_udf_function!(strpos::StrposFunc, STRPOS, strpos);
make_udf_function!(substr::SubstrFunc, SUBSTR, substr);
+make_udf_function!(substr::SubstrFunc, SUBSTRING, substring);
make_udf_function!(substrindex::SubstrIndexFunc, SUBSTR_INDEX, substr_index);
make_udf_function!(translate::TranslateFunc, TRANSLATE, translate);
pub mod expr_fn {
use datafusion_expr::Expr;
+ export_functions!((
+ character_length,
+ "the number of characters in the `string`",
+ string
+ ),(
+ lpad,
+ "fill up a string to the length by prepending the characters",
+ args,
+ ),(
+ rpad,
+ "fill up a string to the length by appending the characters",
+ args,
+ ),(
+ reverse,
+ "reverses the `string`",
+ string
+ ),(
+ substr,
+ "substring from the `position` to the end",
+ string position
+ ),(
+ substr_index,
+ "Returns the substring from str before count occurrences of the delimiter",
+ string delimiter count
+ ),(
+ strpos,
+ "finds the position from where the `substring` matches the `string`",
+ string substring
+ ),(
+ substring,
+ "substring from the `position` with `length` characters",
+ string position length
+ ),(
+ translate,
+ "replaces the characters in `from` with the counterpart in `to`",
+ string from to
+ ),(
+ right,
+ "returns the last `n` characters in the `string`",
+ string n
+ ),(
+ left,
+ "returns the first `n` characters in the `string`",
+ string n
+ ));
+
#[doc = "the number of characters in the `string`"]
pub fn char_length(string: Expr) -> Expr {
character_length(string)
}
- #[doc = "the number of characters in the `string`"]
- pub fn character_length(string: Expr) -> Expr {
- super::character_length().call(vec![string])
- }
-
#[doc = "Returns a value in the range of 1 to N if the string str is in the string list strlist consisting of N substrings"]
Review Comment:
why not use the macros for these function too?
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1617606993
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
This works fine, but I hate having special conditions in shared functions/macros. Only alternative I can currently think of is rewriting `get_field` and make both args of type `Expr`
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10700:
URL: https://github.com/apache/datafusion/pull/10700
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#issuecomment-2145901923
Thanks for the review! @jayzhan211 @alamb
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1623300815
##########
datafusion/functions/src/string/mod.rs:
##########
@@ -70,117 +70,98 @@ make_udf_function!(uuid::UuidFunc, UUID, uuid);
pub mod expr_fn {
use datafusion_expr::Expr;
- #[doc = "Returns the numeric code of the first character of the argument."]
- pub fn ascii(arg1: Expr) -> Expr {
- super::ascii().call(vec![arg1])
- }
-
- #[doc = "Returns the number of bits in the `string`"]
- pub fn bit_length(arg: Expr) -> Expr {
- super::bit_length().call(vec![arg])
- }
+ export_functions!((
+ ascii,
+ "Returns the numeric code of the first character of the argument.",
+ arg1
+ ),(
+ bit_length,
+ "Returns the number of bits in the `string`",
+ arg1
+ ),(
+ btrim,
+ "Removes all characters, spaces by default, from both sides of a string",
+ args,
+ ),(
+ chr,
+ "Converts the Unicode code point to a UTF8 character",
+ arg1
+ ),(
+ concat,
+ "Concatenates the text representations of all the arguments. NULL arguments are ignored",
+ args,
+ ),(
+ ends_with,
+ "Returns true if the `string` ends with the `suffix`, false otherwise.",
+ string suffix
+ ),(
+ initcap,
+ "Converts the first letter of each word in `string` in uppercase and the remaining characters in lowercase",
+ string
+ ),(
+ levenshtein,
+ "Returns the Levenshtein distance between the two given strings",
+ arg1 arg2
+ ),(
+ lower,
+ "Converts a string to lowercase.",
+ arg1
+ ),(
+ ltrim,
+ "Removes all characters, spaces by default, from the beginning of a string",
+ args,
+ ),(
+ octet_length,
+ "returns the number of bytes of a string",
+ args
+ ),(
+ overlay,
+ "replace the substring of string that starts at the start'th character and extends for count characters with new substring",
+ args,
+ ),(
+ repeat,
+ "Repeats the `string` to `n` times",
+ string n
+ ),(
+ replace,
+ "Replaces all occurrences of `from` with `to` in the `string`",
+ string from to
+ ),(
+ rtrim,
+ "Removes all characters, spaces by default, from the end of a string",
+ args,
+ ),(
+ split_part,
+ "Splits a string based on a delimiter and picks out the desired field based on the index.",
+ string delimiter index
+ ),(
+ starts_with,
+ "Returns true if string starts with prefix.",
+ arg1 arg2
+ ),(
+ to_hex,
+ "Converts an integer to a hexadecimal string.",
+ arg1
+ ),(
+ upper,
+ "Converts a string to uppercase.",
+ arg1
+ ),(
+ uuid,
+ "returns uuid v4 as a string value",
+ ));
#[doc = "Removes all characters, spaces by default, from both sides of a string"]
- pub fn btrim(args: Vec<Expr>) -> Expr {
+ pub fn trim(args: Vec<Expr>) -> Expr {
Review Comment:
Oh, it is alias
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1623300318
##########
datafusion/functions/src/string/mod.rs:
##########
@@ -70,117 +70,98 @@ make_udf_function!(uuid::UuidFunc, UUID, uuid);
pub mod expr_fn {
use datafusion_expr::Expr;
- #[doc = "Returns the numeric code of the first character of the argument."]
- pub fn ascii(arg1: Expr) -> Expr {
- super::ascii().call(vec![arg1])
- }
-
- #[doc = "Returns the number of bits in the `string`"]
- pub fn bit_length(arg: Expr) -> Expr {
- super::bit_length().call(vec![arg])
- }
+ export_functions!((
+ ascii,
+ "Returns the numeric code of the first character of the argument.",
+ arg1
+ ),(
+ bit_length,
+ "Returns the number of bits in the `string`",
+ arg1
+ ),(
+ btrim,
+ "Removes all characters, spaces by default, from both sides of a string",
+ args,
+ ),(
+ chr,
+ "Converts the Unicode code point to a UTF8 character",
+ arg1
+ ),(
+ concat,
+ "Concatenates the text representations of all the arguments. NULL arguments are ignored",
+ args,
+ ),(
+ ends_with,
+ "Returns true if the `string` ends with the `suffix`, false otherwise.",
+ string suffix
+ ),(
+ initcap,
+ "Converts the first letter of each word in `string` in uppercase and the remaining characters in lowercase",
+ string
+ ),(
+ levenshtein,
+ "Returns the Levenshtein distance between the two given strings",
+ arg1 arg2
+ ),(
+ lower,
+ "Converts a string to lowercase.",
+ arg1
+ ),(
+ ltrim,
+ "Removes all characters, spaces by default, from the beginning of a string",
+ args,
+ ),(
+ octet_length,
+ "returns the number of bytes of a string",
+ args
+ ),(
+ overlay,
+ "replace the substring of string that starts at the start'th character and extends for count characters with new substring",
+ args,
+ ),(
+ repeat,
+ "Repeats the `string` to `n` times",
+ string n
+ ),(
+ replace,
+ "Replaces all occurrences of `from` with `to` in the `string`",
+ string from to
+ ),(
+ rtrim,
+ "Removes all characters, spaces by default, from the end of a string",
+ args,
+ ),(
+ split_part,
+ "Splits a string based on a delimiter and picks out the desired field based on the index.",
+ string delimiter index
+ ),(
+ starts_with,
+ "Returns true if string starts with prefix.",
+ arg1 arg2
+ ),(
+ to_hex,
+ "Converts an integer to a hexadecimal string.",
+ arg1
+ ),(
+ upper,
+ "Converts a string to uppercase.",
+ arg1
+ ),(
+ uuid,
+ "returns uuid v4 as a string value",
+ ));
#[doc = "Removes all characters, spaces by default, from both sides of a string"]
- pub fn btrim(args: Vec<Expr>) -> Expr {
+ pub fn trim(args: Vec<Expr>) -> Expr {
Review Comment:
Is this rename intended?
--
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] Reduce code repetition in `datafusion/functions` mod files [datafusion]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1623283870
##########
datafusion/functions/src/unicode/mod.rs:
##########
@@ -47,22 +47,64 @@ make_udf_function!(reverse::ReverseFunc, REVERSE, reverse);
make_udf_function!(rpad::RPadFunc, RPAD, rpad);
make_udf_function!(strpos::StrposFunc, STRPOS, strpos);
make_udf_function!(substr::SubstrFunc, SUBSTR, substr);
+make_udf_function!(substr::SubstrFunc, SUBSTRING, substring);
make_udf_function!(substrindex::SubstrIndexFunc, SUBSTR_INDEX, substr_index);
make_udf_function!(translate::TranslateFunc, TRANSLATE, translate);
pub mod expr_fn {
use datafusion_expr::Expr;
+ export_functions!((
+ character_length,
+ "the number of characters in the `string`",
+ string
+ ),(
+ lpad,
+ "fill up a string to the length by prepending the characters",
+ args,
+ ),(
+ rpad,
+ "fill up a string to the length by appending the characters",
+ args,
+ ),(
+ reverse,
+ "reverses the `string`",
+ string
+ ),(
+ substr,
+ "substring from the `position` to the end",
+ string position
+ ),(
+ substr_index,
+ "Returns the substring from str before count occurrences of the delimiter",
+ string delimiter count
+ ),(
+ strpos,
+ "finds the position from where the `substring` matches the `string`",
+ string substring
+ ),(
+ substring,
+ "substring from the `position` with `length` characters",
+ string position length
+ ),(
+ translate,
+ "replaces the characters in `from` with the counterpart in `to`",
+ string from to
+ ),(
+ right,
+ "returns the last `n` characters in the `string`",
+ string n
+ ),(
+ left,
+ "returns the first `n` characters in the `string`",
+ string n
+ ));
+
#[doc = "the number of characters in the `string`"]
pub fn char_length(string: Expr) -> Expr {
character_length(string)
}
- #[doc = "the number of characters in the `string`"]
- pub fn character_length(string: Expr) -> Expr {
- super::character_length().call(vec![string])
- }
-
#[doc = "Returns a value in the range of 1 to N if the string str is in the string list strlist consisting of N substrings"]
Review Comment:
@alamb extremely sorry about the accidental force push, meant to push with lease after the rebase ...
--
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] Reduce repetition in math and functions modules with macros [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on code in PR #10700:
URL: https://github.com/apache/datafusion/pull/10700#discussion_r1618187445
##########
datafusion/functions/src/macros.rs:
##########
@@ -59,6 +59,30 @@ macro_rules! export_functions {
};
}
+macro_rules! make_function {
+ // single vector argument (a single argument followed by a comma)
+ ($FUNC:ident, $DOC:expr, $arg:ident,) => {
+ #[doc = $DOC]
+ pub fn $FUNC($arg: Vec<datafusion_expr::Expr>) -> datafusion_expr::Expr {
+ super::$FUNC().call($arg)
+ }
+ };
+ // variadic arguments (zero or more arguments, without commas)
+ ($FUNC:ident, $DOC:expr, $($arg:ident)*) => {
+ #[doc = $DOC]
+ pub fn $FUNC($($arg: datafusion_expr::Expr),*) -> datafusion_expr::Expr {
+ super::$FUNC().call(vec![$($arg),*])
+ }
+ };
+ // special to get_field function in datafusion/functions/src/core/mod.rs
+ ($FUNC:ident, $DOC:expr, $arg:ident, $arg2:ident) => {
Review Comment:
I don't agree to rewrite the second argument to Expr.
I think we can remove it out of macro and define it specifically given it's specialty
Use macro for general / trivial case only
--
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