Skip to content

Commit

Permalink
apacheGH-41016: [C++] Fix null count check in BooleanArray.true_count…
Browse files Browse the repository at this point in the history
…() (apache#41070)

### Rationale for this change

Loading the `null_count` attribute doesn't take into account the possible value of -1, leading to a code path where the validity buffer is accessed, but which is not necessarily present in that case.

### What changes are included in this PR?

Use `data->MayHaveNulls()` instead of `data->null_count.load()`

### Are these changes tested?

Yes

* GitHub Issue: apache#41016

Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
jorisvandenbossche authored Apr 15, 2024
1 parent 80258fe commit 729dcb8
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_primitive.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ int64_t BooleanArray::false_count() const {
}

int64_t BooleanArray::true_count() const {
if (data_->null_count.load() != 0) {
if (data_->MayHaveNulls()) {
DCHECK(data_->buffers[0]);
return internal::CountAndSetBits(data_->buffers[0]->data(), data_->offset,
data_->buffers[1]->data(), data_->offset,
Expand Down
7 changes: 7 additions & 0 deletions cpp/src/arrow/array/array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,13 @@ TEST(TestBooleanArray, TrueCountFalseCount) {
CheckArray(checked_cast<const BooleanArray&>(*arr));
CheckArray(checked_cast<const BooleanArray&>(*arr->Slice(5)));
CheckArray(checked_cast<const BooleanArray&>(*arr->Slice(0, 0)));

// GH-41016 true_count() with array without validity buffer with null_count of -1
auto arr_unknown_null_count = ArrayFromJSON(boolean(), "[true, false, true]");
arr_unknown_null_count->data()->null_count = kUnknownNullCount;
ASSERT_EQ(arr_unknown_null_count->data()->null_count.load(), -1);
ASSERT_EQ(arr_unknown_null_count->null_bitmap(), nullptr);
ASSERT_EQ(checked_pointer_cast<BooleanArray>(arr_unknown_null_count)->true_count(), 2);
}

TEST(TestPrimitiveAdHoc, TestType) {
Expand Down

0 comments on commit 729dcb8

Please sign in to comment.