You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "sunchao (via GitHub)" <gi...@apache.org> on 2023/02/03 00:44:28 UTC
[GitHub] [arrow-datafusion] sunchao commented on a diff in pull request #5151: Support arithmetic scalar operation with DictionaryArray
sunchao commented on code in PR #5151:
URL: https://github.com/apache/arrow-datafusion/pull/5151#discussion_r1095208879
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -315,6 +318,46 @@ macro_rules! compute_op_dyn_scalar {
}};
}
+/// Invoke a dyn compute kernel on a data array and a scalar value
+/// LEFT is Primitive or Dictionary array of numeric values, RIGHT is scalar value
+/// OP_TYPE is the return type of scalar function
+/// SCALAR_TYPE is the type of the scalar value
+macro_rules! compute_primitive_op_dyn_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:ident, $OP_TYPE:expr, $SCALAR_TYPE:ident) => {{
+ // generate the scalar function name, such as lt_dyn_scalar, from the $OP parameter
+ // (which could have a value of lt_dyn) and the suffix _scalar
+ if let Some(value) = $RIGHT {
+ Ok(Arc::new(paste::expr! {[<$OP _dyn_scalar>]::<$SCALAR_TYPE>}(
+ $LEFT,
+ value,
+ )?))
+ } else {
+ // when the $RIGHT is a NULL, generate a NULL array of $OP_TYPE
+ Ok(Arc::new(new_null_array($OP_TYPE, $LEFT.len())))
+ }
+ }};
+}
+
+/// Invoke a dyn decimal compute kernel on a data array and a scalar value
+/// LEFT is Decimal or Dictionary array of decimal values, RIGHT is scalar value
+/// OP_TYPE is the return type of scalar function
+/// SCALAR_TYPE is the type of the scalar value
Review Comment:
where is `SCALAR_TYPE`?
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -376,6 +419,37 @@ macro_rules! binary_primitive_array_op {
}};
}
+/// Invoke a compute dyn kernel on an array and a scalar
+/// The binary_primitive_array_op_dyn_scalar macro only evaluates for primitive
+/// types like integers and floats.
+macro_rules! binary_primitive_array_op_dyn_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:ident) => {{
+ // unwrap underlying (non dictionary) value
+ let right = unwrap_dict_value($RIGHT);
Review Comment:
It's interesting that we have "scalar dictionary":
```rust
/// Dictionary type: index type and value
Dictionary(Box<DataType>, Box<ScalarValue>),
```
wonder how useful this is ...
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -315,6 +318,46 @@ macro_rules! compute_op_dyn_scalar {
}};
}
+/// Invoke a dyn compute kernel on a data array and a scalar value
+/// LEFT is Primitive or Dictionary array of numeric values, RIGHT is scalar value
+/// OP_TYPE is the return type of scalar function
+/// SCALAR_TYPE is the type of the scalar value
+macro_rules! compute_primitive_op_dyn_scalar {
Review Comment:
maybe highlight the difference between this and `compute_op_dyn_scalar` in the comments.
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -315,6 +318,46 @@ macro_rules! compute_op_dyn_scalar {
}};
}
+/// Invoke a dyn compute kernel on a data array and a scalar value
+/// LEFT is Primitive or Dictionary array of numeric values, RIGHT is scalar value
+/// OP_TYPE is the return type of scalar function
+/// SCALAR_TYPE is the type of the scalar value
+macro_rules! compute_primitive_op_dyn_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:ident, $OP_TYPE:expr, $SCALAR_TYPE:ident) => {{
+ // generate the scalar function name, such as lt_dyn_scalar, from the $OP parameter
+ // (which could have a value of lt_dyn) and the suffix _scalar
+ if let Some(value) = $RIGHT {
+ Ok(Arc::new(paste::expr! {[<$OP _dyn_scalar>]::<$SCALAR_TYPE>}(
+ $LEFT,
+ value,
+ )?))
+ } else {
+ // when the $RIGHT is a NULL, generate a NULL array of $OP_TYPE
+ Ok(Arc::new(new_null_array($OP_TYPE, $LEFT.len())))
+ }
+ }};
+}
+
+/// Invoke a dyn decimal compute kernel on a data array and a scalar value
+/// LEFT is Decimal or Dictionary array of decimal values, RIGHT is scalar value
+/// OP_TYPE is the return type of scalar function
+/// SCALAR_TYPE is the type of the scalar value
+macro_rules! compute_primitive_decimal_op_dyn_scalar {
+ ($LEFT:expr, $RIGHT:expr, $OP:ident, $OP_TYPE:expr) => {{
+ // generate the scalar function name, such as lt_dyn_scalar, from the $OP parameter
+ // (which could have a value of lt_dyn) and the suffix _scalar
Review Comment:
nit: "such as lt_decimal_op_dyn_scalar"
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -924,18 +998,39 @@ impl BinaryExpr {
binary_array_op_dyn_scalar!(array, scalar.clone(), neq, bool_type)
}
Operator::Plus => {
- binary_primitive_array_op_scalar!(array, scalar.clone(), add)
+ binary_primitive_array_op_dyn_scalar!(
+ array,
+ scalar.clone(),
+ add,
+ left_type
+ )
}
Operator::Minus => {
- binary_primitive_array_op_scalar!(array, scalar.clone(), subtract)
+ binary_primitive_array_op_dyn_scalar!(
+ array,
+ scalar.clone(),
+ subtract,
+ left_type
+ )
}
Operator::Multiply => {
- binary_primitive_array_op_scalar!(array, scalar.clone(), multiply)
+ binary_primitive_array_op_dyn_scalar!(
+ array,
+ scalar.clone(),
+ multiply,
+ left_type
+ )
}
Operator::Divide => {
- binary_primitive_array_op_scalar!(array, scalar.clone(), divide)
+ binary_primitive_array_op_dyn_scalar!(
+ array,
+ scalar.clone(),
+ divide,
+ left_type
+ )
}
Operator::Modulo => {
+ // todo: change to binary_primitive_array_op_dyn_scalar! once modulo is implemented
Review Comment:
we can remove `binary_primitive_array_op_scalar` once this is done?
--
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