This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 729dcb8120 GH-41016: [C++] Fix null count check in
BooleanArray.true_count() (#41070)
729dcb8120 is described below
commit 729dcb8120dca6d35578d70a845ecda6f79efa1a
Author: Joris Van den Bossche <[email protected]>
AuthorDate: Mon Apr 15 17:30:28 2024 +0200
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]>
---
cpp/src/arrow/array/array_primitive.cc | 2 +-
cpp/src/arrow/array/array_test.cc | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/cpp/src/arrow/array/array_primitive.cc
b/cpp/src/arrow/array/array_primitive.cc
index 7c4a14d934..da3810aa39 100644
--- a/cpp/src/arrow/array/array_primitive.cc
+++ b/cpp/src/arrow/array/array_primitive.cc
@@ -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,
diff --git a/cpp/src/arrow/array/array_test.cc
b/cpp/src/arrow/array/array_test.cc
index 21ac1a09f5..60efdb4768 100644
--- a/cpp/src/arrow/array/array_test.cc
+++ b/cpp/src/arrow/array/array_test.cc
@@ -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) {