Fokko commented on code in PR #1194:
URL: https://github.com/apache/parquet-mr/pull/1194#discussion_r1394558775
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -1715,15 +1729,15 @@ private void buildChildren(Types.GroupBuilder builder,
childBuilder.as(getLogicalTypeAnnotation(schemaElement.logicalType));
}
if (schemaElement.isSetConverted_type()) {
- OriginalType originalType =
getLogicalTypeAnnotation(schemaElement.converted_type,
schemaElement).toOriginalType();
- OriginalType newOriginalType = (schemaElement.isSetLogicalType() &&
getLogicalTypeAnnotation(schemaElement.logicalType) != null) ?
-
getLogicalTypeAnnotation(schemaElement.logicalType).toOriginalType() : null;
- if (!originalType.equals(newOriginalType)) {
Review Comment:
I think there was a 🐛 hidden here. Because we would only compare on the
logical type itself, and not its properties (precision/scale for decimal, or
adjust-for-utc for time/timestamp). Tests started failing, therefore I added
the `getAdjustToUtc` to retrieve the actual value from the Parquet structure.
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -1715,15 +1729,15 @@ private void buildChildren(Types.GroupBuilder builder,
childBuilder.as(getLogicalTypeAnnotation(schemaElement.logicalType));
}
if (schemaElement.isSetConverted_type()) {
- OriginalType originalType =
getLogicalTypeAnnotation(schemaElement.converted_type,
schemaElement).toOriginalType();
- OriginalType newOriginalType = (schemaElement.isSetLogicalType() &&
getLogicalTypeAnnotation(schemaElement.logicalType) != null) ?
-
getLogicalTypeAnnotation(schemaElement.logicalType).toOriginalType() : null;
- if (!originalType.equals(newOriginalType)) {
Review Comment:
I think there was a 🐛 hidden here. Because we would only compare on the
logical type itself, and not its properties (precision/scale for decimal, or
adjust-for-utc for time/timestamp). Tests started failing, therefore I added
the `getAdjustToUtc` to retrieve the actual value from the Parquet structure.
--
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: dev-unsubscr...@parquet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
