Skip to content

Commit 05d6497

Browse files
authored
fix(parquet): support nullable fixed-size lists for vector (#231)
1 parent eafe14e commit 05d6497

11 files changed

Lines changed: 251 additions & 385 deletions

File tree

cmake_modules/arrow.diff

Lines changed: 187 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,71 @@ index b36c38c6d4..f974a33073 100644
4848

4949
/// \brief Return zero-copy string_view to upcoming bytes.
5050
///
51+
diff --git a/cpp/src/arrow/util/bit_run_reader.h b/cpp/src/arrow/util/bit_run_reader.h
52+
index a436a503a0..27d483978c 100644
53+
--- a/cpp/src/arrow/util/bit_run_reader.h
54+
+++ b/cpp/src/arrow/util/bit_run_reader.h
55+
@@ -168,6 +168,26 @@ class ARROW_EXPORT BitRunReader {
56+
using BitRunReader = BitRunReaderLinear;
57+
#endif
58+
59+
+template <typename Visit>
60+
+inline Status VisitBitRuns(const uint8_t* bitmap, int64_t offset, int64_t length,
61+
+ Visit&& visit) {
62+
+ if (bitmap == NULLPTR) {
63+
+ // Assuming all set (as in a null bitmap)
64+
+ return visit(static_cast<int64_t>(0), length, true);
65+
+ }
66+
+ BitRunReader reader(bitmap, offset, length);
67+
+ int64_t position = 0;
68+
+ while (true) {
69+
+ const auto run = reader.NextRun();
70+
+ if (run.length == 0) {
71+
+ break;
72+
+ }
73+
+ ARROW_RETURN_NOT_OK(visit(position, run.length, run.set));
74+
+ position += run.length;
75+
+ }
76+
+ return Status::OK();
77+
+}
78+
+
79+
struct SetBitRun {
80+
int64_t position;
81+
int64_t length;
5182
diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc
52-
index 285e2a5973..db919d7ef8 100644
83+
index 285e2a5973..52f42cf5b3 100644
5384
--- a/cpp/src/parquet/arrow/reader.cc
5485
+++ b/cpp/src/parquet/arrow/reader.cc
86+
@@ -19,12 +19,14 @@
87+
88+
#include <algorithm>
89+
#include <cstring>
90+
+#include <iterator>
91+
#include <memory>
92+
#include <unordered_set>
93+
#include <utility>
94+
#include <vector>
95+
96+
#include "arrow/array.h"
97+
+#include "arrow/array/concatenate.h"
98+
#include "arrow/buffer.h"
99+
#include "arrow/extension_type.h"
100+
#include "arrow/io/memory.h"
101+
@@ -32,12 +34,14 @@
102+
#include "arrow/table.h"
103+
#include "arrow/type.h"
104+
#include "arrow/util/async_generator.h"
105+
+#include "arrow/util/bit_run_reader.h"
106+
#include "arrow/util/bit_util.h"
107+
#include "arrow/util/future.h"
108+
#include "arrow/util/iterator.h"
109+
#include "arrow/util/logging.h"
110+
#include "arrow/util/parallel.h"
111+
#include "arrow/util/range.h"
112+
+#include "arrow/util/span.h"
113+
#include "arrow/util/tracing_internal.h"
114+
#include "parquet/arrow/reader_internal.h"
115+
#include "parquet/column_reader.h"
55116
@@ -254,6 +254,11 @@ class FileReaderImpl : public FileReader {
56117
return GetColumn(i, AllRowGroupsFactory(), out);
57118
}
@@ -151,7 +212,87 @@ index 285e2a5973..db919d7ef8 100644
151212
virtual ::arrow::Result<std::shared_ptr<ChunkedArray>> AssembleArray(
152213
std::shared_ptr<ArrayData> data) {
153214
if (field_->type()->id() == ::arrow::Type::MAP) {
154-
@@ -709,6 +776,39 @@ class PARQUET_NO_EXPORT StructReader : public ColumnReaderImpl {
215+
@@ -642,8 +713,10 @@ class ListReader : public ColumnReaderImpl {
216+
217+
const std::shared_ptr<Field> field() override { return field_; }
218+
219+
- private:
220+
+ protected:
221+
std::shared_ptr<ReaderContext> ctx_;
222+
+
223+
+ private:
224+
std::shared_ptr<Field> field_;
225+
::parquet::internal::LevelInfo level_info_;
226+
std::unique_ptr<ColumnReaderImpl> item_reader_;
227+
@@ -662,12 +735,62 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader<int32_t> {
228+
DCHECK_EQ(field()->type()->id(), ::arrow::Type::FIXED_SIZE_LIST);
229+
const auto& type = checked_cast<::arrow::FixedSizeListType&>(*field()->type());
230+
const int32_t* offsets = reinterpret_cast<const int32_t*>(data->buffers[1]->data());
231+
- for (int x = 1; x <= data->length; x++) {
232+
- int32_t size = offsets[x] - offsets[x - 1];
233+
- if (size != type.list_size()) {
234+
- return Status::Invalid("Expected all lists to be of size=", type.list_size(),
235+
- " but index ", x, " had size=", size);
236+
+ const int32_t list_size = type.list_size();
237+
+ auto validate_offsets = [&](int64_t start, int64_t length,
238+
+ bool has_elements) -> Status {
239+
+ const int32_t expected_size = has_elements ? list_size : 0;
240+
+ ::arrow::util::span<const int32_t> run_offsets(
241+
+ offsets + start, static_cast<size_t>(length + 1));
242+
+ const auto first_invalid_offset = std::adjacent_find(
243+
+ run_offsets.begin(), run_offsets.end(),
244+
+ [&](int32_t left, int32_t right) { return right - left != expected_size; });
245+
+ if (first_invalid_offset != run_offsets.end()) {
246+
+ const int64_t x =
247+
+ start + std::distance(run_offsets.begin(), first_invalid_offset);
248+
+ const int32_t size = offsets[x + 1] - offsets[x];
249+
+ if (has_elements) {
250+
+ return Status::Invalid("Expected all lists to be of size=", list_size,
251+
+ " but index ", x + 1, " had size=", size);
252+
+ }
253+
+ return Status::Invalid("Expected null fixed-size list at index ", x + 1,
254+
+ " to have no child values but had size=", size);
255+
}
256+
+ return Status::OK();
257+
+ };
258+
+ if (data->GetNullCount() != 0) {
259+
+ // Rebuild the child array run-by-run so null fixed-size list slots still
260+
+ // contribute list_size child values in the final layout.
261+
+ ::arrow::ArrayVector child_arrays;
262+
+
263+
+ auto visit_run = [&](int64_t start, int64_t length, bool has_elements) -> Status {
264+
+ RETURN_NOT_OK(validate_offsets(start, length, has_elements));
265+
+
266+
+ const int64_t child_length = length * list_size;
267+
+ // Valid runs reuse the decoded child slice; null runs materialize null
268+
+ // children to preserve the fixed-size list shape.
269+
+ if (!has_elements) {
270+
+ ARROW_ASSIGN_OR_RAISE(
271+
+ auto null_array,
272+
+ ::arrow::MakeArrayOfNull(type.value_type(), child_length, ctx_->pool));
273+
+ child_arrays.push_back(std::move(null_array));
274+
+ return Status::OK();
275+
+ }
276+
+ child_arrays.push_back(
277+
+ ::arrow::MakeArray(data->child_data[0]->Slice(offsets[start], child_length)));
278+
+ return Status::OK();
279+
+ };
280+
+
281+
+ DCHECK_NE(data->buffers[0], nullptr);
282+
+ RETURN_NOT_OK(::arrow::internal::VisitBitRuns(
283+
+ data->buffers[0]->data(), data->offset, data->length, visit_run));
284+
+
285+
+ // TODO(GH-50271): Build one padded child array directly instead of creating
286+
+ // one temporary Array/ArrayData per validity run and concatenating them.
287+
+ ARROW_ASSIGN_OR_RAISE(auto child_array_with_padding,
288+
+ ::arrow::Concatenate(child_arrays, ctx_->pool));
289+
+ data->child_data[0] = child_array_with_padding->data();
290+
+ } else {
291+
+ RETURN_NOT_OK(validate_offsets(/*start=*/0, data->length, /*valid=*/true));
292+
}
293+
data->buffers.resize(1);
294+
std::shared_ptr<Array> result = ::arrow::MakeArray(data);
295+
@@ -709,6 +832,39 @@ class PARQUET_NO_EXPORT StructReader : public ColumnReaderImpl {
155296
}
156297
return Status::OK();
157298
}
@@ -191,7 +332,7 @@ index 285e2a5973..db919d7ef8 100644
191332
Status BuildArray(int64_t length_upper_bound,
192333
std::shared_ptr<ChunkedArray>* out) override;
193334
Status GetDefLevels(const int16_t** data, int64_t* length) override;
194-
@@ -1013,25 +1113,32 @@ Status FileReaderImpl::GetRecordBatchReader(const std::vector<int>& row_groups,
335+
@@ -1013,25 +1169,32 @@ Status FileReaderImpl::GetRecordBatchReader(const std::vector<int>& row_groups,
195336
return Status::OK();
196337
}
197338

@@ -230,7 +371,7 @@ index 285e2a5973..db919d7ef8 100644
230371

231372
RETURN_NOT_OK(::arrow::internal::OptionalParallelFor(
232373
reader_properties_.use_threads(), static_cast<int>(readers.size()),
233-
@@ -1224,6 +1331,23 @@ Status FileReaderImpl::GetColumn(int i, FileColumnIteratorFactory iterator_facto
374+
@@ -1224,6 +1387,23 @@ Status FileReaderImpl::GetColumn(int i, FileColumnIteratorFactory iterator_facto
234375
return Status::OK();
235376
}
236377

@@ -400,10 +541,49 @@ index ec3890a41f..943f69bb6c 100644
400541
return Status::OK();
401542
}
402543
diff --git a/cpp/src/parquet/arrow/writer.cc b/cpp/src/parquet/arrow/writer.cc
403-
index 4fd7ef1b47..87326a54f1 100644
544+
index 4fd7ef1b47..feff99c99b 100644
404545
--- a/cpp/src/parquet/arrow/writer.cc
405546
+++ b/cpp/src/parquet/arrow/writer.cc
406-
@@ -314,6 +314,14 @@ class FileWriterImpl : public FileWriter {
547+
@@ -26,6 +26,7 @@
548+
#include <vector>
549+
550+
#include "arrow/array.h"
551+
+#include "arrow/array/concatenate.h"
552+
#include "arrow/extension_type.h"
553+
#include "arrow/ipc/writer.h"
554+
#include "arrow/record_batch.h"
555+
@@ -142,13 +143,24 @@ class ArrowColumnWriterV2 {
556+
leaf_idx, ctx, [&](const MultipathLevelBuilderResult& result) {
557+
size_t visited_component_size = result.post_list_visited_elements.size();
558+
DCHECK_GT(visited_component_size, 0);
559+
- if (visited_component_size != 1) {
560+
- return Status::NotImplemented(
561+
- "Lists with non-zero length null components are not supported");
562+
+ std::shared_ptr<Array> values_array;
563+
+ if (visited_component_size == 1) {
564+
+ const ElementRange& range = result.post_list_visited_elements[0];
565+
+ values_array = result.leaf_array->Slice(range.start, range.Size());
566+
+ } else {
567+
+ // Multiple leaf ranges can be produced when child values are
568+
+ // skipped, such as null fixed-size-list slots, or when
569+
+ // list-view ranges are non-contiguous. Concatenate the slices
570+
+ // in logical write order.
571+
+ ::arrow::ArrayVector arrays;
572+
+ arrays.reserve(visited_component_size);
573+
+ for (const auto& range : result.post_list_visited_elements) {
574+
+ DCHECK(!range.Empty());
575+
+ arrays.push_back(result.leaf_array->Slice(range.start, range.Size()));
576+
+ }
577+
+ ARROW_ASSIGN_OR_RAISE(values_array,
578+
+ ::arrow::Concatenate(arrays, ctx->memory_pool));
579+
}
580+
- const ElementRange& range = result.post_list_visited_elements[0];
581+
- std::shared_ptr<Array> values_array =
582+
- result.leaf_array->Slice(range.start, range.Size());
583+
584+
return column_writer->WriteArrow(result.def_levels, result.rep_levels,
585+
result.def_rep_level_count, *values_array,
586+
@@ -314,6 +326,14 @@ class FileWriterImpl : public FileWriter {
407587
return Status::OK();
408588
}
409589

@@ -418,7 +598,7 @@ index 4fd7ef1b47..87326a54f1 100644
418598
Status Close() override {
419599
if (!closed_) {
420600
// Make idempotent
421-
@@ -418,10 +426,13 @@ class FileWriterImpl : public FileWriter {
601+
@@ -418,10 +438,13 @@ class FileWriterImpl : public FileWriter {
422602

423603
// Max number of rows allowed in a row group.
424604
const int64_t max_row_group_length = this->properties().max_row_group_length();

docs/source/user_guide/data_types.rst

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,9 @@ and `Arrow DataTypes <https://arrow.apache.org/docs/format/Columnar.html#data-ty
201201
VECTOR fields are rejected. VECTOR columns also cannot be partition or
202202
bucket keys. Dedicated vector storage is not included yet.
203203

204-
**Note:** A data file written by another engine that records the column as
205-
Arrow ``FixedSizeList`` instead of ``LIST``, such as Paimon Rust or Python,
206-
can only be read while it holds no NULL vector. Parquet stores a NULL list
207-
slot with no values, which the Arrow 17 Parquet reader rejects for a
208-
``FixedSizeList`` column.
204+
Paimon C++ also reads Parquet files written by Paimon Rust or Python whose
205+
embedded Arrow schema restores VECTOR columns as ``FixedSizeList``,
206+
including NULL vector values.
209207

210208
* - ``MAP<kt, vt>``
211209
- Map

src/paimon/core/io/vector_file_batch_reader.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ Result<std::shared_ptr<arrow::Array>> CastListToVector(
119119
fmt::format("Cannot restore VECTOR from type {}", array->type()->ToString()));
120120
}
121121
PAIMON_RETURN_NOT_OK(VectorUtils::ValidateVectorElements(*array));
122+
if (array->null_count() == array->length()) {
123+
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Array> result,
124+
arrow::MakeArrayOfNull(read_type, array->length(), pool));
125+
return result;
126+
}
122127
arrow::compute::ExecContext exec_context(pool);
123128
arrow::TypeHolder type_holder(read_type.get());
124129
arrow::compute::CastOptions options = arrow::compute::CastOptions::Safe();

src/paimon/format/parquet/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ set(PAIMON_PARQUET_FILE_FORMAT
2020
file_reader_wrapper.cpp
2121
page_filtered_row_group_reader.cpp
2222
parquet_timestamp_converter.cpp
23-
parquet_vector_converter.cpp
2423
parquet_file_batch_reader.cpp
2524
parquet_file_format_factory.cpp
2625
parquet_format_writer.cpp
@@ -56,7 +55,6 @@ if(PAIMON_BUILD_TESTS)
5655
file_reader_wrapper_test.cpp
5756
page_filtered_row_group_reader_test.cpp
5857
parquet_timestamp_converter_test.cpp
59-
parquet_vector_converter_test.cpp
6058
parquet_vector_io_test.cpp
6159
parquet_field_id_converter_test.cpp
6260
parquet_file_batch_reader_test.cpp

src/paimon/format/parquet/parquet_format_writer.cpp

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include <string_view>
2424
#include <utility>
2525

26-
#include "arrow/array/array_nested.h"
2726
#include "arrow/c/bridge.h"
2827
#include "arrow/memory_pool.h"
2928
#include "arrow/record_batch.h"
@@ -32,9 +31,7 @@
3231
#include "paimon/common/metrics/metrics_impl.h"
3332
#include "paimon/common/utils/arrow/arrow_output_stream_adapter.h"
3433
#include "paimon/common/utils/arrow/status_utils.h"
35-
#include "paimon/common/utils/checked_cast.h"
3634
#include "paimon/format/parquet/parquet_format_defs.h"
37-
#include "paimon/format/parquet/parquet_vector_converter.h"
3835
#include "parquet/arrow/writer.h"
3936
#include "parquet/properties.h"
4037

@@ -58,33 +55,17 @@ Result<std::unique_ptr<ParquetFormatWriter>> ParquetFormatWriter::Create(
5855
::parquet::ArrowWriterProperties::Builder arrow_properties_builder;
5956
auto arrow_writer_properties =
6057
arrow_properties_builder.enable_deprecated_int96_timestamps()->build();
61-
auto logical_type = arrow::struct_(schema->fields());
62-
auto write_type =
63-
checked_pointer_cast<arrow::StructType>(ParquetVectorConverter::GetWriteType(logical_type));
64-
auto write_schema = arrow::schema(write_type->fields(), schema->metadata());
6558
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
6659
std::unique_ptr<::parquet::arrow::FileWriter> file_writer,
67-
::parquet::arrow::FileWriter::Open(*write_schema, pool.get(), out, writer_properties,
60+
::parquet::arrow::FileWriter::Open(*schema, pool.get(), out, writer_properties,
6861
arrow_writer_properties));
69-
return std::unique_ptr<ParquetFormatWriter>(new ParquetFormatWriter(
70-
std::move(file_writer), out, schema, max_memory_use,
71-
/*needs_vector_conversion=*/!logical_type->Equals(write_type), pool));
62+
return std::unique_ptr<ParquetFormatWriter>(
63+
new ParquetFormatWriter(std::move(file_writer), out, schema, max_memory_use, pool));
7264
}
7365

7466
Status ParquetFormatWriter::AddBatch(ArrowArray* batch) {
7567
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<::arrow::RecordBatch> record_batch,
7668
arrow::ImportRecordBatch(batch, schema_));
77-
if (needs_vector_conversion_) {
78-
// TODO(ChaomingZhangCN): Remove this conversion after upgrading Arrow. Arrow 17
79-
// mishandles nullable FixedSizeList values when writing them as Parquet LIST.
80-
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::StructArray> struct_array,
81-
record_batch->ToStructArray());
82-
std::shared_ptr<arrow::Array> array = struct_array;
83-
PAIMON_ASSIGN_OR_RAISE(array,
84-
ParquetVectorConverter::ConvertToWriteType(array, pool_.get()));
85-
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(record_batch,
86-
arrow::RecordBatch::FromStructArray(array, pool_.get()));
87-
}
8869
if (static_cast<uint64_t>(pool_->bytes_allocated()) > max_memory_use_) {
8970
PAIMON_RETURN_NOT_OK_FROM_ARROW(writer_->NewBufferedRowGroup());
9071
}
@@ -132,14 +113,13 @@ Result<uint64_t> ParquetFormatWriter::GetEstimateLength() const {
132113
ParquetFormatWriter::ParquetFormatWriter(std::unique_ptr<::parquet::arrow::FileWriter> writer,
133114
const std::shared_ptr<ArrowOutputStreamAdapter>& out,
134115
const std::shared_ptr<arrow::Schema>& schema,
135-
uint64_t max_memory_use, bool needs_vector_conversion,
116+
uint64_t max_memory_use,
136117
const std::shared_ptr<arrow::MemoryPool>& pool)
137118
: pool_(pool),
138119
out_(out),
139120
writer_(std::move(writer)),
140121
schema_(schema),
141122
metrics_(std::make_shared<MetricsImpl>()),
142-
max_memory_use_(max_memory_use),
143-
needs_vector_conversion_(needs_vector_conversion) {}
123+
max_memory_use_(max_memory_use) {}
144124

145125
} // namespace paimon::parquet

src/paimon/format/parquet/parquet_format_writer.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ class ParquetFormatWriter : public FormatWriter {
7272
ParquetFormatWriter(std::unique_ptr<::parquet::arrow::FileWriter> writer,
7373
const std::shared_ptr<ArrowOutputStreamAdapter>& out,
7474
const std::shared_ptr<arrow::Schema>& schema, uint64_t max_memory_use,
75-
bool needs_vector_conversion,
7675
const std::shared_ptr<arrow::MemoryPool>& pool);
7776

7877
Result<uint64_t> GetEstimateLength() const;
@@ -84,7 +83,6 @@ class ParquetFormatWriter : public FormatWriter {
8483
std::shared_ptr<Metrics> metrics_;
8584
int64_t total_records_written_ = 0;
8685
uint64_t max_memory_use_;
87-
bool needs_vector_conversion_;
8886
};
8987

9088
} // namespace paimon::parquet

0 commit comments

Comments
 (0)