You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/19 05:35:15 UTC
[GitHub] [spark] LuciferYang edited a comment on pull request #31776: [SPARK-34661][SQL] Clean up `OriginalType` and `DecimalMetadata ` usage in Parquet related code
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org