Skip to content

Commit 1e40252

Browse files
jorisvandenbosscheraulcd
authored andcommitted
GH-41016: [C++] Fix null count check in BooleanArray.true_count() (#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: #41016 Authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
1 parent 0d3af8d commit 1e40252

File tree

2 files changed

+8
-1
lines changed

2 files changed

+8
-1
lines changed

cpp/src/arrow/array/array_primitive.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ int64_t BooleanArray::false_count() const {
5656
}
5757

5858
int64_t BooleanArray::true_count() const {
59-
if (data_->null_count.load() != 0) {
59+
if (data_->MayHaveNulls()) {
6060
DCHECK(data_->buffers[0]);
6161
return internal::CountAndSetBits(data_->buffers[0]->data(), data_->offset,
6262
data_->buffers[1]->data(), data_->offset,

cpp/src/arrow/array/array_test.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,6 +1307,13 @@ TEST(TestBooleanArray, TrueCountFalseCount) {
13071307
CheckArray(checked_cast<const BooleanArray&>(*arr));
13081308
CheckArray(checked_cast<const BooleanArray&>(*arr->Slice(5)));
13091309
CheckArray(checked_cast<const BooleanArray&>(*arr->Slice(0, 0)));
1310+
1311+
// GH-41016 true_count() with array without validity buffer with null_count of -1
1312+
auto arr_unknown_null_count = ArrayFromJSON(boolean(), "[true, false, true]");
1313+
arr_unknown_null_count->data()->null_count = kUnknownNullCount;
1314+
ASSERT_EQ(arr_unknown_null_count->data()->null_count.load(), -1);
1315+
ASSERT_EQ(arr_unknown_null_count->null_bitmap(), nullptr);
1316+
ASSERT_EQ(checked_pointer_cast<BooleanArray>(arr_unknown_null_count)->true_count(), 2);
13101317
}
13111318

13121319
TEST(TestPrimitiveAdHoc, TestType) {

0 commit comments

Comments
 (0)