pitrou commented on code in PR #50909:
URL: https://github.com/apache/arrow/pull/50909#discussion_r4132183082
##########
cpp/src/parquet/schema.cc:
##########
@@ -453,10 +453,21 @@ std::unique_ptr<Node> PrimitiveNode::FromParquet(const
void* opaque_element) {
std::unique_ptr<PrimitiveNode> primitive_node;
if (element->__isset.logicalType) {
// updated writer with logical type present
- primitive_node = std::unique_ptr<PrimitiveNode>(
- 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_applicable(physical_type, element->type_length)) {
+ ARROW_LOG(WARNING) << "Dropping unsupported logical type "
Review Comment:
> I have no strong opinion here, but isn't it better for a user to know that
they're using a library that doesn't understand the full semantic meaning of
the files they're reading?
Either they care about those semantics and they will notice anyway (because
the contents won't be read as the expected Arrow datatype: for example a
TIMESTAMP FLBA(12) column would be read as FixedSizeBinary rather Timestamp),
or they don't care about those semantics and the warning message is just a
distraction.
> realistically they should upgrade it
Only if there is a newer version of the library that supports said
combination, and only if they can reasonably upgrade (perhaps they are using
some application that links Parquet C++, and the upgrade cycle depends on the
application's release cycle).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
