ARROW-17071: [C++][Compute] Fixing off-by-one error in hash join node (apache#13616)

michalursa · web-flow · commit abcfbd785e98 · 2022-07-18T08:37:10.000-10:00
Fixing off-by-one error in hash join node.
Zeroing allocated bit vectors in hash join node to fix another valgrind error.

Authored-by: michalursa &lt;michal@ursacomputing.com&gt;
Signed-off-by: Weston Pace &lt;weston.pace@gmail.com&gt;
diff --git a/cpp/src/arrow/compute/exec/util.h b/cpp/src/arrow/compute/exec/util.h
@@ -335,7 +335,7 @@ class TailSkipForSIMD {
   static int FixSelection(int64_t num_rows_safe, int num_selected,
                           const uint16_t* selection) {
     int num_selected_safe = num_selected;
-    while (num_selected_safe > 0 && selection[num_selected_safe] >= num_rows_safe) {
+    while (num_selected_safe > 0 && selection[num_selected_safe - 1] >= num_rows_safe) {
       --num_selected_safe;
     }
     return num_selected_safe;
diff --git a/cpp/src/arrow/compute/light_array.cc b/cpp/src/arrow/compute/light_array.cc
@@ -237,13 +237,17 @@ Status ResizableArrayData::ResizeFixedLengthBuffers(int num_rows_new) {
         buffers_[kValidityBuffer],
         AllocateResizableBuffer(
             bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes, pool_));
+    memset(mutable_data(kValidityBuffer), 0,
+           bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes);
     if (column_metadata.is_fixed_length) {
       if (column_metadata.fixed_length == 0) {
         ARROW_ASSIGN_OR_RAISE(
             buffers_[kFixedLengthBuffer],
             AllocateResizableBuffer(
                 bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes,
                 pool_));
+        memset(mutable_data(kFixedLengthBuffer), 0,
+               bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes);
       } else {
         ARROW_ASSIGN_OR_RAISE(
             buffers_[kFixedLengthBuffer],
@@ -267,13 +271,22 @@ Status ResizableArrayData::ResizeFixedLengthBuffers(int num_rows_new) {
     ARROW_DCHECK(buffers_[kValidityBuffer] != NULLPTR &&
                  buffers_[kVariableLengthBuffer] != NULLPTR);
 
+    int64_t bytes_for_bits_before =
+        bit_util::BytesForBits(num_rows_allocated_) + kNumPaddingBytes;
+    int64_t bytes_for_bits_after =
+        bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes;
+
     RETURN_NOT_OK(buffers_[kValidityBuffer]->Resize(
         bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes));
+    memset(mutable_data(kValidityBuffer) + bytes_for_bits_before, 0,
+           bytes_for_bits_after - bytes_for_bits_before);
 
     if (column_metadata.is_fixed_length) {
       if (column_metadata.fixed_length == 0) {
         RETURN_NOT_OK(buffers_[kFixedLengthBuffer]->Resize(
             bit_util::BytesForBits(num_rows_allocated_new) + kNumPaddingBytes));
+        memset(mutable_data(kFixedLengthBuffer) + bytes_for_bits_before, 0,
+               bytes_for_bits_after - bytes_for_bits_before);
       } else {
         RETURN_NOT_OK(buffers_[kFixedLengthBuffer]->Resize(
             num_rows_allocated_new * column_metadata.fixed_length + kNumPaddingBytes));

Original file line number	Diff line number	Diff line change
`@@ -335,7 +335,7 @@ class TailSkipForSIMD {`
`335`	`335`	`static int FixSelection(int64_t num_rows_safe, int num_selected,`
`336`	`336`	`const uint16_t* selection) {`
`337`	`337`	`int num_selected_safe = num_selected;`
`338`		`- while (num_selected_safe > 0 && selection[num_selected_safe] >= num_rows_safe) {`
	`338`	`+ while (num_selected_safe > 0 && selection[num_selected_safe - 1] >= num_rows_safe) {`
`339`	`339`	`--num_selected_safe;`
`340`	`340`	`}`
`341`	`341`	`return num_selected_safe;`