yongster commented on code in PR #10991:
URL: https://github.com/apache/arrow-rs/pull/10991#discussion_r3939598503


##########
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:
   Done in 9ec63d572. The should-null bitmap is computed once (`right.values() 
& validity`), then this check uses `BooleanBuffer::has_true()` on that mask.



##########
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:
   Done. `should_null` is now computed once and reused by both the generic 
nullif path and `nullif_take`, so we no longer inspect `is_valid`/`value` per 
index.



##########
arrow-select/src/take.rs:
##########


Review Comment:
   Yes — the `take.rs` changes here are a subset of #10994. I'll wait for that 
to land, then rebase this PR so it only contains the `nullif.rs` 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to