apacheGH-44541: NumericArray<T> should not use child's ctor directly

mapleFU · Oct 28, 2024 · a8169c4 · a8169c4
1 parent a3c39ec
commit a8169c4
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 5 deletions.
diff --git a/cpp/src/arrow/array/array_primitive.h b/cpp/src/arrow/array/array_primitive.h
@@ -90,7 +90,9 @@ class NumericArray : public PrimitiveArray {
   using value_type = typename TypeClass::c_type;
   using IteratorType = stl::ArrayIterator<NumericArray<TYPE>>;
 
-  explicit NumericArray(const std::shared_ptr<ArrayData>& data) { SetData(data); }
+  explicit NumericArray(const std::shared_ptr<ArrayData>& data) {
+    NumericArray::SetData(data);
+  }
 
   // Only enable this constructor without a type argument for types without additional
   // metadata
@@ -99,8 +101,14 @@ class NumericArray : public PrimitiveArray {
                const std::shared_ptr<Buffer>& data,
                const std::shared_ptr<Buffer>& null_bitmap = NULLPTR,
                int64_t null_count = kUnknownNullCount, int64_t offset = 0) {
-    SetData(ArrayData::Make(TypeTraits<T1>::type_singleton(), length, {null_bitmap, data},
-                            null_count, offset));
+    NumericArray::SetData(ArrayData::Make(TypeTraits<T1>::type_singleton(), length,
+                                          {null_bitmap, data}, null_count, offset));
+  }
+
+  NumericArray(std::shared_ptr<DataType> type, int64_t length, const std::shared_ptr<Buffer>& data,
+               const std::shared_ptr<Buffer>& null_bitmap = NULLPTR,
+               int64_t null_count = kUnknownNullCount, int64_t offset = 0) {
+    NumericArray::SetData(ArrayData::Make(std::move(type), length, {null_bitmap, data}, null_count, offset));
   }
 
   const value_type* raw_values() const { return values_; }
@@ -119,8 +127,6 @@ class NumericArray : public PrimitiveArray {
   IteratorType end() const { return IteratorType(*this, length()); }
 
  protected:
-  using PrimitiveArray::PrimitiveArray;
-
   void SetData(const std::shared_ptr<ArrayData>& data) {
     this->PrimitiveArray::SetData(data);
     values_ = raw_values_

diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc
@@ -585,6 +585,18 @@ TEST_F(TestArray, TestValidateNullCount) {
   }
 }
 
+TEST_F(TestArray, TestValidValues) {
+  // GH-44541: The value_ should be valid when construct.
+  std::vector<int32_t> original_data{1, 2, 3, 4, 5, 6, 7};
+  std::shared_ptr<Int32Array> arr = std::make_shared<Int32Array>(::arrow::int32(), 7,
+                                                            Buffer::Wrap(original_data));
+  for (size_t i = 0; i < original_data.size(); ++i) {
+    EXPECT_TRUE(arr->IsValid(i));
+    EXPECT_FALSE(arr->IsNull(i));
+    EXPECT_EQ(original_data[i], arr->Value(i));
+  }
+}
+
 void AssertAppendScalar(MemoryPool* pool, const std::shared_ptr<Scalar>& scalar) {
   std::unique_ptr<arrow::ArrayBuilder> builder;
   auto null_scalar = MakeNullScalar(scalar->type);