jamespfaulkner commented on code in PR #14401:
URL: https://github.com/apache/iceberg/pull/14401#discussion_r2487355052
##########
core/src/main/java/org/apache/iceberg/data/avro/DataWriter.java:
##########
@@ -130,6 +130,12 @@ public ValueWriter<?> primitive(Schema primitive) {
}
return GenericWriters.timestampNanos();
+ case "timestamp-millis":
Review Comment:
So I created a test within `TestAvroDataWriter` that exercised both of the
code paths under `"timestamp-millis"`. However, upon further investigation, the
files written through Iceberg are either going to use the logical type
`timestamp-micros` or `timestamp-nanos` (corresponding to
[TimestampType](https://github.com/jamespfaulkner/iceberg/blob/e1e0a7404740b2bf9e6638afb5f0ff19f2536713/api/src/main/java/org/apache/iceberg/types/Types.java#L247)
and
[TimestampNanoType](https://github.com/jamespfaulkner/iceberg/blob/e1e0a7404740b2bf9e6638afb5f0ff19f2536713/api/src/main/java/org/apache/iceberg/types/Types.java#L301).
I dont think we want to introduce another type.
I'm therefore thinking this change isn't required in the `DataWriter`. It
would introduce inconsistencies between `avro.DataWriter` and
`avro.DataReader`, but this PR is really only required to address issues with
avro data written outside of iceberg.
I'm therefore thinking it's better to remove the changes from `DataWriter`.
WDYT?
##########
api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java:
##########
@@ -74,6 +74,10 @@ public static LocalDateTime timestampFromNanos(long
nanosFromEpoch) {
return ChronoUnit.NANOS.addTo(EPOCH, nanosFromEpoch).toLocalDateTime();
}
+ public static LocalDateTime timestampFromMillis(long millisFromEpoch) {
Review Comment:
Done.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
