You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/05/11 13:27:12 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #2512: Remove remaining uses of `binary_array_op_scalar!` in binary.rs
alamb opened a new pull request, #2512:
URL: https://github.com/apache/arrow-datafusion/pull/2512
Draft until https://github.com/apache/arrow-datafusion/pull/2510 is merged
# Which issue does this PR close?
re https://github.com/apache/arrow-datafusion/issues/1179 and https://github.com/apache/arrow-datafusion/issues/2482 .
Closes #.
# Rationale for this change
1. `binary_array_op_scalar!` doesn't handle null constants (see https://github.com/apache/arrow-datafusion/pull/2510)
2. It is duplicative of `binary_array_op_dyn_scalar!`
# What changes are included in this PR?
1. Remove `binary_array_op_dyn_scalar!`
2. Remove code that then becomes unused without that macro 🎉
# Are there any user-facing changes?
No, all internal changes
--
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
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2512: Remove remaining uses of `binary_array_op_scalar!` in binary.rs
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2512:
URL: https://github.com/apache/arrow-datafusion/pull/2512#discussion_r870302946
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -1142,6 +1079,20 @@ macro_rules! binary_array_op_dyn_scalar {
}}
}
+/// Compares the array with the scalar value for equality, sometimes
+/// used in other kernels
+pub(crate) fn array_eq_scalar(lhs: &dyn Array, rhs: &ScalarValue) -> Result<ArrayRef> {
Review Comment:
I made a function to call `binary_array_op_dyn_scalar!` from `null_if` rather than having it call the macro directly to clean that code up.
--
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
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2512: Remove remaining uses of `binary_array_op_scalar!` in binary.rs
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2512:
URL: https://github.com/apache/arrow-datafusion/pull/2512#discussion_r870303487
##########
datafusion/physical-expr/src/expressions/nullif.rs:
##########
@@ -82,18 +80,18 @@ pub fn nullif_func(args: &[ColumnarValue]) -> Result<ColumnarValue> {
match (lhs, rhs) {
(ColumnarValue::Array(lhs), ColumnarValue::Scalar(rhs)) => {
- let cond_array = binary_array_op_scalar!(lhs, rhs.clone(), eq).unwrap()?;
+ let cond_array = array_eq_scalar(lhs, rhs)?;
let array = primitive_bool_array_op!(lhs, *cond_array, nullif)?;
Ok(ColumnarValue::Array(array))
}
(ColumnarValue::Array(lhs), ColumnarValue::Array(rhs)) => {
// Get args0 == args1 evaluated and produce a boolean array
- let cond_array = binary_array_op!(lhs, rhs, eq)?;
+ let cond_array = eq_dyn(lhs, rhs)?;
Review Comment:
Hooray for `eq_dyn` thanks again @Jimexist
--
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
[GitHub] [arrow-datafusion] alamb commented on pull request #2512: Remove unused `binary_array_op_scalar!` in binary.rs
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2512:
URL: https://github.com/apache/arrow-datafusion/pull/2512#issuecomment-1130046184
cc @WinkerDu @yjshen
--
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
[GitHub] [arrow-datafusion] andygrove merged pull request #2512: Remove unused `binary_array_op_scalar!` in binary.rs
Posted by GitBox <gi...@apache.org>.
andygrove merged PR #2512:
URL: https://github.com/apache/arrow-datafusion/pull/2512
--
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