From 83385132ac99323881e27b27d98daf35be5c7b2f Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Sat, 27 Jun 2026 15:11:18 +0800 Subject: [PATCH 1/5] support to read fixed size list array with nulls --- .../parquet/arrow/arrow_reader_writer_test.cc | 69 +++++++++++++++++ cpp/src/parquet/arrow/reader.cc | 75 ++++++++++++++++--- 2 files changed, 133 insertions(+), 11 deletions(-) diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc index 8735aea731ce..b7ac5186d148 100644 --- a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc +++ b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc @@ -3412,6 +3412,75 @@ TEST(ArrowReadWrite, FixedSizeList) { CheckSimpleRoundtrip(table, 2, props_store_schema); } +TEST(ArrowReadWrite, FixedSizeListNull) { + using ::arrow::field; + using ::arrow::fixed_size_list; + + auto type = fixed_size_list(::arrow::int16(), /*size=*/3); + + const char* json = R"([ + null, + [1, 2, 3], + null, + [4, 5, 6], + null])"; + auto array = ::arrow::ArrayFromJSON(type, json); + auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); + auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); + CheckSimpleRoundtrip(table, 2, props_store_schema); +} + +TEST(ArrowReadWrite, FixedSizeListAllNull) { + using ::arrow::field; + using ::arrow::fixed_size_list; + + auto type = fixed_size_list(::arrow::int16(), /*size=*/3); + + const char* json = R"([ + null, + null, + null])"; + auto array = ::arrow::ArrayFromJSON(type, json); + auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); + auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); + CheckSimpleRoundtrip(table, 2, props_store_schema); +} + +TEST(ArrowReadWrite, NestedFixedSizeList) { + using ::arrow::field; + using ::arrow::fixed_size_list; + + auto type = fixed_size_list(fixed_size_list(::arrow::int16(), 2), /*size=*/2); + + const char* json = R"([ + [[1, 2], [3, 4]], + null, + [[5, 6], null], + [null, [7, 8]]])"; + auto array = ::arrow::ArrayFromJSON(type, json); + auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); + auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); + CheckSimpleRoundtrip(table, 2, props_store_schema); +} + +TEST(ArrowReadWrite, ListOfFixedSizeList) { + using ::arrow::field; + using ::arrow::fixed_size_list; + using ::arrow::list; + + auto type = list(fixed_size_list(::arrow::int16(), 2)); + + const char* json = R"([ + [[1, 2], null, [3, 4]], + null, + [null, [5, 6]], + []])"; + auto array = ::arrow::ArrayFromJSON(type, json); + auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); + auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); + CheckSimpleRoundtrip(table, 2, props_store_schema); +} + TEST(ArrowReadWrite, ListOfStructOfList2) { using ::arrow::field; using ::arrow::list; diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc index cc107c1802e3..0906423e38ac 100644 --- a/cpp/src/parquet/arrow/reader.cc +++ b/cpp/src/parquet/arrow/reader.cc @@ -19,13 +19,15 @@ #include #include +#include #include -#include +#include #include #include #include -#include "arrow/array.h" +#include "arrow/array.h" // IWYU pragma: keep +#include "arrow/array/concatenate.h" #include "arrow/buffer.h" #include "arrow/extension_type.h" #include "arrow/io/memory.h" @@ -35,6 +37,7 @@ #include "arrow/type.h" #include "arrow/type_traits.h" #include "arrow/util/async_generator.h" +#include "arrow/util/bit_run_reader.h" #include "arrow/util/bit_util.h" #include "arrow/util/future.h" #include "arrow/util/iterator.h" @@ -45,13 +48,10 @@ #include "arrow/util/type_traits.h" #include "parquet/arrow/reader_internal.h" -#include "parquet/bloom_filter.h" -#include "parquet/bloom_filter_reader.h" #include "parquet/column_reader.h" #include "parquet/exception.h" #include "parquet/file_reader.h" #include "parquet/metadata.h" -#include "parquet/page_index.h" #include "parquet/properties.h" #include "parquet/schema.h" @@ -725,13 +725,66 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { DCHECK_EQ(data->buffers.size(), 2); DCHECK_EQ(field()->type()->id(), ::arrow::Type::FIXED_SIZE_LIST); const auto& type = checked_cast<::arrow::FixedSizeListType&>(*field()->type()); - const int32_t* offsets = reinterpret_cast(data->buffers[1]->data()); - for (int x = 1; x <= data->length; x++) { - int32_t size = offsets[x] - offsets[x - 1]; - if (size != type.list_size()) { - return Status::Invalid("Expected all lists to be of size=", type.list_size(), - " but index ", x, " had size=", size); + const auto* offsets = reinterpret_cast(data->buffers[1]->data()); + const int32_t list_size = type.list_size(); + const uint8_t* valid_bits = + data->buffers[0] != nullptr ? data->buffers[0]->data() : nullptr; + auto validate_offsets = [&](int64_t start, int64_t length, + int32_t expected_size) -> Status { + std::span run_offsets(offsets + start, + static_cast(length + 1)); + const auto first_invalid_offset = std::ranges::adjacent_find( + run_offsets, + [&](int32_t left, int32_t right) { return right - left != expected_size; }); + if (first_invalid_offset != run_offsets.end()) { + const int64_t x = + start + std::ranges::distance(run_offsets.begin(), first_invalid_offset); + return Status::Invalid("Expected offset at index ", x + 1, " to be ", + offsets[x] + expected_size, " but got ", offsets[x + 1]); } + return Status::OK(); + }; + if (valid_bits != nullptr) { + bool needs_padding = false; + ::arrow::ArrayVector child_arrays; + + auto append_child_run = [&](int64_t start, int64_t length, bool valid) -> Status { + const int64_t child_length = length * list_size; + if (!valid) { + ARROW_ASSIGN_OR_RAISE( + auto null_array, + ::arrow::MakeArrayOfNull(type.value_type(), child_length, ctx_->pool)); + child_arrays.push_back(std::move(null_array)); + return Status::OK(); + } + child_arrays.push_back( + ::arrow::MakeArray(data->child_data[0]->Slice(offsets[start], child_length))); + return Status::OK(); + }; + + auto visit_run = [&](int64_t start, int64_t length, bool valid) -> Status { + RETURN_NOT_OK(validate_offsets(start, length, valid ? list_size : 0)); + if (valid && !needs_padding) { + return Status::OK(); + } + if (!needs_padding) { + needs_padding = true; + if (start > 0) { + RETURN_NOT_OK(append_child_run(/*start=*/0, start, /*valid=*/true)); + } + } + return append_child_run(start, length, valid); + }; + + RETURN_NOT_OK(::arrow::internal::VisitBitRuns(valid_bits, data->offset, + data->length, visit_run)); + if (needs_padding) { + ARROW_ASSIGN_OR_RAISE(auto child_array_with_padding, + ::arrow::Concatenate(child_arrays, ctx_->pool)); + data->child_data[0] = child_array_with_padding->data(); + } + } else { + RETURN_NOT_OK(validate_offsets(/*start=*/0, data->length, list_size)); } data->buffers.resize(1); std::shared_ptr result = ::arrow::MakeArray(data); From fb4a9f79d8e2d42753ec165ad5eabcf2cf5257dc Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Thu, 9 Jul 2026 17:41:56 +0800 Subject: [PATCH 2/5] address review --- cpp/src/parquet/arrow/reader.cc | 55 ++++++++++++++------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc index 0906423e38ac..2b5dd3712422 100644 --- a/cpp/src/parquet/arrow/reader.cc +++ b/cpp/src/parquet/arrow/reader.cc @@ -727,10 +727,8 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { const auto& type = checked_cast<::arrow::FixedSizeListType&>(*field()->type()); const auto* offsets = reinterpret_cast(data->buffers[1]->data()); const int32_t list_size = type.list_size(); - const uint8_t* valid_bits = - data->buffers[0] != nullptr ? data->buffers[0]->data() : nullptr; - auto validate_offsets = [&](int64_t start, int64_t length, - int32_t expected_size) -> Status { + auto validate_offsets = [&](int64_t start, int64_t length, bool valid) -> Status { + const int32_t expected_size = valid ? list_size : 0; std::span run_offsets(offsets + start, static_cast(length + 1)); const auto first_invalid_offset = std::ranges::adjacent_find( @@ -739,17 +737,27 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { if (first_invalid_offset != run_offsets.end()) { const int64_t x = start + std::ranges::distance(run_offsets.begin(), first_invalid_offset); - return Status::Invalid("Expected offset at index ", x + 1, " to be ", - offsets[x] + expected_size, " but got ", offsets[x + 1]); + const int32_t size = offsets[x + 1] - offsets[x]; + if (valid) { + return Status::Invalid("Expected all lists to be of size=", list_size, + " but index ", x + 1, " had size=", size); + } + return Status::Invalid("Expected null fixed-size list at index ", x + 1, + " to have no child values but had size=", size); } return Status::OK(); }; - if (valid_bits != nullptr) { - bool needs_padding = false; + if (data->GetNullCount() != 0) { + // Rebuild the child array run-by-run so null fixed-size list slots still + // contribute list_size child values in the final layout. ::arrow::ArrayVector child_arrays; - auto append_child_run = [&](int64_t start, int64_t length, bool valid) -> Status { + auto visit_run = [&](int64_t start, int64_t length, bool valid) -> Status { + RETURN_NOT_OK(validate_offsets(start, length, valid)); + const int64_t child_length = length * list_size; + // Valid runs reuse the decoded child slice; null runs materialize null + // children to preserve the fixed-size list shape. if (!valid) { ARROW_ASSIGN_OR_RAISE( auto null_array, @@ -762,29 +770,14 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { return Status::OK(); }; - auto visit_run = [&](int64_t start, int64_t length, bool valid) -> Status { - RETURN_NOT_OK(validate_offsets(start, length, valid ? list_size : 0)); - if (valid && !needs_padding) { - return Status::OK(); - } - if (!needs_padding) { - needs_padding = true; - if (start > 0) { - RETURN_NOT_OK(append_child_run(/*start=*/0, start, /*valid=*/true)); - } - } - return append_child_run(start, length, valid); - }; - - RETURN_NOT_OK(::arrow::internal::VisitBitRuns(valid_bits, data->offset, - data->length, visit_run)); - if (needs_padding) { - ARROW_ASSIGN_OR_RAISE(auto child_array_with_padding, - ::arrow::Concatenate(child_arrays, ctx_->pool)); - data->child_data[0] = child_array_with_padding->data(); - } + DCHECK_NE(data->buffers[0], nullptr); + RETURN_NOT_OK(::arrow::internal::VisitBitRuns( + data->buffers[0]->data(), data->offset, data->length, visit_run)); + ARROW_ASSIGN_OR_RAISE(auto child_array_with_padding, + ::arrow::Concatenate(child_arrays, ctx_->pool)); + data->child_data[0] = child_array_with_padding->data(); } else { - RETURN_NOT_OK(validate_offsets(/*start=*/0, data->length, list_size)); + RETURN_NOT_OK(validate_offsets(/*start=*/0, data->length, /*valid=*/true)); } data->buffers.resize(1); std::shared_ptr result = ::arrow::MakeArray(data); From bbecb76d9489a029260d4ded3c11233324dcee8b Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Tue, 21 Jul 2026 19:00:38 +0800 Subject: [PATCH 3/5] address review --- cpp/src/parquet/arrow/reader.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc index 2b5dd3712422..7bc5727128b3 100644 --- a/cpp/src/parquet/arrow/reader.cc +++ b/cpp/src/parquet/arrow/reader.cc @@ -727,8 +727,9 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { const auto& type = checked_cast<::arrow::FixedSizeListType&>(*field()->type()); const auto* offsets = reinterpret_cast(data->buffers[1]->data()); const int32_t list_size = type.list_size(); - auto validate_offsets = [&](int64_t start, int64_t length, bool valid) -> Status { - const int32_t expected_size = valid ? list_size : 0; + auto validate_offsets = [&](int64_t start, int64_t length, + bool has_elements) -> Status { + const int32_t expected_size = has_elements ? list_size : 0; std::span run_offsets(offsets + start, static_cast(length + 1)); const auto first_invalid_offset = std::ranges::adjacent_find( @@ -738,7 +739,7 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { const int64_t x = start + std::ranges::distance(run_offsets.begin(), first_invalid_offset); const int32_t size = offsets[x + 1] - offsets[x]; - if (valid) { + if (has_elements) { return Status::Invalid("Expected all lists to be of size=", list_size, " but index ", x + 1, " had size=", size); } @@ -752,13 +753,13 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { // contribute list_size child values in the final layout. ::arrow::ArrayVector child_arrays; - auto visit_run = [&](int64_t start, int64_t length, bool valid) -> Status { - RETURN_NOT_OK(validate_offsets(start, length, valid)); + auto visit_run = [&](int64_t start, int64_t length, bool has_elements) -> Status { + RETURN_NOT_OK(validate_offsets(start, length, has_elements)); const int64_t child_length = length * list_size; // Valid runs reuse the decoded child slice; null runs materialize null // children to preserve the fixed-size list shape. - if (!valid) { + if (!has_elements) { ARROW_ASSIGN_OR_RAISE( auto null_array, ::arrow::MakeArrayOfNull(type.value_type(), child_length, ctx_->pool)); From f8cfd46095801b70737a35c3a39e73a81d4f15e0 Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Thu, 23 Jul 2026 00:08:44 +0800 Subject: [PATCH 4/5] address review --- cpp/src/parquet/arrow/reader.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc index 7bc5727128b3..9212f0abb6ce 100644 --- a/cpp/src/parquet/arrow/reader.cc +++ b/cpp/src/parquet/arrow/reader.cc @@ -774,6 +774,9 @@ class PARQUET_NO_EXPORT FixedSizeListReader : public ListReader { DCHECK_NE(data->buffers[0], nullptr); RETURN_NOT_OK(::arrow::internal::VisitBitRuns( data->buffers[0]->data(), data->offset, data->length, visit_run)); + + // TODO(GH-50271): Build one padded child array directly instead of creating + // one temporary Array/ArrayData per validity run and concatenating them. ARROW_ASSIGN_OR_RAISE(auto child_array_with_padding, ::arrow::Concatenate(child_arrays, ctx_->pool)); data->child_data[0] = child_array_with_padding->data(); From 6b0bd58fefaa1fafe161d9e78f54ea339390d5a2 Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Thu, 23 Jul 2026 10:56:52 +0800 Subject: [PATCH 5/5] address review --- .../parquet/arrow/arrow_reader_writer_test.cc | 123 ++++++------------ 1 file changed, 43 insertions(+), 80 deletions(-) diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc index b7ac5186d148..3bee0e79283c 100644 --- a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc +++ b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc @@ -3395,91 +3395,54 @@ TEST(ArrowReadWrite, EmptyListView) { ASSERT_EQ(0, list_view.value_sizes()->size()); } -TEST(ArrowReadWrite, FixedSizeList) { - using ::arrow::field; - using ::arrow::fixed_size_list; - using ::arrow::struct_; - - auto type = fixed_size_list(::arrow::int16(), /*size=*/3); - - const char* json = R"([ - [1, 2, 3], - [4, 5, 6], - [7, 8, 9]])"; - auto array = ::arrow::ArrayFromJSON(type, json); - auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); - auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); - CheckSimpleRoundtrip(table, 2, props_store_schema); -} - -TEST(ArrowReadWrite, FixedSizeListNull) { - using ::arrow::field; - using ::arrow::fixed_size_list; - - auto type = fixed_size_list(::arrow::int16(), /*size=*/3); - - const char* json = R"([ - null, - [1, 2, 3], - null, - [4, 5, 6], - null])"; - auto array = ::arrow::ArrayFromJSON(type, json); - auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); - auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); - CheckSimpleRoundtrip(table, 2, props_store_schema); -} - -TEST(ArrowReadWrite, FixedSizeListAllNull) { - using ::arrow::field; - using ::arrow::fixed_size_list; - - auto type = fixed_size_list(::arrow::int16(), /*size=*/3); +struct FixedSizeListTestCase { + std::shared_ptr type; + std::string json; +}; - const char* json = R"([ - null, - null, - null])"; - auto array = ::arrow::ArrayFromJSON(type, json); - auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); +class TestFixedSizeListRoundTrip + : public ::testing::TestWithParam {}; + +static const std::vector kFixedSizeListTestCases = { + {.type = ::arrow::fixed_size_list(::arrow::int16(), /*list_size=*/3), .json = R"([ + {"root": [1, 2, 3]}, + {"root": [4, 5, 6]}, + {"root": [7, 8, 9]}])"}, + {.type = ::arrow::fixed_size_list(::arrow::int16(), /*list_size=*/3), .json = R"([ + {"root": null}, + {"root": [1, 2, 3]}, + {"root": null}, + {"root": [4, 5, 6]}, + {"root": null}])"}, + {.type = ::arrow::fixed_size_list(::arrow::int16(), /*list_size=*/3), .json = R"([ + {"root": null}, + {"root": null}, + {"root": null}])"}, + {.type = ::arrow::fixed_size_list( + ::arrow::fixed_size_list(::arrow::int16(), /*list_size=*/2), + /*list_size=*/2), + .json = R"([ + {"root": [[1, 2], [3, 4]]}, + {"root": null}, + {"root": [[5, 6], null]}, + {"root": [null, [7, 8]]}])"}, + {.type = ::arrow::list(::arrow::fixed_size_list(::arrow::int16(), /*list_size=*/2)), + .json = R"([ + {"root": [[1, 2], null, [3, 4]]}, + {"root": null}, + {"root": [null, [5, 6]]}, + {"root": []}])"}}; + +TEST_P(TestFixedSizeListRoundTrip, RoundTrip) { + const auto& test_case = GetParam(); + auto table = ::arrow::TableFromJSON( + ::arrow::schema({::arrow::field("root", test_case.type)}), {test_case.json}); auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); CheckSimpleRoundtrip(table, 2, props_store_schema); } -TEST(ArrowReadWrite, NestedFixedSizeList) { - using ::arrow::field; - using ::arrow::fixed_size_list; - - auto type = fixed_size_list(fixed_size_list(::arrow::int16(), 2), /*size=*/2); - - const char* json = R"([ - [[1, 2], [3, 4]], - null, - [[5, 6], null], - [null, [7, 8]]])"; - auto array = ::arrow::ArrayFromJSON(type, json); - auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); - auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); - CheckSimpleRoundtrip(table, 2, props_store_schema); -} - -TEST(ArrowReadWrite, ListOfFixedSizeList) { - using ::arrow::field; - using ::arrow::fixed_size_list; - using ::arrow::list; - - auto type = list(fixed_size_list(::arrow::int16(), 2)); - - const char* json = R"([ - [[1, 2], null, [3, 4]], - null, - [null, [5, 6]], - []])"; - auto array = ::arrow::ArrayFromJSON(type, json); - auto table = ::arrow::Table::Make(::arrow::schema({field("root", type)}), {array}); - auto props_store_schema = ArrowWriterProperties::Builder().store_schema()->build(); - CheckSimpleRoundtrip(table, 2, props_store_schema); -} +INSTANTIATE_TEST_SUITE_P(ArrowReadWrite, TestFixedSizeListRoundTrip, + ::testing::ValuesIn(kFixedSizeListTestCases)); TEST(ArrowReadWrite, ListOfStructOfList2) { using ::arrow::field;