pvary commented on a change in pull request #1612:
URL: https://github.com/apache/iceberg/pull/1612#discussion_r522886526
##########
File path:
hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampObjectInspectorHive3.java
##########
@@ -52,9 +54,8 @@ public void testIcebergTimestampObjectInspector() {
Assert.assertNull(oi.getPrimitiveJavaObject(null));
Assert.assertNull(oi.getPrimitiveWritableObject(null));
- long epochMilli = 1601471970000L;
- LocalDateTime local =
LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMilli), ZoneId.of("UTC"));
- Timestamp ts = Timestamp.ofEpochMilli(epochMilli);
+ LocalDateTime local = LocalDateTime.of(2020, 11, 22, 8, 11, 16, 123456789);
+ Timestamp ts = Timestamp.valueOf("2020-11-22 8:11:16.123456789");
Review comment:
The purpose of the check here is that we really want to check that the
conversion is correct from the `LocaleDateTime` to
`org.apache.hadoop.hive.common.type.Timestamp`.
For the Hive Timestamp should behave like LocalDateTime in a way that is
should be independent of the TZ. Sadly there is no constructor for the Hive
Timestamp where I can provide the (year/month/day/etc), and this was the only
constructor where I do not have to convert the data to some TZ aware construct
to create the Timestamp.
Important to note that this `Timestamp` is
`org.apache.hadoop.hive.common.type.Timestamp` which by definition should not
break across TZ-s, so if we find such an issue then we should fix it.
We can continue this discussion in the Timestamp PR by the way 😄
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
