viirya commented on code in PR #2940:
URL: https://github.com/apache/arrow-rs/pull/2940#discussion_r1008947176


##########
arrow/src/compute/kernels/boolean.rs:
##########
@@ -499,69 +494,72 @@ where
     //
     // Thus: result = left null bitmap & (!right_values | !right_bitmap)
     //              OR left null bitmap & !(right_values & right_bitmap)
-    //
-    // Do the right expression !(right_values & right_bitmap) first since 
there are two steps
-    // TRICK: convert BooleanArray buffer as a bitmap for faster operation
-    let rcb = match right.data().null_bitmap() {
-        Some(right_bitmap) => {
-            let and = buffer_bin_and(
-                right.values(),
-                right.offset(),
-                right_bitmap.buffer(),
-                right.offset(),
-                right.len(),
-            );
-            buffer_unary_not(&and, 0, right.len())
-        }
-        None => buffer_unary_not(right.values(), right.offset(), right.len()),
-    };
 
-    // AND of original left null bitmap with right expression
-    // Here we take care of the possible offsets of the left and right arrays 
all at once.
-    let modified_null_buffer = match left_data.null_bitmap() {
-        Some(left_null_bitmap) => buffer_bin_and(
-            left_null_bitmap.buffer(),
-            left_data.offset(),
-            &rcb,
+    // Compute right_values & right_bitmap
+    let (right, right_offset) = match right_data.null_buffer() {
+        Some(buffer) => (
+            buffer_bin_and(
+                &right_data.buffers()[0],
+                right_data.offset(),
+                buffer,
+                right_data.offset(),
+                len,
+            ),
             0,
-            left_data.len(),
         ),
-        None => rcb,
+        None => (right_data.buffers()[0].clone(), right_data.offset()),
     };
 
-    // Align/shift left data on offset as needed, since new bitmaps are 
shifted and aligned to 0 already
-    // NOTE: this probably only works for primitive arrays.
-    let data_buffers = if left.offset() == 0 {
-        left_data.buffers().to_vec()
-    } else {
-        // Shift each data buffer by type's bit_width * offset.
-        left_data
-            .buffers()
-            .iter()
-            .map(|buf| buf.slice(left.offset() * T::get_byte_width()))
-            .collect::<Vec<_>>()
+    // Compute left null bitmap & !right
+    let mut valid_count = 0;
+    let combined = match left_data.null_buffer() {
+        Some(left) => {
+            bitwise_bin_op_helper(left, left_offset, &right, right_offset, 
len, |l, r| {
+                let t = l & !r;
+                valid_count += t.count_ones() as usize;
+                t
+            })
+        }
+        None => bitwise_unary_op_helper(&right, right_offset, len, |b| {
+            let t = !b;
+            valid_count += t.count_ones() as usize;
+            t
+        }),

Review Comment:
   The logic looks the same as before.



-- 
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