amogh-jahagirdar commented on code in PR #9008:
URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668793711


##########
api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java:
##########
@@ -132,6 +132,51 @@ public void testStringToTimestampLiteral() {
         .isEqualTo(avroValue);
   }
 
+  @Test
+  public void 
testStringToTimestampLiteralWithMicrosecondPrecisionFromNanoseconds() {
+    // use Avro's timestamp conversion to validate the result
+    Schema avroSchema = 
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+    TimeConversions.TimestampMicrosConversion avroConversion =
+        new TimeConversions.TimestampMicrosConversion();
+
+    Literal<CharSequence> timestampStr = 
Literal.of("2017-08-18T14:21:01.123456789");
+    Literal<Long> timestamp = 
timestampStr.to(Types.TimestampType.withoutZone());
+    long avroValue =
+        avroConversion.toLong(
+            LocalDateTime.of(2017, 8, 18, 14, 21, 1, 
123456000).toInstant(ZoneOffset.UTC),
+            avroSchema,
+            avroSchema.getLogicalType());
+
+    assertThat((long) timestamp.value())
+        .as("Timestamp without zone should match UTC")
+        .isEqualTo(avroValue);
+  }
+
+  @Test
+  public void 
testStringToTimestampLiteralWithNanosecondPrecisionFromNanoseconds() {
+    Literal<CharSequence> timestampStr = 
Literal.of("2017-08-18T14:21:01.123456789");
+    Literal<Long> timestamp = 
timestampStr.to(Types.TimestampNanoType.withoutZone());
+
+    // use Avro's timestamp conversion to validate the result within one 
microsecond
+    Schema avroSchema = 
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+    TimeConversions.TimestampMicrosConversion avroConversion =
+        new TimeConversions.TimestampMicrosConversion();
+    long avroValue =
+        avroConversion.toLong(
+            LocalDateTime.of(2017, 8, 18, 14, 21, 1, 
123456000).toInstant(ZoneOffset.UTC),
+            avroSchema,
+            avroSchema.getLogicalType());
+    assertThat(timestamp.value() - avroValue * 1000)
+        .as("Timestamp without zone should match UTC")
+        .isLessThan(1000);

Review Comment:
   Yeah I was taking a second pass over all the tests, @jacobmarble @epgif it 
seems like we can have a precise assertion here. Also could you move the below 
assertion 
   This one should go first imo
   ```
       long expected = 1503066061123456789L;
       assertThat((long) timestamp.value())
           .as("Timestamp without zone should match UTC")
           .isEqualTo(expected);
   ```
   
   If the expected timestamp.value() were to be incorrect then there's no way 
timestamp.value() - avroValue * 1000 would be correct; so that assertion should 
fail first.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to