Skip to content

Commit 7fa1c4d

Browse files
committed
fix(parquet): align output schema to the reader's list types per field
Files that carry serialized ARROW:schema metadata keep their stored list types, so the reader may produce list, large_list, or a mix regardless of the requested type. The output schema, built from the Iceberg projection as plain list, is now rewritten per field to match the reader so the projection casts each array to the type it actually is. - The alignment is unconditional, so a stored large_list read with use_large_list=false no longer reports list while the array is large_list. - Each output field is correlated to the reader field through the projection (by field id), not by name, so a renamed column is still matched to the array it is read from. Nested struct fields recurse the same way; list and map elements are matched positionally. - Fields not read from the source (null or default) take the configured use_large_list preference at every level. Tests: store ARROW:schema via ArrowWriterProperties::store_schema(), assert the footer key is present in both helpers, and add a mixed list/large_list read that checks each field keeps its own list type.
1 parent 41c4b50 commit 7fa1c4d

2 files changed

Lines changed: 308 additions & 38 deletions

File tree

src/iceberg/parquet/parquet_reader.cc

Lines changed: 138 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
#include <algorithm>
2323
#include <numeric>
24+
#include <variant>
25+
#include <vector>
2426

2527
#include <arrow/c/bridge.h>
2628
#include <arrow/memory_pool.h>
@@ -122,40 +124,138 @@ std::shared_ptr<::arrow::Field> UseLargeListField(
122124
return field->WithType(UseLargeListType(field->type()));
123125
}
124126

125-
// Rewrite all fields in a field vector to use large_list instead of list.
126-
::arrow::FieldVector UseLargeListFields(const ::arrow::FieldVector& fields) {
127-
::arrow::FieldVector rewritten;
128-
rewritten.reserve(fields.size());
129-
for (const auto& field : fields) {
130-
rewritten.push_back(UseLargeListField(field));
127+
// Rebuild a type so its nested lists match the list type (list vs large_list) of the
128+
// arrays the reader produces, correlating struct fields to the reader by field id via the
129+
// projection rather than by name, so a renamed column is still matched to the array it is
130+
// read from. `projections` are the child projections of the field whose type this is.
131+
std::shared_ptr<::arrow::DataType> AlignTypeToReader(
132+
const std::shared_ptr<::arrow::DataType>& output_type,
133+
const std::shared_ptr<::arrow::DataType>& reader_type,
134+
const std::vector<FieldProjection>& projections, bool use_large_list_default);
135+
136+
// Rewrite the fields of a struct level (including the top level) so their list types
137+
// match the reader's. `projections[i].from` gives the reader field for output field `i`,
138+
// matching how ProjectStructArray reads the arrays. A field not projected from the source
139+
// (null, default, constant or metadata) is filled with an array of the output type, so it
140+
// takes the configured preference instead.
141+
::arrow::FieldVector AlignFieldsToReader(const ::arrow::FieldVector& output_fields,
142+
const ::arrow::FieldVector& reader_fields,
143+
const std::vector<FieldProjection>& projections,
144+
bool use_large_list_default) {
145+
::arrow::FieldVector aligned;
146+
aligned.reserve(output_fields.size());
147+
148+
for (size_t i = 0; i < output_fields.size(); ++i) {
149+
const auto& output_field = output_fields[i];
150+
// Defensive: the projection carries one entry per output field. If it does not line
151+
// up, leave the field untouched rather than risk an out-of-bounds access.
152+
if (i >= projections.size()) {
153+
aligned.push_back(output_field);
154+
continue;
155+
}
156+
157+
const auto& projection = projections[i];
158+
if (projection.kind == FieldProjection::Kind::kProjected) {
159+
auto reader_index = std::get<size_t>(projection.from);
160+
if (reader_index >= reader_fields.size()) {
161+
aligned.push_back(output_field);
162+
continue;
163+
}
164+
aligned.push_back(output_field->WithType(
165+
AlignTypeToReader(output_field->type(), reader_fields[reader_index]->type(),
166+
projection.children, use_large_list_default)));
167+
} else {
168+
aligned.push_back(use_large_list_default ? UseLargeListField(output_field)
169+
: output_field);
170+
}
131171
}
132-
return rewritten;
172+
173+
return aligned;
133174
}
134175

135-
// Returns true if the type contains a large_list, at any level of nesting.
136-
bool ContainsLargeList(const ::arrow::DataType& type) {
137-
if (type.id() == ::arrow::Type::LARGE_LIST) {
138-
return true;
176+
std::shared_ptr<::arrow::DataType> AlignTypeToReader(
177+
const std::shared_ptr<::arrow::DataType>& output_type,
178+
const std::shared_ptr<::arrow::DataType>& reader_type,
179+
const std::vector<FieldProjection>& projections, bool use_large_list_default) {
180+
switch (output_type->id()) {
181+
case ::arrow::Type::STRUCT: {
182+
if (reader_type->id() != ::arrow::Type::STRUCT) {
183+
return output_type;
184+
}
185+
const auto& output_struct =
186+
internal::checked_cast<const ::arrow::StructType&>(*output_type);
187+
const auto& reader_struct =
188+
internal::checked_cast<const ::arrow::StructType&>(*reader_type);
189+
return ::arrow::struct_(AlignFieldsToReader(output_struct.fields(),
190+
reader_struct.fields(), projections,
191+
use_large_list_default));
192+
}
193+
case ::arrow::Type::LIST: {
194+
// A list carries exactly one child projection, its element, matched positionally.
195+
if (projections.size() != 1) {
196+
return output_type;
197+
}
198+
const auto& output_list =
199+
internal::checked_cast<const ::arrow::ListType&>(*output_type);
200+
const auto& element = projections.front().children;
201+
if (reader_type->id() == ::arrow::Type::LARGE_LIST) {
202+
const auto& reader_list =
203+
internal::checked_cast<const ::arrow::LargeListType&>(*reader_type);
204+
return ::arrow::large_list(output_list.value_field()->WithType(AlignTypeToReader(
205+
output_list.value_field()->type(), reader_list.value_field()->type(), element,
206+
use_large_list_default)));
207+
}
208+
if (reader_type->id() == ::arrow::Type::LIST) {
209+
const auto& reader_list =
210+
internal::checked_cast<const ::arrow::ListType&>(*reader_type);
211+
return ::arrow::list(output_list.value_field()->WithType(AlignTypeToReader(
212+
output_list.value_field()->type(), reader_list.value_field()->type(), element,
213+
use_large_list_default)));
214+
}
215+
return output_type;
216+
}
217+
case ::arrow::Type::MAP: {
218+
// A map carries two child projections, its key and its value, matched positionally.
219+
if (reader_type->id() != ::arrow::Type::MAP || projections.size() != 2) {
220+
return output_type;
221+
}
222+
const auto& output_map =
223+
internal::checked_cast<const ::arrow::MapType&>(*output_type);
224+
const auto& reader_map =
225+
internal::checked_cast<const ::arrow::MapType&>(*reader_type);
226+
return std::make_shared<::arrow::MapType>(
227+
output_map.key_field()->WithType(AlignTypeToReader(
228+
output_map.key_field()->type(), reader_map.key_field()->type(),
229+
projections[0].children, use_large_list_default)),
230+
output_map.item_field()->WithType(AlignTypeToReader(
231+
output_map.item_field()->type(), reader_map.item_field()->type(),
232+
projections[1].children, use_large_list_default)),
233+
output_map.keys_sorted());
234+
}
235+
default:
236+
return output_type;
139237
}
140-
return std::ranges::any_of(
141-
type.fields(), [](const auto& field) { return ContainsLargeList(*field->type()); });
142238
}
143239

144-
// Returns true if the reader produces large_list arrays.
145-
//
146-
// Arrow honors the requested large_list type only when it derives the Arrow schema from
147-
// the Parquet schema. A file that carries serialized ARROW:schema metadata keeps its
148-
// original list type instead, so whether large lists are produced can only be told from
149-
// the schema of the reader.
150-
bool ProducesLargeList(const ::arrow::RecordBatchReader& reader) {
151-
const auto& schema = reader.schema();
152-
if (schema == nullptr) {
153-
// an empty reader produces no arrays to be described
154-
return false;
240+
// Align the output schema to the arrays the reader actually produces. Arrow honors the
241+
// requested large_list type only when it derives the schema from the Parquet schema; a
242+
// file that carries serialized ARROW:schema metadata keeps its stored list types, so the
243+
// reader may produce list, large_list, or a mix of the two. The output schema, built from
244+
// the Iceberg projection and always using plain list, is rewritten per field to match the
245+
// reader so that ProjectRecordBatch casts each array to the type it actually is. Fields
246+
// are correlated to the reader through the projection (by field id), never by name.
247+
std::shared_ptr<::arrow::Schema> AlignOutputSchemaToReaderSchema(
248+
const std::shared_ptr<::arrow::Schema>& output_schema,
249+
const std::shared_ptr<::arrow::Schema>& reader_schema,
250+
const SchemaProjection& projection, bool use_large_list_default) {
251+
if (reader_schema == nullptr || output_schema == nullptr) {
252+
return output_schema;
155253
}
156-
return std::ranges::any_of(schema->fields(), [](const auto& field) {
157-
return ContainsLargeList(*field->type());
158-
});
254+
255+
return ::arrow::schema(
256+
AlignFieldsToReader(output_schema->fields(), reader_schema->fields(),
257+
projection.fields, use_large_list_default),
258+
output_schema->metadata());
159259
}
160260

161261
} // namespace
@@ -330,15 +430,17 @@ class ParquetReader::Impl {
330430
ICEBERG_ARROW_ASSIGN_OR_RETURN(context_->output_arrow_schema_,
331431
::arrow::ImportSchema(&arrow_schema));
332432

333-
if (use_large_list_ && ProducesLargeList(*context_->record_batch_reader_)) {
334-
// Align the output schema with the large_list arrays produced by the Parquet
335-
// reader. Note that Arrow ignores the requested list type when the file carries
336-
// serialized ARROW:schema metadata, in which case the reader keeps producing plain
337-
// list arrays and the output schema must keep describing them as such.
338-
context_->output_arrow_schema_ =
339-
::arrow::schema(UseLargeListFields(context_->output_arrow_schema_->fields()),
340-
context_->output_arrow_schema_->metadata());
341-
}
433+
// Align the output schema with the arrays the reader actually produces. The reader's
434+
// schema determines the actual list types (list vs large_list) for each field, which
435+
// may differ from the desired output due to:
436+
// 1. The reader's requested list type (set via set_list_type)
437+
// 2. ARROW:schema metadata in the file that overrides the Parquet schema type
438+
// 3. Mixed list and large_list types in files with stored schemas
439+
// For each projected field, we use the reader's actual type. For missing fields
440+
// (columns not in the file), we apply the configured use_large_list preference.
441+
context_->output_arrow_schema_ = AlignOutputSchemaToReaderSchema(
442+
context_->output_arrow_schema_, context_->record_batch_reader_->schema(),
443+
projection_, use_large_list_);
342444

343445
return {};
344446
}

src/iceberg/test/parquet_test.cc

Lines changed: 170 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,75 @@ class ParquetReaderTest : public TempFileTestBase {
319319
auto io = internal::checked_cast<arrow::ArrowFileSystemFileIO&>(*file_io_);
320320
auto outfile = io.fs()->OpenOutputStream(temp_parquet_file_).ValueOrDie();
321321

322+
// Build ArrowWriterProperties to store the Arrow schema in ARROW:schema metadata
323+
auto arrow_writer_props =
324+
::parquet::ArrowWriterProperties::Builder().store_schema()->build();
325+
322326
// write a single row group so that one batch holds every row
323-
ASSERT_TRUE(::parquet::arrow::WriteTable(*table, ::arrow::default_memory_pool(),
324-
outfile, table->num_rows())
327+
ASSERT_TRUE(::parquet::arrow::WriteTable(
328+
*table, ::arrow::default_memory_pool(), outfile, table->num_rows(),
329+
::parquet::default_writer_properties(), arrow_writer_props)
330+
.ok());
331+
ASSERT_TRUE(outfile->Close().ok());
332+
333+
// Verify ARROW:schema is stored
334+
auto input_file = io.fs()->OpenInputFile(temp_parquet_file_).ValueOrDie();
335+
auto metadata = ::parquet::ReadMetaData(input_file);
336+
const auto& kv_metadata = metadata->key_value_metadata();
337+
ASSERT_TRUE(kv_metadata != nullptr);
338+
ASSERT_TRUE(kv_metadata->FindKey("ARROW:schema") >= 0)
339+
<< "ARROW:schema not found in file metadata";
340+
}
341+
342+
// Writes a mixed list/large_list parquet file with stored ARROW:schema.
343+
void CreateMixedListParquetFileWithArrowSchema() {
344+
const std::string kParquetFieldIdKey = "PARQUET:field_id";
345+
auto arrow_schema = ::arrow::schema(
346+
{::arrow::field("id", ::arrow::int32(), /*nullable=*/false,
347+
::arrow::KeyValueMetadata::Make({kParquetFieldIdKey}, {"1"})),
348+
::arrow::field(
349+
"small_lists",
350+
::arrow::list(::arrow::field(
351+
"element", ::arrow::int32(), /*nullable=*/true,
352+
::arrow::KeyValueMetadata::Make({kParquetFieldIdKey}, {"101"}))),
353+
/*nullable=*/true,
354+
::arrow::KeyValueMetadata::Make({kParquetFieldIdKey}, {"2"})),
355+
::arrow::field(
356+
"large_lists",
357+
::arrow::large_list(::arrow::field(
358+
"element", ::arrow::int32(), /*nullable=*/true,
359+
::arrow::KeyValueMetadata::Make({kParquetFieldIdKey}, {"102"}))),
360+
/*nullable=*/true,
361+
::arrow::KeyValueMetadata::Make({kParquetFieldIdKey}, {"3"}))});
362+
auto batch = ::arrow::RecordBatch::FromStructArray(
363+
::arrow::json::ArrayFromJSONString(
364+
::arrow::struct_(arrow_schema->fields()),
365+
R"([[1, [10, 20], [100, 200]], [2, [30], [300]]])")
366+
.ValueOrDie())
367+
.ValueOrDie();
368+
auto table = ::arrow::Table::FromRecordBatches(arrow_schema, {batch}).ValueOrDie();
369+
370+
auto io = internal::checked_cast<arrow::ArrowFileSystemFileIO&>(*file_io_);
371+
auto outfile = io.fs()->OpenOutputStream(temp_parquet_file_).ValueOrDie();
372+
373+
// Build ArrowWriterProperties to store the Arrow schema in ARROW:schema metadata
374+
auto arrow_writer_props =
375+
::parquet::ArrowWriterProperties::Builder().store_schema()->build();
376+
377+
// write a single row group so that one batch holds every row
378+
ASSERT_TRUE(::parquet::arrow::WriteTable(
379+
*table, ::arrow::default_memory_pool(), outfile, table->num_rows(),
380+
::parquet::default_writer_properties(), arrow_writer_props)
325381
.ok());
326382
ASSERT_TRUE(outfile->Close().ok());
383+
384+
// Verify ARROW:schema is stored
385+
auto input_file = io.fs()->OpenInputFile(temp_parquet_file_).ValueOrDie();
386+
auto metadata = ::parquet::ReadMetaData(input_file);
387+
const auto& kv_metadata = metadata->key_value_metadata();
388+
ASSERT_TRUE(kv_metadata != nullptr);
389+
ASSERT_TRUE(kv_metadata->FindKey("ARROW:schema") >= 0)
390+
<< "ARROW:schema not found in file metadata";
327391
}
328392

329393
void VerifyNextBatch(Reader& reader, std::string_view expected_json) {
@@ -655,6 +719,110 @@ TEST_F(ParquetReaderTest, ReadListAsLargeListWithArrowSchema) {
655719
}
656720
}
657721

722+
TEST_F(ParquetReaderTest, ReadMixedListAndLargeListWithArrowSchema) {
723+
// Reading a file with both list and large_list fields (stored via ARROW:schema
724+
// metadata) must report an output schema that describes the actual types of each field,
725+
// preserving the per-field list type. This tests the per-field mapping logic.
726+
CreateMixedListParquetFileWithArrowSchema();
727+
728+
auto schema = std::make_shared<Schema>(std::vector<SchemaField>{
729+
SchemaField::MakeRequired(1, "id", int32()),
730+
SchemaField::MakeOptional(2, "small_lists",
731+
std::make_shared<ListType>(SchemaField::MakeOptional(
732+
/*field_id=*/101, "element", int32()))),
733+
SchemaField::MakeOptional(3, "large_lists",
734+
std::make_shared<ListType>(SchemaField::MakeOptional(
735+
/*field_id=*/102, "element", int32())))});
736+
737+
// Read with default setting (use_large_list=false)
738+
auto reader_result = ReaderFactoryRegistry::Open(
739+
FileFormatType::kParquet,
740+
{.path = temp_parquet_file_, .io = file_io_, .projection = schema});
741+
ASSERT_THAT(reader_result, IsOk());
742+
auto reader = std::move(reader_result.value());
743+
744+
// Verify the output schema has the correct per-field list types
745+
auto schema_result = reader->Schema();
746+
ASSERT_THAT(schema_result, IsOk());
747+
auto arrow_c_schema = std::move(schema_result.value());
748+
auto arrow_type = ::arrow::ImportType(&arrow_c_schema).ValueOrDie();
749+
750+
// small_lists should be LIST (as stored in the file)
751+
ASSERT_EQ(arrow_type->field(1)->type()->id(), ::arrow::Type::LIST)
752+
<< "small_lists field should be LIST";
753+
754+
// large_lists should be LARGE_LIST (as stored in the file)
755+
ASSERT_EQ(arrow_type->field(2)->type()->id(), ::arrow::Type::LARGE_LIST)
756+
<< "large_lists field should be LARGE_LIST";
757+
758+
// Verify the arrays are readable with the reported schema
759+
auto data = reader->Next();
760+
ASSERT_THAT(data, IsOk());
761+
ASSERT_TRUE(data.value().has_value());
762+
auto arrow_c_array = data.value().value();
763+
764+
auto import_result = ::arrow::ImportArray(&arrow_c_array, arrow_type);
765+
ASSERT_TRUE(import_result.ok()) << import_result.status().ToString();
766+
auto arrow_array = import_result.ValueOrDie();
767+
ASSERT_TRUE(arrow_array->ValidateFull().ok());
768+
769+
const auto& struct_array =
770+
internal::checked_cast<const ::arrow::StructArray&>(*arrow_array);
771+
ASSERT_EQ(struct_array.length(), 2);
772+
773+
// Verify the field types in the actual arrays
774+
ASSERT_EQ(struct_array.field(1)->type()->id(), ::arrow::Type::LIST);
775+
ASSERT_EQ(struct_array.field(2)->type()->id(), ::arrow::Type::LARGE_LIST);
776+
777+
ASSERT_NO_FATAL_FAILURE(VerifyExhausted(*reader));
778+
}
779+
780+
TEST_F(ParquetReaderTest, ReadRenamedLargeListColumnWithArrowSchema) {
781+
// The projected column has a different name than the file but the same field id. The
782+
// output schema must be aligned to the reader by field id, not by name: the file stores
783+
// this column as large_list (via ARROW:schema), so a name-based match would miss the
784+
// rename, report the column as list while the array is large_list, and fail to import
785+
// it.
786+
CreateMixedListParquetFileWithArrowSchema();
787+
788+
// field id 3 is stored in the file as a large_list named "large_lists"; project it
789+
// under a different name to force matching by field id
790+
auto schema = std::make_shared<Schema>(std::vector<SchemaField>{
791+
SchemaField::MakeOptional(3, "renamed",
792+
std::make_shared<ListType>(SchemaField::MakeOptional(
793+
/*field_id=*/102, "element", int32())))});
794+
795+
// read with the default use_large_list=false, so only field-id matching can preserve
796+
// large_list
797+
auto reader_result = ReaderFactoryRegistry::Open(
798+
FileFormatType::kParquet,
799+
{.path = temp_parquet_file_, .io = file_io_, .projection = schema});
800+
ASSERT_THAT(reader_result, IsOk());
801+
auto reader = std::move(reader_result.value());
802+
803+
auto schema_result = reader->Schema();
804+
ASSERT_THAT(schema_result, IsOk());
805+
auto arrow_c_schema = std::move(schema_result.value());
806+
auto arrow_type = ::arrow::ImportType(&arrow_c_schema).ValueOrDie();
807+
808+
// the renamed column keeps the file's large_list type because it is matched by field id
809+
ASSERT_EQ(arrow_type->field(0)->type()->id(), ::arrow::Type::LARGE_LIST)
810+
<< "renamed column must keep the file's large_list type, matched by field id";
811+
812+
// importing the produced array against the reported schema must succeed; a name-based
813+
// mismatch would report list here while the array is large_list and this import would
814+
// fail
815+
auto data = reader->Next();
816+
ASSERT_THAT(data, IsOk());
817+
ASSERT_TRUE(data.value().has_value());
818+
auto arrow_c_array = data.value().value();
819+
auto import_result = ::arrow::ImportArray(&arrow_c_array, arrow_type);
820+
ASSERT_TRUE(import_result.ok()) << import_result.status().ToString();
821+
ASSERT_TRUE(import_result.ValueOrDie()->ValidateFull().ok());
822+
823+
ASSERT_NO_FATAL_FAILURE(VerifyExhausted(*reader));
824+
}
825+
658826
TEST_F(ParquetReaderTest, ReadSplit) {
659827
CreateSplitParquetFile();
660828

0 commit comments

Comments
 (0)