Allow Timestamp/Datetime values to use up to 6 digits of microsecond …

…precision (#988) * fix Timestamp/Datetime values to accept microsecond precision b/w 1-6 places - add timestamp/datetime additional unit tests * checkstyle fixes * add integration tests for timestamp micros precision fix
opendistro-for-elasticsearch · Jan 14, 2021 · 47d50d2 · 47d50d2
1 parent 067c7ee
commit 47d50d2
Show file tree

Hide file tree

Showing 7 changed files with 198 additions and 16 deletions.
diff --git a/...src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprDatetimeValue.java b/...src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprDatetimeValue.java
@@ -26,22 +26,37 @@
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.DateTimeParseException;
+import java.time.temporal.ChronoField;
 import java.time.temporal.ChronoUnit;
 import lombok.RequiredArgsConstructor;
 
 @RequiredArgsConstructor
 public class ExprDatetimeValue extends AbstractExprValue {
-  private static final DateTimeFormatter formatter = DateTimeFormatter
-      .ofPattern("yyyy-MM-dd HH:mm:ss[.SSSSSS]");
   private final LocalDateTime datetime;
 
+  private static final DateTimeFormatter FORMATTER_VARIABLE_MICROS;
+  private static final int MIN_FRACTION_SECONDS = 0;
+  private static final int MAX_FRACTION_SECONDS = 6;
+
+  static {
+    FORMATTER_VARIABLE_MICROS = new DateTimeFormatterBuilder()
+        .appendPattern("yyyy-MM-dd HH:mm:ss")
+        .appendFraction(
+            ChronoField.MICRO_OF_SECOND,
+            MIN_FRACTION_SECONDS,
+            MAX_FRACTION_SECONDS,
+            true)
+        .toFormatter();
+  }
+
   /**
    * Constructor with datetime string as input.
    */
   public ExprDatetimeValue(String datetime) {
     try {
-      this.datetime = LocalDateTime.parse(datetime, formatter);
+      this.datetime = LocalDateTime.parse(datetime, FORMATTER_VARIABLE_MICROS);
     } catch (DateTimeParseException e) {
       throw new SemanticCheckException(String.format("datetime:%s in unsupported format, please "
           + "use yyyy-MM-dd HH:mm:ss[.SSSSSS]", datetime));

diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprTimeValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprTimeValue.java
@@ -21,11 +21,11 @@
 import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType;
 import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException;
 import java.time.LocalTime;
-import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.DateTimeParseException;
+import java.time.temporal.ChronoField;
 import java.util.Objects;
-import lombok.EqualsAndHashCode;
 import lombok.RequiredArgsConstructor;
 
 /**
@@ -35,15 +35,30 @@
 public class ExprTimeValue extends AbstractExprValue {
   private final LocalTime time;
 
+  private static final DateTimeFormatter FORMATTER_VARIABLE_MICROS;
+  private static final int MIN_FRACTION_SECONDS = 0;
+  private static final int MAX_FRACTION_SECONDS = 6;
+
+  static {
+    FORMATTER_VARIABLE_MICROS = new DateTimeFormatterBuilder()
+            .appendPattern("HH:mm:ss")
+            .appendFraction(
+                    ChronoField.MICRO_OF_SECOND,
+                    MIN_FRACTION_SECONDS,
+                    MAX_FRACTION_SECONDS,
+                    true)
+            .toFormatter();
+  }
+
   /**
    * Constructor.
    */
   public ExprTimeValue(String time) {
     try {
-      this.time = LocalTime.parse(time);
+      this.time = LocalTime.parse(time, FORMATTER_VARIABLE_MICROS);
     } catch (DateTimeParseException e) {
       throw new SemanticCheckException(String.format("time:%s in unsupported format, please use "
-          + "HH:mm:ss", time));
+          + "HH:mm:ss[.SSSSSS]", time));
     }
   }
 

diff --git a/...rc/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprTimestampValue.java b/...rc/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprTimestampValue.java
@@ -26,7 +26,9 @@
 import java.time.LocalTime;
 import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
 import java.time.format.DateTimeParseException;
+import java.time.temporal.ChronoField;
 import java.time.temporal.ChronoUnit;
 import java.util.Objects;
 import lombok.RequiredArgsConstructor;
@@ -43,18 +45,33 @@ public class ExprTimestampValue extends AbstractExprValue {
   /**
    * todo. only support timestamp in format yyyy-MM-dd HH:mm:ss.
    */
-  private static final DateTimeFormatter FORMATTER = DateTimeFormatter
-      .ofPattern("yyyy-MM-dd HH:mm:ss[.SSSSSS]");
   private static final DateTimeFormatter FORMATTER_WITNOUT_NANO = DateTimeFormatter
       .ofPattern("yyyy-MM-dd HH:mm:ss");
   private final Instant timestamp;
 
+  private static final DateTimeFormatter FORMATTER_VARIABLE_MICROS;
+  private static final int MIN_FRACTION_SECONDS = 0;
+  private static final int MAX_FRACTION_SECONDS = 6;
+
+  static {
+    FORMATTER_VARIABLE_MICROS = new DateTimeFormatterBuilder()
+        .appendPattern("yyyy-MM-dd HH:mm:ss")
+        .appendFraction(
+                ChronoField.MICRO_OF_SECOND,
+                MIN_FRACTION_SECONDS,
+                MAX_FRACTION_SECONDS,
+                true)
+        .toFormatter();
+  }
+
   /**
    * Constructor.
    */
   public ExprTimestampValue(String timestamp) {
     try {
-      this.timestamp = LocalDateTime.parse(timestamp, FORMATTER).atZone(ZONE).toInstant();
+      this.timestamp = LocalDateTime.parse(timestamp, FORMATTER_VARIABLE_MICROS)
+          .atZone(ZONE)
+          .toInstant();
     } catch (DateTimeParseException e) {
       throw new SemanticCheckException(String.format("timestamp:%s in unsupported format, please "
           + "use yyyy-MM-dd HH:mm:ss[.SSSSSS]", timestamp));
@@ -66,7 +83,7 @@ public ExprTimestampValue(String timestamp) {
   public String value() {
     return timestamp.getNano() == 0 ? FORMATTER_WITNOUT_NANO.withZone(ZONE)
         .format(timestamp.truncatedTo(ChronoUnit.SECONDS))
-        : FORMATTER.withZone(ZONE).format(timestamp);
+        : FORMATTER_VARIABLE_MICROS.withZone(ZONE).format(timestamp);
   }
 
   @Override

diff --git a/...src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/DateTimeValueTest.java b/...src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/DateTimeValueTest.java
@@ -34,6 +34,8 @@
 
 public class DateTimeValueTest {
 
+  private static final int MICROS_PRECISION_MAX = 6;
+
   @Test
   public void timeValueInterfaceTest() {
     ExprValue timeValue = new ExprTimeValue("01:01:01");
@@ -103,7 +105,7 @@ public void dateInUnsupportedFormat() {
   public void timeInUnsupportedFormat() {
     SemanticCheckException exception =
         assertThrows(SemanticCheckException.class, () -> new ExprTimeValue("01:01:0"));
-    assertEquals("time:01:01:0 in unsupported format, please use HH:mm:ss",
+    assertEquals("time:01:01:0 in unsupported format, please use HH:mm:ss[.SSSSSS]",
         exception.getMessage());
   }
 
@@ -172,7 +174,96 @@ public void stringTimeValue() {
     SemanticCheckException exception =
         assertThrows(SemanticCheckException.class,
             () -> new ExprStringValue("01:01:0").timeValue());
-    assertEquals("time:01:01:0 in unsupported format, please use HH:mm:ss",
+    assertEquals("time:01:01:0 in unsupported format, please use HH:mm:ss[.SSSSSS]",
+        exception.getMessage());
+  }
+
+  @Test
+  public void timeWithVariableMicroPrecision() {
+    String timeWithMicrosFormat = "10:11:12.%s";
+
+    // Check all lengths of microsecond precision, up to max precision accepted
+    StringBuilder micros = new StringBuilder();
+    for (int microPrecision = 1; microPrecision <= MICROS_PRECISION_MAX; microPrecision++) {
+      micros.append(microPrecision);
+      String timeWithMicros = String.format(timeWithMicrosFormat, micros);
+
+      ExprValue timeValue = new ExprTimeValue(timeWithMicros);
+      assertEquals(LocalTime.parse(timeWithMicros), timeValue.timeValue());
+    }
+  }
+
+  @Test
+  public void timestampWithVariableMicroPrecision() {
+    String dateValue = "2020-08-17";
+    String timeWithMicrosFormat = "10:11:12.%s";
+
+    // Check all lengths of microsecond precision, up to max precision accepted
+    StringBuilder micros = new StringBuilder();
+    for (int microPrecision = 1; microPrecision <= MICROS_PRECISION_MAX; microPrecision++) {
+      micros.append(microPrecision);
+      String timeWithMicros = String.format(timeWithMicrosFormat, micros);
+
+      String timestampString = String.format("%s %s", dateValue, timeWithMicros);
+      ExprValue timestampValue = new ExprTimestampValue(timestampString);
+
+      assertEquals(LocalDate.parse(dateValue), timestampValue.dateValue());
+      assertEquals(LocalTime.parse(timeWithMicros), timestampValue.timeValue());
+      String localDateTime = String.format("%sT%s", dateValue, timeWithMicros);
+      assertEquals(LocalDateTime.parse(localDateTime), timestampValue.datetimeValue());
+    }
+  }
+
+  @Test
+  public void datetimeWithVariableMicroPrecision() {
+    String dateValue = "2020-08-17";
+    String timeWithMicrosFormat = "10:11:12.%s";
+
+    // Check all lengths of microsecond precision, up to max precision accepted
+    StringBuilder micros = new StringBuilder();
+    for (int microPrecision = 1; microPrecision <= MICROS_PRECISION_MAX; microPrecision++) {
+      micros.append(microPrecision);
+      String timeWithMicros = String.format(timeWithMicrosFormat, micros);
+
+      String datetimeString = String.format("%s %s", dateValue, timeWithMicros);
+      ExprValue datetimeValue = new ExprDatetimeValue(datetimeString);
+
+      assertEquals(LocalDate.parse(dateValue), datetimeValue.dateValue());
+      assertEquals(LocalTime.parse(timeWithMicros), datetimeValue.timeValue());
+      String localDateTime = String.format("%sT%s", dateValue, timeWithMicros);
+      assertEquals(LocalDateTime.parse(localDateTime), datetimeValue.datetimeValue());
+    }
+  }
+
+  @Test
+  public void timestampOverMaxMicroPrecision() {
+    SemanticCheckException exception =
+        assertThrows(SemanticCheckException.class,
+            () -> new ExprTimestampValue("2020-07-07 01:01:01.1234567"));
+    assertEquals(
+        "timestamp:2020-07-07 01:01:01.1234567 in unsupported format, "
+                + "please use yyyy-MM-dd HH:mm:ss[.SSSSSS]",
+        exception.getMessage());
+  }
+
+  @Test
+  public void datetimeOverMaxMicroPrecision() {
+    SemanticCheckException exception =
+        assertThrows(SemanticCheckException.class,
+            () -> new ExprDatetimeValue("2020-07-07 01:01:01.1234567"));
+    assertEquals(
+        "datetime:2020-07-07 01:01:01.1234567 in unsupported format, "
+                + "please use yyyy-MM-dd HH:mm:ss[.SSSSSS]",
+        exception.getMessage());
+  }
+
+  @Test
+  public void timeOverMaxMicroPrecision() {
+    SemanticCheckException exception =
+        assertThrows(SemanticCheckException.class,
+            () -> new ExprTimeValue("01:01:01.1234567"));
+    assertEquals(
+        "time:01:01:01.1234567 in unsupported format, please use HH:mm:ss[.SSSSSS]",
         exception.getMessage());
   }
 }
diff --git a/...a/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunctionTest.java b/...a/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunctionTest.java
@@ -553,7 +553,7 @@ public void microsecond() {
     expression = dsl.microsecond(DSL.literal(new ExprTimestampValue("2020-08-17 01:02:03.000010")));
     assertEquals(INTEGER, expression.type());
     assertEquals(integerValue(10), expression.valueOf(env));
-    assertEquals("microsecond(TIMESTAMP '2020-08-17 01:02:03.000010')", expression.toString());
+    assertEquals("microsecond(TIMESTAMP '2020-08-17 01:02:03.00001')", expression.toString());
   }
 
   @Test

diff --git a/...-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/DateTimeFunctionIT.java b/...-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/DateTimeFunctionIT.java
@@ -217,20 +217,44 @@ public void testMicrosecond() throws IOException {
     verifySchema(result, schema("f", null, "integer"));
     verifySome(result.getJSONArray("datarows"), rows(123456));
 
+    // Explicit timestamp value with less than 6 microsecond digits
+    result = executeQuery(String.format(
+            "source=%s | eval f =  microsecond(timestamp('2020-09-16 17:30:00.1234')) | fields f", TEST_INDEX_DATE));
+    verifySchema(result, schema("f", null, "integer"));
+    verifySome(result.getJSONArray("datarows"), rows(123400));
+
     result = executeQuery(String.format(
             "source=%s | eval f =  microsecond(time('17:30:00.000010')) | fields f", TEST_INDEX_DATE));
     verifySchema(result, schema("f", null, "integer"));
     verifySome(result.getJSONArray("datarows"), rows(10));
 
+    // Explicit time value with less than 6 microsecond digits
+    result = executeQuery(String.format(
+        "source=%s | eval f =  microsecond(time('17:30:00.1234')) | fields f", TEST_INDEX_DATE));
+    verifySchema(result, schema("f", null, "integer"));
+    verifySome(result.getJSONArray("datarows"), rows(123400));
+
     result = executeQuery(String.format(
             "source=%s | eval f =  microsecond('2020-09-16 17:30:00.123456') | fields f", TEST_INDEX_DATE));
     verifySchema(result, schema("f", null, "integer"));
     verifySome(result.getJSONArray("datarows"), rows(123456));
 
+    // Implicit timestamp value with less than 6 microsecond digits
+    result = executeQuery(String.format(
+        "source=%s | eval f =  microsecond('2020-09-16 17:30:00.1234') | fields f", TEST_INDEX_DATE));
+    verifySchema(result, schema("f", null, "integer"));
+    verifySome(result.getJSONArray("datarows"), rows(123400));
+
     result = executeQuery(String.format(
             "source=%s | eval f =  microsecond('17:30:00.000010') | fields f", TEST_INDEX_DATE));
     verifySchema(result, schema("f", null, "integer"));
     verifySome(result.getJSONArray("datarows"), rows(10));
+
+    // Implicit time value with less than 6 microsecond digits
+    result = executeQuery(String.format(
+            "source=%s | eval f =  microsecond('17:30:00.1234') | fields f", TEST_INDEX_DATE));
+    verifySchema(result, schema("f", null, "integer"));
+    verifySome(result.getJSONArray("datarows"), rows(123400));
   }
 
   @Test

diff --git a/...-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java b/...-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java
@@ -15,15 +15,14 @@
 
 package com.amazon.opendistroforelasticsearch.sql.sql;
 
+import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK;
 import static com.amazon.opendistroforelasticsearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.rows;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.schema;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifyDataRows;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifySchema;
-import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK;
 import static com.amazon.opendistroforelasticsearch.sql.util.TestUtils.getResponseBody;
 
-import com.amazon.opendistroforelasticsearch.sql.ast.expression.In;
 import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils;
 import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase;
 import com.amazon.opendistroforelasticsearch.sql.util.TestUtils;
@@ -251,17 +250,38 @@ public void testMicrosecond() throws IOException {
         schema("microsecond(timestamp('2020-09-16 17:30:00.123456'))", null, "integer"));
     verifyDataRows(result, rows(123456));
 
+    // Explicit timestamp value with less than 6 microsecond digits
+    result = executeQuery("select microsecond(timestamp('2020-09-16 17:30:00.1234'))");
+    verifySchema(result,
+        schema("microsecond(timestamp('2020-09-16 17:30:00.1234'))", null, "integer"));
+    verifyDataRows(result, rows(123400));
+
     result = executeQuery("select microsecond(time('17:30:00.000010'))");
     verifySchema(result, schema("microsecond(time('17:30:00.000010'))", null, "integer"));
     verifyDataRows(result, rows(10));
 
+    // Explicit time value with less than 6 microsecond digits
+    result = executeQuery("select microsecond(time('17:30:00.1234'))");
+    verifySchema(result, schema("microsecond(time('17:30:00.1234'))", null, "integer"));
+    verifyDataRows(result, rows(123400));
+
     result = executeQuery("select microsecond('2020-09-16 17:30:00.123456')");
     verifySchema(result, schema("microsecond('2020-09-16 17:30:00.123456')", null, "integer"));
     verifyDataRows(result, rows(123456));
 
+    // Implicit timestamp value with less than 6 microsecond digits
+    result = executeQuery("select microsecond('2020-09-16 17:30:00.1234')");
+    verifySchema(result, schema("microsecond('2020-09-16 17:30:00.1234')", null, "integer"));
+    verifyDataRows(result, rows(123400));
+
     result = executeQuery("select microsecond('17:30:00.000010')");
     verifySchema(result, schema("microsecond('17:30:00.000010')", null, "integer"));
     verifyDataRows(result, rows(10));
+
+    // Implicit time value with less than 6 microsecond digits
+    result = executeQuery("select microsecond('17:30:00.1234')");
+    verifySchema(result, schema("microsecond('17:30:00.1234')", null, "integer"));
+    verifyDataRows(result, rows(123400));
   }
 
   @Test