Addressed review comments (#1851)

docs/developer-guide/syntax-reference.rst

@@ -1085,8 +1085,8 @@ Scalar functions
 +------------------------+------------------------------------------------------------+---------------------------------------------------+
 | CONCAT                 |  ``CONCAT(col1, '_hello')``                                | Concatenate two strings.                          |
 +------------------------+------------------------------------------------------------+---------------------------------------------------+
-| DATETOSTRING           |  ``DATETOSTRING(START_DATE, 'yyyy-MM-dd')``                | Converts an INT date value into                   |
-|                        |                                                            | the string representation of the date in          |
+| DATETOSTRING           |  ``DATETOSTRING(START_DATE, 'yyyy-MM-dd')``                | Converts an integer representation of a date into |
+|                        |                                                            | a string representing the date in                 |
 |                        |                                                            | the given format. Single quotes in the            |
 |                        |                                                            | timestamp format can be escaped with '', for      |
 |                        |                                                            | example: 'yyyy-MM-dd''T'''.                       |
@@ -1162,11 +1162,11 @@ Scalar functions
 +------------------------+------------------------------------------------------------+---------------------------------------------------+
 | ROUND                  |  ``ROUND(col1)``                                           | Round a value to the nearest BIGINT value.        |
 +------------------------+------------------------------------------------------------+---------------------------------------------------+
-| STRINGTODATE           |  ``STRINGTODATE(col1, 'yyyy-MM-dd')``                      | Converts a string value in the given              |
-|                        |                                                            | format into the INT value                         |
-|                        |                                                            | that represents days since epoch. Single          |
-|                        |                                                            | quotes in the timestamp format can be escaped with|
-|                        |                                                            | '', for example: 'yyyy-MM-dd''T'''.               |
+| STRINGTODATE           |  ``STRINGTODATE(col1, 'yyyy-MM-dd')``                      | Converts a string representation of a date in the |
+|                        |                                                            | given format into an integer representing days    |
+|                        |                                                            | since epoch. Single quotes in the timestamp       |
+|                        |                                                            | format can be escaped with '', for example:       |
+|                        |                                                            | 'yyyy-MM-dd''T'''.                                |
 +------------------------+------------------------------------------------------------+---------------------------------------------------+
 | STRINGTOTIMESTAMP      |  ``STRINGTOTIMESTAMP(col1, 'yyyy-MM-dd HH:mm:ss.SSS')``    | Converts a string value in the given              |
 |                        |                                                            | format into the BIGINT value                      |

ksql-cli/src/test/java/io/confluent/ksql/CliTest.java

@@ -661,23 +661,33 @@ public void shouldListFunctions() {
   @Test
   public void shouldDescribeScalarFunction() throws Exception {
     final String expectedOutput =
-        "Name        : TIMESTAMPTOSTRING\n" +
-        "Author      : Confluent\n" +
-        "Overview    : Converts a BIGINT millisecond timestamp value into the string " +
-        "representation of the \n" +
-        "              timestamp in the given format.\n" +
-        "Type        : scalar\n" +
-        "Jar         : internal\n" +
-        "Variations  : \n";
+        "Name        : TIMESTAMPTOSTRING\n"
+            + "Author      : Confluent\n"
+            + "Overview    : Converts a BIGINT millisecond timestamp value into the string"
+            + " representation of the \n"
+            + "              timestamp in the given format.\n"
+            + "Type        : scalar\n"
+            + "Jar         : internal\n"
+            + "Variations  :";
 
     localCli.handleLine("describe function timestamptostring;");
     final String outputString = terminal.getOutputString();
     assertThat(outputString, containsString(expectedOutput));
 
     // variations for Udfs are loaded non-deterministically. Don't assume which variation is first
     final String expectedVariation =
-        "\tVariation   : TIMESTAMPTOSTRING(BIGINT, VARCHAR)\n" +
-        "\tReturns     : VARCHAR\n";
+        "\tVariation   : TIMESTAMPTOSTRING(epochMilli BIGINT, formatPattern VARCHAR)\n"
+            + "\tReturns     : VARCHAR\n"
+            + "\tDescription : Converts a BIGINT millisecond timestamp value into the string"
+            + " representation of the \n"
+            + "                timestamp in the given format. Single quotes in the timestamp"
+            + " format can be escaped \n"
+            + "                with '', for example: 'yyyy-MM-dd''T''HH:mm:ssX' The format"
+            + " pattern should be in the\n"
+            + "                 format expected by java.time.format.DateTimeFormatter\n"
+            + "\tepochMilli  : Milliseconds since January 1, 1970, 00:00:00 GMT.\n"
+            + "\tformatPattern: The format pattern should be in the format expected by \n"
+            + "                 java.time.format.DateTimeFormatter.";
     assertThat(outputString, containsString(expectedVariation));
   }
 

ksql-common/src/main/java/io/confluent/ksql/util/timestamp/StringToTimestampParser.java

@@ -39,7 +39,11 @@ public StringToTimestampParser(final String pattern) {
         .toFormatter();
   }
 
-  public long parse(final String text) {
+    public long parse(final String text) {
+      return parse(text, ZoneId.systemDefault());
+    }
+
+    public long parse(final String text, final ZoneId zoneId) {
     TemporalAccessor parsed = formatter.parseBest(
         text,
         ZonedDateTime::from,
@@ -51,13 +55,13 @@ public long parse(final String text) {
           +  "cannot be parsed into a timestamp");
     }
 
-    if (parsed instanceof ZonedDateTime) {
-      parsed = ((ZonedDateTime) parsed)
-          .withZoneSameInstant(ZoneId.systemDefault())
-          .toLocalDateTime();
+    if (parsed instanceof LocalDateTime) {
+      parsed = ((LocalDateTime) parsed).atZone(zoneId);
     }
 
-    final LocalDateTime dateTime = (LocalDateTime) parsed;
+    final LocalDateTime dateTime = ((ZonedDateTime) parsed)
+        .withZoneSameInstant(ZoneId.systemDefault())
+        .toLocalDateTime();
     return Timestamp.valueOf(dateTime).getTime();
   }
 

ksql-engine/src/main/java/io/confluent/ksql/function/udf/datetime/DateToString.java

@@ -19,10 +19,13 @@
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import io.confluent.ksql.function.KsqlFunctionException;
 import io.confluent.ksql.function.udf.Udf;
 import io.confluent.ksql.function.udf.UdfDescription;
+import io.confluent.ksql.function.udf.UdfParameter;
 import java.time.LocalDate;
 import java.time.format.DateTimeFormatter;
+import java.util.concurrent.ExecutionException;
 
 @UdfDescription(name = "datetostring", author = "Confluent",
     description = "Converts an integer representing days since epoch to a date string"
@@ -39,9 +42,21 @@ public class DateToString {
   @Udf(description = "Converts an integer representing days since epoch to a string"
       + " using the given format pattern. The format pattern should be in the format"
       + " expected by java.time.format.DateTimeFormatter")
-  public String dateToString(final Integer daysSinceEpoch, final String formatPattern) {
-    final DateTimeFormatter formatter = formatters.getUnchecked(formatPattern);
-    return LocalDate.ofEpochDay(daysSinceEpoch).format(formatter);
+  public String dateToString(
+      @UdfParameter(value = "epochDays",
+          description = "The Epoch Day to convert,"
+              + " based on the epoch 1970-01-01") final Integer epochDays,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern) {
+    try {
+      final DateTimeFormatter formatter = formatters.get(formatPattern);
+      return LocalDate.ofEpochDay(epochDays).format(formatter);
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to format date " + epochDays
+          + " with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
   }
 
 }

ksql-engine/src/main/java/io/confluent/ksql/function/udf/datetime/StringToDate.java

@@ -19,10 +19,13 @@
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import io.confluent.ksql.function.KsqlFunctionException;
 import io.confluent.ksql.function.udf.Udf;
 import io.confluent.ksql.function.udf.UdfDescription;
+import io.confluent.ksql.function.udf.UdfParameter;
 import java.time.LocalDate;
 import java.time.format.DateTimeFormatter;
+import java.util.concurrent.ExecutionException;
 
 @UdfDescription(name = "stringtodate", author = "Confluent",
     description = "Converts a string representation of a date into an integer representing"
@@ -37,13 +40,22 @@ public class StringToDate {
           .maximumSize(1000)
           .build(CacheLoader.from(DateTimeFormatter::ofPattern));
 
-  @Udf(description = "Converts a string representation of a date into an integer representing"
-      + " days since epoch using the given format pattern."
-      + " The format pattern should be in the format expected by"
-      + " java.time.format.DateTimeFormatter")
-  public int stringToDate(final String formattedDate, final String formatPattern) {
-    final DateTimeFormatter formatter = formatters.getUnchecked(formatPattern);
-    return ((int)LocalDate.parse(formattedDate, formatter).toEpochDay());
+  @Udf(description = "Converts formattedDate, a string representation of a date into"
+      + " an integer representing days since epoch using the given formatPattern.")
+  public int stringToDate(
+      @UdfParameter(value = "formattedDate",
+          description = "The string representation of a date.") final String formattedDate,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern) {
+    try {
+      final DateTimeFormatter formatter = formatters.get(formatPattern);
+      return ((int)LocalDate.parse(formattedDate, formatter).toEpochDay());
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to parse date '" + formattedDate
+          + "' with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
   }
 
 }

ksql-engine/src/main/java/io/confluent/ksql/function/udf/datetime/StringToTimestamp.java

@@ -19,27 +19,66 @@
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import io.confluent.ksql.function.KsqlFunctionException;
 import io.confluent.ksql.function.udf.Udf;
 import io.confluent.ksql.function.udf.UdfDescription;
+import io.confluent.ksql.function.udf.UdfParameter;
 import io.confluent.ksql.util.timestamp.StringToTimestampParser;
+import java.time.ZoneId;
+import java.util.concurrent.ExecutionException;
 
 @UdfDescription(name = "stringtotimestamp", author = "Confluent",
-    description = "Converts a string value in the given format into the BIGINT value"
-        + " that represents the millisecond timestamp.")
+    description = "Converts a string representation of a date in the given format"
+        + " into the BIGINT value that represents the millisecond timestamp.")
 public class StringToTimestamp {
 
   private final LoadingCache<String, StringToTimestampParser> parsers =
       CacheBuilder.newBuilder()
           .maximumSize(1000)
           .build(CacheLoader.from(StringToTimestampParser::new));
 
-  @Udf(description = "Converts a string value in the given format into the BIGINT value"
-      + " that represents the millisecond timestamp."
+  @Udf(description = "Converts a string representation of a date in the given format"
+      + " into the BIGINT value that represents the millisecond timestamp."
       + " Single quotes in the timestamp format can be escaped with '',"
       + " for example: 'yyyy-MM-dd''T''HH:mm:ssX'.")
-  public long stringToTimestamp(final String formattedTimestamp, final String formatPattern) {
-    final StringToTimestampParser timestampParser = parsers.getUnchecked(formatPattern);
-    return timestampParser.parse(formattedTimestamp);
+  public long stringToTimestamp(
+      @UdfParameter(value = "formattedTimestamp",
+          description = "The string representation of a date.") final String formattedTimestamp,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern) {
+    try {
+      final StringToTimestampParser timestampParser = parsers.get(formatPattern);
+      return timestampParser.parse(formattedTimestamp);
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to parse timestamp '" + formattedTimestamp
+           + "' with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
+  }
+
+  @Udf(description = "Converts a string representation of a date at the given time zone"
+      + " in the given format into the BIGINT value that represents the millisecond timestamp."
+      + " Single quotes in the timestamp format can be escaped with '',"
+      + " for example: 'yyyy-MM-dd''T''HH:mm:ssX'.")
+  public long stringToTimestamp(
+      @UdfParameter(value = "formattedTimestamp",
+          description = "The string representation of a date.") final String formattedTimestamp,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern,
+      @UdfParameter(value = "timeZone",
+          description =  " timeZone is a java.util.TimeZone ID format, for example: \"UTC\","
+              + " \"America/Los_Angeles\", \"PDT\", \"Europe/London\"") final String timeZone) {
+    try {
+      final StringToTimestampParser timestampParser = parsers.get(formatPattern);
+      final ZoneId zoneId = ZoneId.of(timeZone);
+      return timestampParser.parse(formattedTimestamp, zoneId);
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to parse timestamp '" + formattedTimestamp
+          + "' at timezone '" + timeZone + "' with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
   }
 
 }

ksql-engine/src/main/java/io/confluent/ksql/function/udf/datetime/TimestampToString.java

@@ -19,11 +19,15 @@
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import io.confluent.ksql.function.KsqlFunctionException;
 import io.confluent.ksql.function.udf.Udf;
 import io.confluent.ksql.function.udf.UdfDescription;
+import io.confluent.ksql.function.udf.UdfParameter;
 import java.sql.Timestamp;
+import java.time.DateTimeException;
 import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
+import java.util.concurrent.ExecutionException;
 
 @UdfDescription(name = "timestamptostring", author = "Confluent",
     description = "Converts a BIGINT millisecond timestamp value into"
@@ -40,28 +44,53 @@ public class TimestampToString {
       + " timestamp format can be escaped with '', for example: 'yyyy-MM-dd''T''HH:mm:ssX'"
       + " The format pattern should be in the format expected"
       + " by java.time.format.DateTimeFormatter")
-  public String timestampToString(final long millisSinceEpoch, final String formatPattern) {
-    final Timestamp timestamp = new Timestamp(millisSinceEpoch);
-    final DateTimeFormatter formatter = formatters.getUnchecked(formatPattern);
-    return timestamp.toInstant()
-        .atZone(ZoneId.systemDefault())
-        .format(formatter);
+  public String timestampToString(
+      @UdfParameter(value = "epochMilli",
+          description = "Milliseconds since"
+              + " January 1, 1970, 00:00:00 GMT.") final long epochMilli,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern) {
+    try {
+      final Timestamp timestamp = new Timestamp(epochMilli);
+      final DateTimeFormatter formatter = formatters.get(formatPattern);
+      return timestamp.toInstant()
+          .atZone(ZoneId.systemDefault())
+          .format(formatter);
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to format timestamp " + epochMilli
+          + " with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
+
   }
 
   @Udf(description = "Converts a BIGINT millisecond timestamp value into the"
       + " string representation of the timestamp in the given format. Single quotes in the"
-      + " timestamp format can be escaped with '', for example: 'yyyy-MM-dd''T''HH:mm:ssX'"
-      + " The format pattern should be in the format expected by java.time.format.DateTimeFormatter"
-      + " TIMEZONE is a java.util.TimeZone ID format, for example: \"UTC\","
-      + " \"America/Los_Angeles\", \"PDT\", \"Europe/London\"")
-  public String timestampToString(final long millisSinceEpoch, final String formatPattern,
-      final String timeZone) {
-    final Timestamp timestamp = new Timestamp(millisSinceEpoch);
-    final DateTimeFormatter formatter = formatters.getUnchecked(formatPattern);
-    final ZoneId zoneId = ZoneId.of(timeZone);
-    return timestamp.toInstant()
-        .atZone(zoneId)
-        .format(formatter);
+      + " timestamp format can be escaped with '', for example: 'yyyy-MM-dd''T''HH:mm:ssX'")
+  public String timestampToString(
+      @UdfParameter(value = "epochMilli",
+          description = "Milliseconds since"
+              + " January 1, 1970, 00:00:00 GMT.") final long epochMilli,
+      @UdfParameter(value = "formatPattern",
+          description = "The format pattern should be in the format expected by"
+              + " java.time.format.DateTimeFormatter.") final String formatPattern,
+      @UdfParameter(value = "timeZone",
+          description =  " timeZone is a java.util.TimeZone ID format, for example: \"UTC\","
+              + " \"America/Los_Angeles\", \"PDT\", \"Europe/London\"") final String timeZone) {
+    try {
+      final Timestamp timestamp = new Timestamp(epochMilli);
+      final DateTimeFormatter formatter = formatters.get(formatPattern);
+      final ZoneId zoneId = ZoneId.of(timeZone);
+      return timestamp.toInstant()
+          .atZone(zoneId)
+          .format(formatter);
+    } catch (final ExecutionException | RuntimeException e) {
+      throw new KsqlFunctionException("Failed to format timestamp " + epochMilli
+          + " at timeZone '" + timeZone + "' with formatter '" + formatPattern
+          + "': " + e.getMessage(), e);
+    }
+
   }
 
 }