[ 
https://issues.apache.org/jira/browse/PARQUET-2382?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17786452#comment-17786452
 ] 

ASF GitHub Bot commented on PARQUET-2382:
-----------------------------------------

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.





> Remove the deprecated OriginalType
> ----------------------------------
>
>                 Key: PARQUET-2382
>                 URL: https://issues.apache.org/jira/browse/PARQUET-2382
>             Project: Parquet
>          Issue Type: Improvement
>            Reporter: Fokko Driesprong
>            Assignee: Fokko Driesprong
>            Priority: Major
>             Fix For: 1.14.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to