Skip to content

Commit 2d5fdbf

Browse files
lxy-9602lxy-9602
authored
refactor: ColumnarArray: change constructor parameter to raw pointer to clarify non-owning semantics (alibaba#305)
1 parent c8978da commit 2d5fdbf

7 files changed

Lines changed: 39 additions & 34 deletions

File tree

‎src/paimon/common/data/binary_array_test.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ TEST(BinaryArrayTest, TestFromLongArray) {
332332
R"([[123, null], [789], [12345], [12]])")
333333
.ValueOrDie();
334334
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
335-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 2);
335+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);
336336

337337
BinaryArray ret = BinaryArray::FromLongArray(&array, pool.get());
338338

@@ -363,7 +363,7 @@ TEST(BinaryArrayTest, TestFromAllNullLongArray) {
363363
R"([[null, null], [789], [12345], [12]])")
364364
.ValueOrDie();
365365
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
366-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 2);
366+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);
367367

368368
BinaryArray ret = BinaryArray::FromLongArray(&array, pool.get());
369369

‎src/paimon/common/data/columnar/columnar_array.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ std::shared_ptr<InternalArray> ColumnarArray::GetArray(int32_t pos) const {
7070
assert(list_array);
7171
int32_t offset = list_array->value_offset(offset_ + pos);
7272
int32_t length = list_array->value_length(offset_ + pos);
73-
return std::make_shared<ColumnarArray>(list_array->values(), pool_, offset, length);
73+
return std::make_shared<ColumnarArray>(list_array->values().get(), pool_, offset, length);
7474
}
7575

7676
std::shared_ptr<InternalMap> ColumnarArray::GetMap(int32_t pos) const {

‎src/paimon/common/data/columnar/columnar_array.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,16 @@ class Bytes;
5050
class MemoryPool;
5151

5252
/// Columnar array to support access to vector column data.
53+
///
54+
/// NOTE: This class holds a non-owning raw pointer to the underlying arrow::Array for efficiency.
55+
/// The caller must ensure that the pointed-to Array outlives this ColumnarArray instance.
56+
/// Typically, lifetime is guaranteed by the owning ColumnarBatchContext or the parent
57+
/// arrow container (e.g., ListArray, MapArray) that holds the shared_ptr.
5358
class ColumnarArray : public InternalArray {
5459
public:
55-
ColumnarArray(const std::shared_ptr<arrow::Array>& array,
56-
const std::shared_ptr<MemoryPool>& pool, int32_t offset, int32_t length)
57-
: pool_(pool), array_(array.get()), offset_(offset), length_(length) {
60+
ColumnarArray(const arrow::Array* array, const std::shared_ptr<MemoryPool>& pool,
61+
int32_t offset, int32_t length)
62+
: pool_(pool), array_(array), offset_(offset), length_(length) {
5863
assert(array_);
5964
assert(array_->length() >= offset + length);
6065
}

‎src/paimon/common/data/columnar/columnar_array_test.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ TEST(ColumnarArrayTest, TestSimple) {
4040
arrow::list(arrow::boolean()), "[[true, false], [true], [false], [false, true]]")
4141
.ValueOrDie();
4242
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
43-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/2, 1);
43+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/2, 1);
4444
ASSERT_EQ(array.Size(), 1);
4545
ASSERT_EQ(array.GetBoolean(0), true);
4646
std::vector<char> expected_array = {static_cast<char>(1)};
@@ -51,7 +51,7 @@ TEST(ColumnarArrayTest, TestSimple) {
5151
"[[1, 1, 2], [3], [2], [2]]")
5252
.ValueOrDie();
5353
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
54-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/5, 1);
54+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/5, 1);
5555
ASSERT_EQ(array.GetByte(0), 2);
5656
std::vector<char> expected_array = {static_cast<char>(2)};
5757
ASSERT_EQ(array.ToByteArray().value(), expected_array);
@@ -61,7 +61,7 @@ TEST(ColumnarArrayTest, TestSimple) {
6161
"[[1, 1, 2], [3], [2], [-4]]")
6262
.ValueOrDie();
6363
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
64-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 3);
64+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 3);
6565
ASSERT_EQ(array.GetShort(0), 1);
6666
ASSERT_EQ(array.GetShort(1), 1);
6767
ASSERT_EQ(array.GetShort(2), 2);
@@ -73,7 +73,7 @@ TEST(ColumnarArrayTest, TestSimple) {
7373
"[[1, 1, 2], [3], [2], [-4]]")
7474
.ValueOrDie();
7575
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
76-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/3, 1);
76+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/3, 1);
7777
ASSERT_EQ(array.GetInt(0), 3);
7878
std::vector<int32_t> expected_array = {3};
7979
ASSERT_EQ(array.ToIntArray().value(), expected_array);
@@ -83,7 +83,7 @@ TEST(ColumnarArrayTest, TestSimple) {
8383
"[[1, 1, 2], [3], [2], [-4]]")
8484
.ValueOrDie();
8585
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
86-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/4, 1);
86+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/4, 1);
8787
ASSERT_EQ(array.GetLong(0), 2);
8888
std::vector<int64_t> expected_array = {2};
8989
ASSERT_EQ(array.ToLongArray().value(), expected_array);
@@ -93,15 +93,15 @@ TEST(ColumnarArrayTest, TestSimple) {
9393
"[[1, 1, 2], [3], [null], null]")
9494
.ValueOrDie();
9595
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
96-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/4, 1);
96+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/4, 1);
9797
ASSERT_NOK_WITH_MSG(array.ToLongArray(), "is null");
9898
}
9999
{
100100
auto f1 = arrow::ipc::internal::json::ArrayFromJSON(
101101
arrow::list(arrow::float32()), "[[0.0, 1.1, 2.2], [3.3], [4.4], [5.5]]")
102102
.ValueOrDie();
103103
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
104-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 3);
104+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 3);
105105
ASSERT_NEAR(array.GetFloat(1), 1.1, 0.001);
106106
std::vector<float> expected_array = {0.0, 1.1, 2.2};
107107
ASSERT_EQ(array.ToFloatArray().value(), expected_array);
@@ -111,7 +111,7 @@ TEST(ColumnarArrayTest, TestSimple) {
111111
arrow::list(arrow::float64()), "[[0.0, 1.1, 2.2], [3.3], [4.4], [5.5]]")
112112
.ValueOrDie();
113113
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
114-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/3, 1);
114+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/3, 1);
115115
ASSERT_NEAR(array.GetDouble(0), 3.3, 0.001);
116116
std::vector<double> expected_array = {3.3};
117117
ASSERT_EQ(array.ToDoubleArray().value(), expected_array);
@@ -121,7 +121,7 @@ TEST(ColumnarArrayTest, TestSimple) {
121121
arrow::list(arrow::utf8()), R"([["abc", "def"], ["efg"], ["hello"], ["hi"]])")
122122
.ValueOrDie();
123123
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
124-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/4, 1);
124+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/4, 1);
125125
ASSERT_EQ(array.GetString(0).ToString(), "hi");
126126
ASSERT_EQ(std::string(array.GetStringView(0)), "hi");
127127
}
@@ -134,7 +134,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
134134
"[[1, 1, 2], [3], [2], [-4]]")
135135
.ValueOrDie();
136136
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
137-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/3, 1);
137+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/3, 1);
138138
ASSERT_EQ(array.GetDate(0), 3);
139139
}
140140
{
@@ -143,7 +143,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
143143
R"([["1.234", "1234.000"], ["-9876.543"], ["666.888"]])")
144144
.ValueOrDie();
145145
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
146-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 2);
146+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);
147147
ASSERT_EQ(array.GetDecimal(0, 10, 3), Decimal(10, 3, 1234));
148148
}
149149
{
@@ -153,7 +153,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
153153
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
154154
.ValueOrDie();
155155
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
156-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 1);
156+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 1);
157157
auto ts = array.GetTimestamp(0, 9);
158158
ASSERT_EQ(ts, Timestamp(59000, 0));
159159
}
@@ -162,7 +162,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
162162
R"([["aaa", "bb"], ["ccc"], ["bbb"]])")
163163
.ValueOrDie();
164164
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
165-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 2);
165+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);
166166
ASSERT_EQ(*array.GetBinary(1), Bytes("bb", pool.get()));
167167
ASSERT_EQ(std::string(array.GetStringView(1)), "bb");
168168
}
@@ -181,7 +181,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
181181
])")
182182
.ValueOrDie();
183183
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
184-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 2);
184+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 2);
185185
auto result_row = array.GetRow(1, 4);
186186
ASSERT_EQ(result_row->GetLong(0), 2);
187187
ASSERT_EQ(result_row->GetLong(1), 2);
@@ -193,7 +193,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
193193
arrow::list(arrow::list(arrow::int64())), "[[[1, 2, 3], [4, 5, 6]], []]")
194194
.ValueOrDie();
195195
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
196-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 1);
196+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 1);
197197
auto result_array = array.GetArray(0);
198198
auto inner_result_array = array.GetArray(0);
199199
std::vector<int64_t> values = {1, 2, 3};
@@ -208,7 +208,7 @@ TEST(ColumnarArrayTest, TestComplexAndNestedType) {
208208
.ValueOrDie();
209209
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
210210
ASSERT_TRUE(list_array);
211-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/0, 1);
211+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/0, 1);
212212
auto result_key = array.GetMap(0)->KeyArray();
213213
auto result_value = array.GetMap(0)->ValueArray();
214214
ASSERT_EQ(result_key->ToIntArray().value(), std::vector<int32_t>({1, 4}));
@@ -225,7 +225,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
225225
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
226226
.ValueOrDie();
227227
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
228-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
228+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
229229
auto ts = array.GetTimestamp(0, 0);
230230
ASSERT_EQ(ts, Timestamp(951866603000, 0)) << ts.GetMillisecond();
231231
}
@@ -236,7 +236,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
236236
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
237237
.ValueOrDie();
238238
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
239-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
239+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
240240
auto ts = array.GetTimestamp(0, 3);
241241
ASSERT_EQ(ts, Timestamp(951866603001, 0)) << ts.GetMillisecond();
242242
}
@@ -247,7 +247,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
247247
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
248248
.ValueOrDie();
249249
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
250-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
250+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
251251
auto ts = array.GetTimestamp(0, 6);
252252
ASSERT_EQ(ts, Timestamp(951866603001, 1000)) << ts.GetMillisecond();
253253
}
@@ -258,7 +258,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
258258
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
259259
.ValueOrDie();
260260
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
261-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
261+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
262262
auto ts = array.GetTimestamp(0, 9);
263263
ASSERT_EQ(ts, Timestamp(951866603001, 1001)) << ts.GetMillisecond();
264264
}
@@ -269,7 +269,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
269269
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
270270
.ValueOrDie();
271271
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
272-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
272+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
273273
auto ts = array.GetTimestamp(0, 0);
274274
ASSERT_EQ(ts, Timestamp(951866603000, 0)) << ts.GetMillisecond();
275275
}
@@ -280,7 +280,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
280280
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
281281
.ValueOrDie();
282282
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
283-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
283+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
284284
auto ts = array.GetTimestamp(0, 3);
285285
ASSERT_EQ(ts, Timestamp(951866603001, 0)) << ts.GetMillisecond();
286286
}
@@ -291,7 +291,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
291291
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
292292
.ValueOrDie();
293293
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
294-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
294+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
295295
auto ts = array.GetTimestamp(0, 6);
296296
ASSERT_EQ(ts, Timestamp(951866603001, 1000)) << ts.GetMillisecond();
297297
}
@@ -302,7 +302,7 @@ TEST(ColumnarArrayTest, TestTimestampType) {
302302
"1899-01-01T00:59:20"],["2033-05-18T03:33:20"]])")
303303
.ValueOrDie();
304304
auto list_array = arrow::internal::checked_pointer_cast<arrow::ListArray>(f1);
305-
auto array = ColumnarArray(list_array->values(), pool, /*offset=*/1, 2);
305+
auto array = ColumnarArray(list_array->values().get(), pool, /*offset=*/1, 2);
306306
auto ts = array.GetTimestamp(0, 9);
307307
ASSERT_EQ(ts, Timestamp(951866603001, 1001)) << ts.GetMillisecond();
308308
}

‎src/paimon/common/data/columnar/columnar_map.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ ColumnarMap::ColumnarMap(const std::shared_ptr<arrow::Array>& key_array,
3535
length_(length) {}
3636

3737
std::shared_ptr<InternalArray> ColumnarMap::KeyArray() const {
38-
return std::make_shared<ColumnarArray>(key_array_, pool_, offset_, length_);
38+
return std::make_shared<ColumnarArray>(key_array_.get(), pool_, offset_, length_);
3939
}
4040
std::shared_ptr<InternalArray> ColumnarMap::ValueArray() const {
41-
return std::make_shared<ColumnarArray>(value_array_, pool_, offset_, length_);
41+
return std::make_shared<ColumnarArray>(value_array_.get(), pool_, offset_, length_);
4242
}
4343

4444
} // namespace paimon

‎src/paimon/common/data/columnar/columnar_row.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ std::shared_ptr<InternalArray> ColumnarRow::GetArray(int32_t pos) const {
6565
assert(list_array);
6666
int32_t offset = list_array->value_offset(row_id_);
6767
int32_t length = list_array->value_length(row_id_);
68-
return std::make_shared<ColumnarArray>(list_array->values(), pool_, offset, length);
68+
return std::make_shared<ColumnarArray>(list_array->values().get(), pool_, offset, length);
6969
}
7070

7171
std::shared_ptr<InternalMap> ColumnarRow::GetMap(int32_t pos) const {

‎src/paimon/common/data/columnar/columnar_row_ref.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ std::shared_ptr<InternalArray> ColumnarRowRef::GetArray(int32_t pos) const {
6767
assert(list_array);
6868
int32_t offset = list_array->value_offset(row_id_);
6969
int32_t length = list_array->value_length(row_id_);
70-
return std::make_shared<ColumnarArray>(list_array->values(), ctx_->pool, offset, length);
70+
return std::make_shared<ColumnarArray>(list_array->values().get(), ctx_->pool, offset, length);
7171
}
7272

7373
std::shared_ptr<InternalMap> ColumnarRowRef::GetMap(int32_t pos) const {

0 commit comments

Comments
 (0)