From 4050266d5550ea5203f4d5d55b3acd03aa996b15 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Tue, 18 Aug 2026 23:58:04 +0000 Subject: [PATCH 1/8] 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 27c302fe0d4..c09d309a2cb 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" @@ -2173,6 +2174,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 3cb91f9a84e..82eb5f544c3 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -454,10 +454,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 e74785d85a4..fd54fba57a4 160000 --- a/cpp/submodules/parquet-testing +++ b/cpp/submodules/parquet-testing @@ -1 +1 @@ -Subproject commit e74785d85a4ecee829e1e405444d6a1b24b8bc9c +Subproject commit fd54fba57a4854b9f6c8798286499ee34ce225fd From 24e0931febbd3a14455e21dd2050de4948e55dec Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Thu, 20 Aug 2026 16:23:48 +0000 Subject: [PATCH 2/8] 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 c09d309a2cb..014aafec7d3 100644 --- a/cpp/src/parquet/arrow/arrow_schema_test.cc +++ b/cpp/src/parquet/arrow/arrow_schema_test.cc @@ -2175,8 +2175,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 82eb5f544c3..cede3e425ab 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -456,10 +456,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( From 470a0d64b45fd84b9028c02ae3515029c651ad17 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Fri, 25 Sep 2026 06:54:52 +0000 Subject: [PATCH 3/8] Identify unsupported Parquet types by full column path Co-authored-by: Isaac --- cpp/src/parquet/schema.cc | 35 ++++++++++++++++++++++++++++++++-- cpp/src/parquet/schema.h | 11 +++++++++++ cpp/submodules/parquet-testing | 2 +- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index cede3e425ab..a4f20b1008a 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -51,6 +51,29 @@ void CheckColumnBounds(int column_index, size_t max_columns) { } } +std::string ColumnPathFromParquet(const SchemaElement* schema, + const SchemaElement* element) { + if (schema == nullptr) return element->name; + + std::vector> parents = { + {schema, schema->num_children}}; + for (const auto* node = schema + 1; node < element; ++node) { + --parents.back().second; + if (node->num_children > 0) parents.emplace_back(node, node->num_children); + while (!parents.empty() && parents.back().second == 0) { + parents.pop_back(); + } + } + + std::string path; + for (size_t i = 1; i < parents.size(); ++i) { + path += parents[i].first->name; + path += '.'; + } + path += element->name; + return path; +} + } // namespace namespace schema { @@ -443,6 +466,11 @@ std::unique_ptr GroupNode::FromParquet(const void* opaque_element, } std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { + return FromParquet(opaque_element, nullptr); +} + +std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element, + const void* opaque_schema) { const format::SchemaElement* element = static_cast(opaque_element); @@ -463,7 +491,10 @@ std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { ARROW_LOG(WARNING) << "Dropping unsupported logical type " << logical_type->ToString() << " on physical type " << TypeToString(physical_type) << " for column '" - << element->name << "'"; + << ColumnPathFromParquet( + static_cast(opaque_schema), + element) + << "'"; logical_type = UndefinedLogicalType::Make(); } primitive_node = std::unique_ptr(new PrimitiveNode( @@ -586,7 +617,7 @@ std::unique_ptr Unflatten(std::span elements, if (element.num_children == 0 && element.__isset.type) { // Leaf (primitive) node: always has a type - return PrimitiveNode::FromParquet(opaque_element); + return PrimitiveNode::FromParquet(opaque_element, elements); } else { // Group node (may have 0 children, but cannot have a type) // Protect against denial-of-service through stack exhaustion when parsing diff --git a/cpp/src/parquet/schema.h b/cpp/src/parquet/schema.h index 65732603ea1..94e06cf84d9 100644 --- a/cpp/src/parquet/schema.h +++ b/cpp/src/parquet/schema.h @@ -36,6 +36,10 @@ namespace parquet { class SchemaDescriptor; +namespace format { +class SchemaElement; +} + namespace schema { class Node; @@ -237,6 +241,13 @@ class PARQUET_EXPORT PrimitiveNode : public Node { void VisitConst(ConstVisitor* visitor) const override; private: + PARQUET_EXPORT friend std::unique_ptr Unflatten( + const format::SchemaElement* elements, int length); + + // opaque_schema is the flattened schema containing opaque_element, or nullptr. + static std::unique_ptr FromParquet(const void* opaque_element, + const void* opaque_schema); + PrimitiveNode(const std::string& name, Repetition::type repetition, Type::type type, ConvertedType::type converted_type = ConvertedType::NONE, int length = -1, int precision = -1, int scale = -1, int field_id = -1); diff --git a/cpp/submodules/parquet-testing b/cpp/submodules/parquet-testing index fd54fba57a4..56653c437c8 160000 --- a/cpp/submodules/parquet-testing +++ b/cpp/submodules/parquet-testing @@ -1 +1 @@ -Subproject commit fd54fba57a4854b9f6c8798286499ee34ce225fd +Subproject commit 56653c437c8092f704a092d0d1d4e600124cd49f From 2780c4a543505436d6bebc811200e957eea9a9f6 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Fri, 25 Sep 2026 07:07:54 +0000 Subject: [PATCH 4/8] Simplify access to the schema parsing overload Co-authored-by: Isaac --- cpp/src/parquet/schema.h | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/cpp/src/parquet/schema.h b/cpp/src/parquet/schema.h index 94e06cf84d9..df4537914cb 100644 --- a/cpp/src/parquet/schema.h +++ b/cpp/src/parquet/schema.h @@ -36,10 +36,6 @@ namespace parquet { class SchemaDescriptor; -namespace format { -class SchemaElement; -} - namespace schema { class Node; @@ -203,6 +199,9 @@ using NodeVector = std::vector; class PARQUET_EXPORT PrimitiveNode : public Node { public: static std::unique_ptr FromParquet(const void* opaque_element); + // opaque_schema is the flattened schema containing opaque_element, or nullptr. + static std::unique_ptr FromParquet(const void* opaque_element, + const void* opaque_schema); // A field_id -1 (or any negative value) will be serialized as null in Thrift static inline NodePtr Make(const std::string& name, Repetition::type repetition, @@ -241,13 +240,6 @@ class PARQUET_EXPORT PrimitiveNode : public Node { void VisitConst(ConstVisitor* visitor) const override; private: - PARQUET_EXPORT friend std::unique_ptr Unflatten( - const format::SchemaElement* elements, int length); - - // opaque_schema is the flattened schema containing opaque_element, or nullptr. - static std::unique_ptr FromParquet(const void* opaque_element, - const void* opaque_schema); - PrimitiveNode(const std::string& name, Repetition::type repetition, Type::type type, ConvertedType::type converted_type = ConvertedType::NONE, int length = -1, int precision = -1, int scale = -1, int field_id = -1); From b136ffd39c5987892a822238bc43032b1b03c836 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Fri, 25 Sep 2026 19:27:57 +0000 Subject: [PATCH 5/8] Reuse schema traversal for diagnostic column paths Co-authored-by: Isaac --- cpp/src/parquet/schema.cc | 51 +++++++++++++++------------------------ cpp/src/parquet/schema.h | 4 +-- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index a4f20b1008a..ed7ff43eddd 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -51,29 +51,6 @@ void CheckColumnBounds(int column_index, size_t max_columns) { } } -std::string ColumnPathFromParquet(const SchemaElement* schema, - const SchemaElement* element) { - if (schema == nullptr) return element->name; - - std::vector> parents = { - {schema, schema->num_children}}; - for (const auto* node = schema + 1; node < element; ++node) { - --parents.back().second; - if (node->num_children > 0) parents.emplace_back(node, node->num_children); - while (!parents.empty() && parents.back().second == 0) { - parents.pop_back(); - } - } - - std::string path; - for (size_t i = 1; i < parents.size(); ++i) { - path += parents[i].first->name; - path += '.'; - } - path += element->name; - return path; -} - } // namespace namespace schema { @@ -465,12 +442,17 @@ std::unique_ptr GroupNode::FromParquet(const void* opaque_element, return std::unique_ptr(group_node.release()); } +struct SchemaPath { + const SchemaPath* parent; + const std::string& name; +}; + std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { return FromParquet(opaque_element, nullptr); } std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element, - const void* opaque_schema) { + const SchemaPath* parent_path) { const format::SchemaElement* element = static_cast(opaque_element); @@ -488,13 +470,16 @@ std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element, // type annotation. if (logical_type && !logical_type->is_applicable(physical_type, element->type_length)) { + std::vector path{element->name}; + for (auto* parent = parent_path; parent && parent->parent; + parent = parent->parent) { + path.push_back(parent->name); + } + std::reverse(path.begin(), path.end()); ARROW_LOG(WARNING) << "Dropping unsupported logical type " << logical_type->ToString() << " on physical type " << TypeToString(physical_type) << " for column '" - << ColumnPathFromParquet( - static_cast(opaque_schema), - element) - << "'"; + << ColumnPath(std::move(path)).ToDotString() << "'"; logical_type = UndefinedLogicalType::Make(); } primitive_node = std::unique_ptr(new PrimitiveNode( @@ -608,7 +593,8 @@ std::unique_ptr Unflatten(std::span elements, size_t pos = 0; size_t num_reserved = 0; - std::function(int depth)> NextNode = [&](int depth) { + std::function(int, const SchemaPath*)> NextNode; + NextNode = [&](int depth, const SchemaPath* parent_path) { if (pos == elements.size()) { throw ParquetException("Malformed Parquet schema: not enough elements"); } @@ -617,7 +603,7 @@ std::unique_ptr Unflatten(std::span elements, if (element.num_children == 0 && element.__isset.type) { // Leaf (primitive) node: always has a type - return PrimitiveNode::FromParquet(opaque_element, elements); + return PrimitiveNode::FromParquet(opaque_element, parent_path); } else { // Group node (may have 0 children, but cannot have a type) // Protect against denial-of-service through stack exhaustion when parsing @@ -640,13 +626,14 @@ std::unique_ptr Unflatten(std::span elements, throw ParquetException("Malformed Parquet schema: not enough elements"); } NodeVector fields(element.num_children); + const SchemaPath path{parent_path, element.name}; for (int i = 0; i < element.num_children; ++i) { - fields[i] = NextNode(depth + 1); + fields[i] = NextNode(depth + 1, &path); } return GroupNode::FromParquet(opaque_element, std::move(fields)); } }; - auto root = NextNode(/*depth=*/1); + auto root = NextNode(/*depth=*/1, nullptr); if (pos != elements.size()) { throw ParquetException("Malformed Parquet schema: too many elements"); } diff --git a/cpp/src/parquet/schema.h b/cpp/src/parquet/schema.h index df4537914cb..948f90b4e6f 100644 --- a/cpp/src/parquet/schema.h +++ b/cpp/src/parquet/schema.h @@ -39,6 +39,7 @@ class SchemaDescriptor; namespace schema { class Node; +struct SchemaPath; // List encodings: using the terminology from Impala to define different styles // of representing logical lists (a.k.a. ARRAY types) in Parquet schemas. Since @@ -199,9 +200,8 @@ using NodeVector = std::vector; class PARQUET_EXPORT PrimitiveNode : public Node { public: static std::unique_ptr FromParquet(const void* opaque_element); - // opaque_schema is the flattened schema containing opaque_element, or nullptr. static std::unique_ptr FromParquet(const void* opaque_element, - const void* opaque_schema); + const SchemaPath* parent_path); // A field_id -1 (or any negative value) will be serialized as null in Thrift static inline NodePtr Make(const std::string& name, Repetition::type repetition, From e907cda79b3d71f00c26e551caaff1e6d6cee838 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Fri, 25 Sep 2026 19:48:59 +0000 Subject: [PATCH 6/8] Initialize schema traversal lambda directly Co-authored-by: Isaac --- cpp/src/parquet/schema.cc | 82 ++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index ed7ff43eddd..bf307b77c14 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -593,46 +593,48 @@ std::unique_ptr Unflatten(std::span elements, size_t pos = 0; size_t num_reserved = 0; - std::function(int, const SchemaPath*)> NextNode; - NextNode = [&](int depth, const SchemaPath* parent_path) { - if (pos == elements.size()) { - throw ParquetException("Malformed Parquet schema: not enough elements"); - } - const SchemaElement& element = elements[pos++]; - const void* opaque_element = static_cast(&element); - - if (element.num_children == 0 && element.__isset.type) { - // Leaf (primitive) node: always has a type - return PrimitiveNode::FromParquet(opaque_element, parent_path); - } else { - // Group node (may have 0 children, but cannot have a type) - // Protect against denial-of-service through stack exhaustion when parsing - // deeply nested schemas. - if (depth >= max_depth) { - std::stringstream ss; - ss << "Parquet schema too deeply nested, consider increasing schema depth limit " - "(current limit is " - << max_depth << ")"; - throw ParquetException(ss.str()); - } - if (element.num_children < 0) { - throw ParquetException("Malformed Parquet schema: negative number of children"); - } - // Guard against excessive pre-reservation by an invalid schema. - // For example, a sequence of group nodes advertising N, N-1, etc. children - // could lead to quadratic preallocation. - num_reserved += static_cast(element.num_children); - if (num_reserved > elements.size()) { - throw ParquetException("Malformed Parquet schema: not enough elements"); - } - NodeVector fields(element.num_children); - const SchemaPath path{parent_path, element.name}; - for (int i = 0; i < element.num_children; ++i) { - fields[i] = NextNode(depth + 1, &path); - } - return GroupNode::FromParquet(opaque_element, std::move(fields)); - } - }; + std::function(int, const SchemaPath*)> NextNode = + [&](int depth, const SchemaPath* parent_path) { + if (pos == elements.size()) { + throw ParquetException("Malformed Parquet schema: not enough elements"); + } + const SchemaElement& element = elements[pos++]; + const void* opaque_element = static_cast(&element); + + if (element.num_children == 0 && element.__isset.type) { + // Leaf (primitive) node: always has a type + return PrimitiveNode::FromParquet(opaque_element, parent_path); + } else { + // Group node (may have 0 children, but cannot have a type) + // Protect against denial-of-service through stack exhaustion when parsing + // deeply nested schemas. + if (depth >= max_depth) { + std::stringstream ss; + ss << "Parquet schema too deeply nested, consider increasing schema depth " + "limit " + "(current limit is " + << max_depth << ")"; + throw ParquetException(ss.str()); + } + if (element.num_children < 0) { + throw ParquetException( + "Malformed Parquet schema: negative number of children"); + } + // Guard against excessive pre-reservation by an invalid schema. + // For example, a sequence of group nodes advertising N, N-1, etc. children + // could lead to quadratic preallocation. + num_reserved += static_cast(element.num_children); + if (num_reserved > elements.size()) { + throw ParquetException("Malformed Parquet schema: not enough elements"); + } + NodeVector fields(element.num_children); + const SchemaPath path{parent_path, element.name}; + for (int i = 0; i < element.num_children; ++i) { + fields[i] = NextNode(depth + 1, &path); + } + return GroupNode::FromParquet(opaque_element, std::move(fields)); + } + }; auto root = NextNode(/*depth=*/1, nullptr); if (pos != elements.size()) { throw ParquetException("Malformed Parquet schema: too many elements"); From 6a4d0ade127c83146e1cb5d07ac6c676dc365554 Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Sat, 26 Sep 2026 20:14:15 +0000 Subject: [PATCH 7/8] Test nested incompatible logical type warning paths Co-authored-by: Isaac --- cpp/src/parquet/schema_test.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/cpp/src/parquet/schema_test.cc b/cpp/src/parquet/schema_test.cc index 6c8e6366adf..c5977c2351a 100644 --- a/cpp/src/parquet/schema_test.cc +++ b/cpp/src/parquet/schema_test.cc @@ -491,6 +491,29 @@ TEST_F(TestSchemaConverter, NestedExample) { ASSERT_TRUE(check_for_parent_consistency(group_)); } +TEST_F(TestSchemaConverter, IncompatibleNestedLogicalType) { + std::vector elements = { + NewGroup("schema", FieldRepetitionType::REQUIRED, 1), + NewGroup("outer", FieldRepetitionType::OPTIONAL, 1), + NewGroup("inner", FieldRepetitionType::OPTIONAL, 1), + NewPrimitive("int32_uuid", FieldRepetitionType::OPTIONAL, Type::INT32)}; + format::LogicalType logical_type; + logical_type.__set_UUID(format::UUIDType{}); + elements.back().__set_logicalType(logical_type); + + ::testing::internal::CaptureStderr(); + EXPECT_NO_THROW(Convert(elements)); + const auto warning = ::testing::internal::GetCapturedStderr(); + ASSERT_THAT(warning, ::testing::HasSubstr("for column 'outer.inner.int32_uuid'")); + + SchemaDescriptor descr; + descr.Init(std::move(node_)); + const auto* column = descr.Column(0); + ASSERT_EQ(column->physical_type(), Type::INT32); + ASSERT_FALSE(column->logical_type()->is_valid()); + ASSERT_FALSE(column->can_use_min_max()); +} + TEST_F(TestSchemaConverter, ZeroColumns) { // ARROW-3843 SchemaElement elements[1]; From 40e8e79acdba787e51be94f6432c7302aa83a8af Mon Sep 17 00:00:00 2001 From: Divjot Arora Date: Wed, 30 Sep 2026 06:44:15 +0000 Subject: [PATCH 8/8] Remove incompatible logical type logging and path tracking Co-authored-by: Isaac --- cpp/src/parquet/schema.cc | 102 +++++++++++++-------------------- cpp/src/parquet/schema.h | 3 - cpp/src/parquet/schema_test.cc | 23 -------- 3 files changed, 39 insertions(+), 89 deletions(-) diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index bf307b77c14..882c615ed07 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -442,17 +442,7 @@ std::unique_ptr GroupNode::FromParquet(const void* opaque_element, return std::unique_ptr(group_node.release()); } -struct SchemaPath { - const SchemaPath* parent; - const std::string& name; -}; - std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element) { - return FromParquet(opaque_element, nullptr); -} - -std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element, - const SchemaPath* parent_path) { const format::SchemaElement* element = static_cast(opaque_element); @@ -470,16 +460,6 @@ std::unique_ptr PrimitiveNode::FromParquet(const void* opaque_element, // type annotation. if (logical_type && !logical_type->is_applicable(physical_type, element->type_length)) { - std::vector path{element->name}; - for (auto* parent = parent_path; parent && parent->parent; - parent = parent->parent) { - path.push_back(parent->name); - } - std::reverse(path.begin(), path.end()); - ARROW_LOG(WARNING) << "Dropping unsupported logical type " - << logical_type->ToString() << " on physical type " - << TypeToString(physical_type) << " for column '" - << ColumnPath(std::move(path)).ToDotString() << "'"; logical_type = UndefinedLogicalType::Make(); } primitive_node = std::unique_ptr(new PrimitiveNode( @@ -593,49 +573,45 @@ std::unique_ptr Unflatten(std::span elements, size_t pos = 0; size_t num_reserved = 0; - std::function(int, const SchemaPath*)> NextNode = - [&](int depth, const SchemaPath* parent_path) { - if (pos == elements.size()) { - throw ParquetException("Malformed Parquet schema: not enough elements"); - } - const SchemaElement& element = elements[pos++]; - const void* opaque_element = static_cast(&element); - - if (element.num_children == 0 && element.__isset.type) { - // Leaf (primitive) node: always has a type - return PrimitiveNode::FromParquet(opaque_element, parent_path); - } else { - // Group node (may have 0 children, but cannot have a type) - // Protect against denial-of-service through stack exhaustion when parsing - // deeply nested schemas. - if (depth >= max_depth) { - std::stringstream ss; - ss << "Parquet schema too deeply nested, consider increasing schema depth " - "limit " - "(current limit is " - << max_depth << ")"; - throw ParquetException(ss.str()); - } - if (element.num_children < 0) { - throw ParquetException( - "Malformed Parquet schema: negative number of children"); - } - // Guard against excessive pre-reservation by an invalid schema. - // For example, a sequence of group nodes advertising N, N-1, etc. children - // could lead to quadratic preallocation. - num_reserved += static_cast(element.num_children); - if (num_reserved > elements.size()) { - throw ParquetException("Malformed Parquet schema: not enough elements"); - } - NodeVector fields(element.num_children); - const SchemaPath path{parent_path, element.name}; - for (int i = 0; i < element.num_children; ++i) { - fields[i] = NextNode(depth + 1, &path); - } - return GroupNode::FromParquet(opaque_element, std::move(fields)); - } - }; - auto root = NextNode(/*depth=*/1, nullptr); + std::function(int depth)> NextNode = [&](int depth) { + if (pos == elements.size()) { + throw ParquetException("Malformed Parquet schema: not enough elements"); + } + const SchemaElement& element = elements[pos++]; + const void* opaque_element = static_cast(&element); + + if (element.num_children == 0 && element.__isset.type) { + // Leaf (primitive) node: always has a type + return PrimitiveNode::FromParquet(opaque_element); + } else { + // Group node (may have 0 children, but cannot have a type) + // Protect against denial-of-service through stack exhaustion when parsing + // deeply nested schemas. + if (depth >= max_depth) { + std::stringstream ss; + ss << "Parquet schema too deeply nested, consider increasing schema depth limit " + "(current limit is " + << max_depth << ")"; + throw ParquetException(ss.str()); + } + if (element.num_children < 0) { + throw ParquetException("Malformed Parquet schema: negative number of children"); + } + // Guard against excessive pre-reservation by an invalid schema. + // For example, a sequence of group nodes advertising N, N-1, etc. children + // could lead to quadratic preallocation. + num_reserved += static_cast(element.num_children); + if (num_reserved > elements.size()) { + throw ParquetException("Malformed Parquet schema: not enough elements"); + } + NodeVector fields(element.num_children); + for (int i = 0; i < element.num_children; ++i) { + fields[i] = NextNode(depth + 1); + } + return GroupNode::FromParquet(opaque_element, std::move(fields)); + } + }; + auto root = NextNode(/*depth=*/1); if (pos != elements.size()) { throw ParquetException("Malformed Parquet schema: too many elements"); } diff --git a/cpp/src/parquet/schema.h b/cpp/src/parquet/schema.h index 948f90b4e6f..65732603ea1 100644 --- a/cpp/src/parquet/schema.h +++ b/cpp/src/parquet/schema.h @@ -39,7 +39,6 @@ class SchemaDescriptor; namespace schema { class Node; -struct SchemaPath; // List encodings: using the terminology from Impala to define different styles // of representing logical lists (a.k.a. ARRAY types) in Parquet schemas. Since @@ -200,8 +199,6 @@ using NodeVector = std::vector; class PARQUET_EXPORT PrimitiveNode : public Node { public: static std::unique_ptr FromParquet(const void* opaque_element); - static std::unique_ptr FromParquet(const void* opaque_element, - const SchemaPath* parent_path); // A field_id -1 (or any negative value) will be serialized as null in Thrift static inline NodePtr Make(const std::string& name, Repetition::type repetition, diff --git a/cpp/src/parquet/schema_test.cc b/cpp/src/parquet/schema_test.cc index c5977c2351a..6c8e6366adf 100644 --- a/cpp/src/parquet/schema_test.cc +++ b/cpp/src/parquet/schema_test.cc @@ -491,29 +491,6 @@ TEST_F(TestSchemaConverter, NestedExample) { ASSERT_TRUE(check_for_parent_consistency(group_)); } -TEST_F(TestSchemaConverter, IncompatibleNestedLogicalType) { - std::vector elements = { - NewGroup("schema", FieldRepetitionType::REQUIRED, 1), - NewGroup("outer", FieldRepetitionType::OPTIONAL, 1), - NewGroup("inner", FieldRepetitionType::OPTIONAL, 1), - NewPrimitive("int32_uuid", FieldRepetitionType::OPTIONAL, Type::INT32)}; - format::LogicalType logical_type; - logical_type.__set_UUID(format::UUIDType{}); - elements.back().__set_logicalType(logical_type); - - ::testing::internal::CaptureStderr(); - EXPECT_NO_THROW(Convert(elements)); - const auto warning = ::testing::internal::GetCapturedStderr(); - ASSERT_THAT(warning, ::testing::HasSubstr("for column 'outer.inner.int32_uuid'")); - - SchemaDescriptor descr; - descr.Init(std::move(node_)); - const auto* column = descr.Column(0); - ASSERT_EQ(column->physical_type(), Type::INT32); - ASSERT_FALSE(column->logical_type()->is_valid()); - ASSERT_FALSE(column->can_use_min_max()); -} - TEST_F(TestSchemaConverter, ZeroColumns) { // ARROW-3843 SchemaElement elements[1];