From 70625802015f278812b67a68a43928d7408b2fa6 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Tue, 18 Aug 2026 23:58:04 +0000 Subject: [PATCH 1/2] Tolerate unrecognized logical/physical type combinations when reading --- cpp/src/parquet/arrow/arrow_schema_test.cc | 26 ++++++++++++++++++++++ cpp/src/parquet/schema.cc | 15 +++++++++---- cpp/submodules/parquet-testing | 2 +- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/cpp/src/parquet/arrow/arrow_schema_test.cc b/cpp/src/parquet/arrow/arrow_schema_test.cc index 894f68900280..2ecffef8720c 100644 --- a/cpp/src/parquet/arrow/arrow_schema_test.cc +++ b/cpp/src/parquet/arrow/arrow_schema_test.cc @@ -25,6 +25,7 @@ #include "parquet/arrow/reader.h" #include "parquet/arrow/reader_internal.h" #include "parquet/arrow/schema.h" +#include "parquet/column_reader.h" #include "parquet/file_reader.h" #include "parquet/schema.h" #include "parquet/schema_internal.h" @@ -2171,6 +2172,31 @@ TEST(TestFromParquetSchema, UndefinedLogicalType) { *::arrow::field("column with unknown type", ::arrow::binary())); } +TEST(TestFromParquetSchema, IncompatibleLogicalTypeDropped) { + // A file with INT32 annotated as UUID. The reader should succeed and ignore the logical type + // and stats. + auto path = test::get_data_file("int32_with_uuid_logical_type.parquet"); + std::unique_ptr reader = + parquet::ParquetFileReader::OpenFile(path); + + const auto* pq_schema = reader->metadata()->schema(); + ASSERT_EQ(pq_schema->num_columns(), 1); + + const auto* col_desc = pq_schema->Column(0); + ASSERT_EQ(col_desc->physical_type(), parquet::Type::INT32); + ASSERT_FALSE(col_desc->logical_type()->is_valid()); + ASSERT_FALSE(col_desc->can_use_min_max()); + + auto row_group = reader->RowGroup(0); + auto col_reader = std::static_pointer_cast(row_group->Column(0)); + const auto num_rows = 10; + std::vector values(num_rows); + int64_t values_read = 0; + col_reader->ReadBatch(num_rows, nullptr, nullptr, values.data(), &values_read); + ASSERT_EQ(values_read, num_rows); + for (int32_t i = 0; i < num_rows; ++i) ASSERT_EQ(values[i], i); +} + // // Test LevelInfo computation from a Parquet schema // (for Parquet -> Arrow reading). diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index 0cfa49c21c16..f79bcf4cf171 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -453,10 +453,17 @@ std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { std::unique_ptr primitive_node; if (element->__isset.logicalType) { // updated writer with logical type present - primitive_node = std::unique_ptr( - new PrimitiveNode(element->name, LoadEnumSafe(&element->repetition_type), - LogicalType::FromThrift(element->logicalType), - LoadEnumSafe(&element->type), element->type_length, field_id)); + auto physical_type = LoadEnumSafe(&element->type); + auto logical_type = LogicalType::FromThrift(element->logicalType); + // Tolerate unrecognized logical/physical type combinations by dropping the logical type + // annotation. + if (logical_type && !logical_type->is_nested() && + !logical_type->is_applicable(physical_type, element->type_length)) { + logical_type = UndefinedLogicalType::Make(); + } + primitive_node = std::unique_ptr(new PrimitiveNode( + element->name, LoadEnumSafe(&element->repetition_type), std::move(logical_type), + physical_type, element->type_length, field_id)); } else if (element->__isset.converted_type) { // legacy writer with converted type present primitive_node = std::unique_ptr(new PrimitiveNode( diff --git a/cpp/submodules/parquet-testing b/cpp/submodules/parquet-testing index e74785d85a4e..fd54fba57a48 160000 --- a/cpp/submodules/parquet-testing +++ b/cpp/submodules/parquet-testing @@ -1 +1 @@ -Subproject commit e74785d85a4ecee829e1e405444d6a1b24b8bc9c +Subproject commit fd54fba57a4854b9f6c8798286499ee34ce225fd From 0d05d364f06a08a2ffd853130108214918913999 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Thu, 20 Aug 2026 16:23:48 +0000 Subject: [PATCH 2/2] address comments --- cpp/src/parquet/arrow/arrow_schema_test.cc | 4 ++-- cpp/src/parquet/schema.cc | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cpp/src/parquet/arrow/arrow_schema_test.cc b/cpp/src/parquet/arrow/arrow_schema_test.cc index 2ecffef8720c..4285b3c424d0 100644 --- a/cpp/src/parquet/arrow/arrow_schema_test.cc +++ b/cpp/src/parquet/arrow/arrow_schema_test.cc @@ -2173,8 +2173,8 @@ TEST(TestFromParquetSchema, UndefinedLogicalType) { } TEST(TestFromParquetSchema, IncompatibleLogicalTypeDropped) { - // A file with INT32 annotated as UUID. The reader should succeed and ignore the logical type - // and stats. + // A file with INT32 annotated as UUID. The reader should succeed and ignore the logical + // type and stats. auto path = test::get_data_file("int32_with_uuid_logical_type.parquet"); std::unique_ptr reader = parquet::ParquetFileReader::OpenFile(path); diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index f79bcf4cf171..24cc59de7cfd 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -455,10 +455,14 @@ std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { // updated writer with logical type present auto physical_type = LoadEnumSafe(&element->type); auto logical_type = LogicalType::FromThrift(element->logicalType); - // Tolerate unrecognized logical/physical type combinations by dropping the logical type - // annotation. - if (logical_type && !logical_type->is_nested() && + // Tolerate unrecognized logical/physical type combinations by dropping the logical + // type annotation. + if (logical_type && !logical_type->is_applicable(physical_type, element->type_length)) { + ARROW_LOG(WARNING) << "Dropping unsupported logical type " + << logical_type->ToString() << " on physical type " + << TypeToString(physical_type) << " for column '" + << element->name << "'"; logical_type = UndefinedLogicalType::Make(); } primitive_node = std::unique_ptr(new PrimitiveNode(