From a8169c47c05f1442a10bb337352c4e042fbcaf44 Mon Sep 17 00:00:00 2001 From: mwish Date: Mon, 28 Oct 2024 19:15:23 +0800 Subject: [PATCH] GH-44541: NumericArray should not use child's ctor directly --- cpp/src/arrow/array/array_primitive.h | 16 +++++++++++----- cpp/src/arrow/array/array_test.cc | 12 ++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/array/array_primitive.h b/cpp/src/arrow/array/array_primitive.h index 3e2893b7dd898..0366d1abcd3cf 100644 --- 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>; - explicit NumericArray(const std::shared_ptr& data) { SetData(data); } + explicit NumericArray(const std::shared_ptr& 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& data, const std::shared_ptr& null_bitmap = NULLPTR, int64_t null_count = kUnknownNullCount, int64_t offset = 0) { - SetData(ArrayData::Make(TypeTraits::type_singleton(), length, {null_bitmap, data}, - null_count, offset)); + NumericArray::SetData(ArrayData::Make(TypeTraits::type_singleton(), length, + {null_bitmap, data}, null_count, offset)); + } + + NumericArray(std::shared_ptr type, int64_t length, const std::shared_ptr& data, + const std::shared_ptr& 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& 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 index d69e00460dcfc..ab41cde5eb069 100644 --- 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 original_data{1, 2, 3, 4, 5, 6, 7}; + std::shared_ptr arr = std::make_shared(::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) { std::unique_ptr builder; auto null_scalar = MakeNullScalar(scalar->type);