Jefffrey commented on code in PR #10991:
URL: https://github.com/apache/arrow-rs/pull/10991#discussion_r3938968474
##########
arrow-select/src/nullif.rs:
##########
@@ -111,17 +124,34 @@ pub fn nullif(left: &dyn Array, right: &BooleanArray) ->
Result<ArrayRef, ArrowE
Ok(make_array(unsafe { data.build_unchecked() }))
}
+/// Applies `nullif` to arrays that represent logical nulls in their children.
+fn nullif_take(left: &dyn Array, right: &BooleanArray) -> Result<ArrayRef,
ArrowError> {
Review Comment:
perhaps we can reuse the computation we do here:
https://github.com/apache/arrow-rs/blob/b372d1f14cfe8974d8b71849e83bfa85f2a0494c/arrow-select/src/nullif.rs#L58-L71
instead of needing to check validity in this iterator
##########
arrow-select/src/nullif.rs:
##########
@@ -42,19 +42,32 @@ use arrow_schema::{ArrowError, DataType};
/// assert_eq!(nulled.as_primitive(), &Int32Array::from(vec![None, None,
Some(1), Some(9)]));
/// ```
pub fn nullif(left: &dyn Array, right: &BooleanArray) -> Result<ArrayRef,
ArrowError> {
- let left_data = left.to_data();
-
- if left_data.len() != right.len() {
+ if left.len() != right.len() {
return Err(ArrowError::ComputeError(
"Cannot perform comparison operation on arrays of different
length".to_string(),
));
}
- let len = left_data.len();
+ let len = left.len();
+
+ if len == 0 || left.data_type() == &DataType::Null {
+ return Ok(make_array(left.to_data()));
+ }
- if len == 0 || left_data.data_type() == &DataType::Null {
- return Ok(make_array(left_data));
+ match left.data_type() {
+ DataType::RunEndEncoded(_, values) => {
+ if !values.is_nullable() && right.iter().any(|value| value ==
Some(true)) {
Review Comment:
```suggestion
if !values.is_nullable() && right.has_true() {
```
-
https://docs.rs/arrow/latest/arrow/array/struct.BooleanArray.html#method.has_true
-
https://docs.rs/arrow/latest/arrow/buffer/struct.BooleanBuffer.html#method.has_true
##########
arrow-select/src/take.rs:
##########
Review Comment:
- same fixes as https://github.com/apache/arrow-rs/pull/10994? should we
wait for it to land first?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
