LuciferYang edited a comment on pull request #31776: URL: https://github.com/apache/spark/pull/31776#issuecomment-802570617
> I don't really know enough to evaluate this. It looks reasonable and tests pass. Are there any compatibility concerns, or possible changes to behavior you can think of? https://github.com/apache/parquet-mr/blob/d96b19bb97caf6f358579c9e22626553e8dc986d/parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java#L187-L245  @srowen From the above code, it seems that the types of `OriginalType` and `LogicalTypeAnnotation` correspond one by one, but I also have some doubts, although it doesn't seem to cause compatibility problems - `OrginalType.TIME_MILLIS` is converted to `TimestampLogicalTypeAnnotation(isAdjustedToUTC = true, TimeUnit.MILLIS)` and ignores the scenario where `isAdjustedToUTC` is `false`. - `TimestampLogicalTypeAnnotation.toOriginalType()` method will return `OriginalType.TIMESTAMP_MILLIS` when unit is TimeUnit.MILLIS and ignores values of `isAdjustedToUTC` also. - A similar situation exists between `OriginalType.TIME_MILLIS` and `TimeLogicalTypeAnnotation`. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
