Kanishk-Dendukuri commented on code in PR #50647:
URL: https://github.com/apache/arrow/pull/50647#discussion_r3662710110


##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -529,108 +492,246 @@ struct ScalarMinMax {
     }
   }
 
+  // Fold left/right into out_values/out_valid a word at a time (validity 
bitmap
+  // null => all valid). out_values may alias left. Returns the null count
+  static int64_t CombineWordwise(const OutValue* left, const uint8_t* 
left_valid,
+                                 int64_t left_offset, const OutValue* right,
+                                 const uint8_t* right_valid, int64_t 
right_offset,
+                                 bool skip_nulls, int64_t length, OutValue* 
out_values,
+                                 uint8_t* out_valid) {
+    auto left_reader = ::arrow::internal::BitmapUInt64Reader(
+        left_valid, left_valid ? left_offset : 0, left_valid ? length : 0);
+    auto right_reader = ::arrow::internal::BitmapUInt64Reader(
+        right_valid, right_valid ? right_offset : 0, right_valid ? length : 0);

Review Comment:
   for VisitTwoSetBitRuns it looks to be AND only, but we need OR as either 
side can make the output valid. I tried looking for existing OR facilities but 
wasn’t able to find something that would be good for this case (you would know 
more about this so lmk if there are others I should look into or if I am 
missing something). Previously I looked at VisitWords but it has the same issue 
as BitmapOr where we need to look at the validity bitmaps again to see which 
side to take



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