ARROW-15960: [C++] Fix crash on adaptive int builder edge cases

AdaptiveIntBuilder (and AdaptiveUIntBuilder) would crash when asked to append 0 nulls to a 0-length builder. Also fixes the following PyArrow crash: ```python pa.array([None], pa.list_(pa.dictionary(pa.int32(), pa.string()), 0)) ``` Closes #12690 from pitrou/ARROW-15960-adaptive-crash Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
apache · Mar 22, 2022 · ad2fb74 · ad2fb74
1 parent 2aeddad
commit ad2fb74
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 9 deletions.
diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc
@@ -2525,7 +2525,9 @@ TEST_F(TestAdaptiveIntBuilder, TestAppendNull) {
 TEST_F(TestAdaptiveIntBuilder, TestAppendNulls) {
   constexpr int64_t size = 10;
   ASSERT_EQ(0, builder_->null_count());
+  ASSERT_OK(builder_->AppendNulls(0));
   ASSERT_OK(builder_->AppendNulls(size));
+  ASSERT_OK(builder_->AppendNulls(0));
   ASSERT_EQ(size, builder_->null_count());
 
   Done();
@@ -2536,6 +2538,7 @@ TEST_F(TestAdaptiveIntBuilder, TestAppendNulls) {
 }
 
 TEST_F(TestAdaptiveIntBuilder, TestAppendEmptyValue) {
+  ASSERT_OK(builder_->AppendEmptyValues(0));
   ASSERT_OK(builder_->AppendNulls(2));
   ASSERT_OK(builder_->AppendEmptyValue());
   ASSERT_OK(builder_->Append(42));
@@ -2755,7 +2758,9 @@ TEST_F(TestAdaptiveUIntBuilder, TestAppendNull) {
 
 TEST_F(TestAdaptiveUIntBuilder, TestAppendNulls) {
   constexpr int64_t size = 10;
+  ASSERT_OK(builder_->AppendNulls(0));
   ASSERT_OK(builder_->AppendNulls(size));
+  ASSERT_OK(builder_->AppendNulls(0));
   ASSERT_EQ(size, builder_->null_count());
 
   Done();
@@ -2766,6 +2771,7 @@ TEST_F(TestAdaptiveUIntBuilder, TestAppendNulls) {
 }
 
 TEST_F(TestAdaptiveUIntBuilder, TestAppendEmptyValue) {
+  ASSERT_OK(builder_->AppendEmptyValues(0));
   ASSERT_OK(builder_->AppendNulls(2));
   ASSERT_OK(builder_->AppendEmptyValue());
   ASSERT_OK(builder_->Append(42));

diff --git a/cpp/src/arrow/array/builder_adaptive.h b/cpp/src/arrow/array/builder_adaptive.h
@@ -48,9 +48,11 @@ class ARROW_EXPORT AdaptiveIntBuilderBase : public ArrayBuilder {
   /// \param[in] length the number of nulls to append
   Status AppendNulls(int64_t length) final {
     ARROW_RETURN_NOT_OK(CommitPendingData());
-    ARROW_RETURN_NOT_OK(Reserve(length));
-    memset(data_->mutable_data() + length_ * int_size_, 0, int_size_ * length);
-    UnsafeSetNull(length);
+    if (ARROW_PREDICT_TRUE(length > 0)) {
+      ARROW_RETURN_NOT_OK(Reserve(length));
+      memset(data_->mutable_data() + length_ * int_size_, 0, int_size_ * length);
+      UnsafeSetNull(length);
+    }
     return Status::OK();
   }
 
@@ -70,9 +72,11 @@ class ARROW_EXPORT AdaptiveIntBuilderBase : public ArrayBuilder {
 
   Status AppendEmptyValues(int64_t length) final {
     ARROW_RETURN_NOT_OK(CommitPendingData());
-    ARROW_RETURN_NOT_OK(Reserve(length));
-    memset(data_->mutable_data() + length_ * int_size_, 0, int_size_ * length);
-    UnsafeSetNotNull(length);
+    if (ARROW_PREDICT_TRUE(length > 0)) {
+      ARROW_RETURN_NOT_OK(Reserve(length));
+      memset(data_->mutable_data() + length_ * int_size_, 0, int_size_ * length);
+      UnsafeSetNotNull(length);
+    }
     return Status::OK();
   }
 

diff --git a/python/pyarrow/tests/strategies.py b/python/pyarrow/tests/strategies.py
@@ -144,9 +144,7 @@ def list_types(item_strategy=primitive_types):
         st.builds(
             pa.list_,
             item_strategy,
-            # TODO set min_value back to 0
-            # (once https://issues.apache.org/jira/browse/ARROW-15960 is fixed)
-            st.integers(min_value=1, max_value=16)
+            st.integers(min_value=0, max_value=16)
         )
     )