You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/09/18 21:03:11 UTC
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7568: feat: natively support more data types for the `abs` function.
alamb commented on code in PR #7568:
URL: https://github.com/apache/arrow-datafusion/pull/7568#discussion_r1329275892
##########
datafusion/sqllogictest/test_files/math.slt:
##########
@@ -123,35 +135,45 @@ CREATE TABLE test_nullable_integer(
c6 SMALLINT UNSIGNED,
c7 INT UNSIGNED,
c8 BIGINT UNSIGNED,
+ dataset TEXT
Review Comment:
Nice
##########
datafusion/expr/src/built_in_function.rs:
##########
@@ -1162,8 +1163,9 @@ impl BuiltinScalarFunction {
Signature::uniform(2, vec![Int64], self.volatility())
}
BuiltinScalarFunction::ArrowTypeof => Signature::any(1, self.volatility()),
- BuiltinScalarFunction::Abs
- | BuiltinScalarFunction::Acos
+ BuiltinScalarFunction::Abs => Signature::any(1, self.volatility()),
Review Comment:
I don't understand this comment
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -667,6 +673,70 @@ fn compute_truncate64(x: f64, y: i64) -> f64 {
(x * factor).round() / factor
}
+macro_rules! make_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.unary(|x| x.abs());
Review Comment:
👍
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -667,6 +673,70 @@ fn compute_truncate64(x: f64, y: i64) -> f64 {
(x * factor).round() / factor
}
+macro_rules! make_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.unary(|x| x.abs());
+ Ok(Arc::new(res) as ArrayRef)
+ }
+ }};
+}
+
+macro_rules! make_try_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.try_unary(|x| {
+ x.checked_abs().ok_or_else(|| {
+ ArrowError::ComputeError(format!(
+ "{} overflow on abs({})",
+ stringify!($ARRAY_TYPE),
+ x
+ ))
+ })
+ })?;
+ Ok(Arc::new(res) as ArrayRef)
+ }
+ }};
+}
+
+/// Abs SQL function
+/// Return different implementations based on input datatype to reduce branches during execution
+pub(super) fn create_abs_function(
+ input_data_type: &DataType,
+) -> Result<MathArrayFunction> {
+ match input_data_type {
+ DataType::Float32 => Ok(make_abs_function!(Float32Array)),
+ DataType::Float64 => Ok(make_abs_function!(Float64Array)),
+
+ // Types that may overflow, such as abs(-128_i8).
+ DataType::Int8 => Ok(make_try_abs_function!(Int8Array)),
+ DataType::Int16 => Ok(make_try_abs_function!(Int16Array)),
+ DataType::Int32 => Ok(make_try_abs_function!(Int32Array)),
+ DataType::Int64 => Ok(make_try_abs_function!(Int64Array)),
+
+ // Types of results are the same as the input.
+ DataType::Null
+ | DataType::UInt8
+ | DataType::UInt16
+ | DataType::UInt32
+ | DataType::UInt64 => Ok(|args: &[ArrayRef]| Ok(args[0].clone())),
Review Comment:
👍
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -667,6 +673,70 @@ fn compute_truncate64(x: f64, y: i64) -> f64 {
(x * factor).round() / factor
}
+macro_rules! make_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.unary(|x| x.abs());
+ Ok(Arc::new(res) as ArrayRef)
+ }
+ }};
+}
+
+macro_rules! make_try_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.try_unary(|x| {
+ x.checked_abs().ok_or_else(|| {
+ ArrowError::ComputeError(format!(
+ "{} overflow on abs({})",
+ stringify!($ARRAY_TYPE),
+ x
+ ))
+ })
+ })?;
+ Ok(Arc::new(res) as ArrayRef)
+ }
+ }};
+}
+
+/// Abs SQL function
+/// Return different implementations based on input datatype to reduce branches during execution
+pub(super) fn create_abs_function(
+ input_data_type: &DataType,
+) -> Result<MathArrayFunction> {
+ match input_data_type {
+ DataType::Float32 => Ok(make_abs_function!(Float32Array)),
+ DataType::Float64 => Ok(make_abs_function!(Float64Array)),
+
+ // Types that may overflow, such as abs(-128_i8).
+ DataType::Int8 => Ok(make_try_abs_function!(Int8Array)),
+ DataType::Int16 => Ok(make_try_abs_function!(Int16Array)),
+ DataType::Int32 => Ok(make_try_abs_function!(Int32Array)),
+ DataType::Int64 => Ok(make_try_abs_function!(Int64Array)),
+
+ // Types of results are the same as the input.
+ DataType::Null
+ | DataType::UInt8
+ | DataType::UInt16
+ | DataType::UInt32
+ | DataType::UInt64 => Ok(|args: &[ArrayRef]| Ok(args[0].clone())),
Review Comment:
👍
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -667,6 +673,70 @@ fn compute_truncate64(x: f64, y: i64) -> f64 {
(x * factor).round() / factor
}
+macro_rules! make_abs_function {
+ ($ARRAY_TYPE:ident) => {{
+ |args: &[ArrayRef]| {
+ let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+ let res: $ARRAY_TYPE = array.unary(|x| x.abs());
Review Comment:
👍
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org