[GitHub] [calcite] julianhyde commented on a diff in pull request #2913: [CALCITE-5180] Implement BigQuery TIME_TRUNC and TIMESTAMP_TRUNC functions

[GitBox <gi...@apache.org>]

julianhyde commented on code in PR #2913:
URL: https://github.com/apache/calcite/pull/2913#discussion_r975915626


##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -520,7 +520,9 @@ public class SqlParserTest {
       "TEMPORARY",                     "92", "99",
       "THEN",                          "92", "99", "2003", "2011", "2014", "c",
       "TIME",                          "92", "99", "2003", "2011", "2014", "c",
+      "TIME_TRUNC", // BigQuery

Review Comment:
   I'm surprised there are new keywords. We needed a new keyword for `ILIKE` because it's infix, but these can just be function names.



##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -4083,6 +4083,55 @@ void subTestIntervalSecondNegative() {
         .fails("(?s).*Was expecting one of.*");
   }
 
+  @Test void testTimestampTrunc() {
+    List<String> timeUnits = ImmutableList.<String>builder()
+        .add("MINUTE")
+        .add("HOUR")
+        .add("DAY")
+        .add("WEEK")
+        .add("MONTH")
+        .add("QUARTER")
+        .add("YEAR")
+        .add("ISOYEAR")
+        .build();
+
+    final SqlOperatorTable opTable = operatorTableFor(SqlLibrary.BIG_QUERY);
+    String test = "TIMESTAMP_TRUNC(TIMESTAMP '2000-01-01 01:00:00', %s)";
+
+    for (String unit : timeUnits) {
+      wholeExpr(String.format(Locale.ROOT, test, unit))
+          .fails("No match found for function signature "
+              + "TIMESTAMP_TRUNC\\(<TIMESTAMP>,.*\\)");
+      expr(String.format(Locale.ROOT, test, unit))
+          .withOperatorTable(opTable)
+          .columnType("TIMESTAMP(0) NOT NULL");
+    }
+  }
+
+  @Test void testTimeTrunc() {
+    List<String> timeUnits = ImmutableList.<String>builder()
+        .add("MILLISECOND")
+        .add("SECOND")
+        .add("MINUTE")
+        .add("HOUR")
+        .build();
+
+    final SqlOperatorTable opTable = operatorTableFor(SqlLibrary.BIG_QUERY);
+    String test = "TIME_TRUNC(TIME '15:30:00.00', %s)";
+
+    for (String unit : timeUnits) {
+      wholeExpr(String.format(Locale.ROOT, test, unit))
+          .fails("No match found for function signature "
+              + "TIME_TRUNC\\(<TIME>,.*\\)");
+      expr(String.format(Locale.ROOT, test, unit))
+          .withOperatorTable(opTable)
+          .columnType("TIME(0) NOT NULL");
+    }
+    // should fail on incompatible time unit
+    expr("TIME_TRUNC(TIME '15:30:00.00', ^DAY^)")
+        .fails("(?s).*Was expecting one of.*");
+  }
+

Review Comment:
   Are these tests superfluous? between `SqlParserTest` and `SqlOperatorTest` I think you have it covered. I don't think there's any non-trivial validation behavior.



##########
site/_docs/reference.md:
##########
@@ -2625,6 +2625,8 @@ semantics.
 | b m o p | SUBSTR(string, position [, substringLength ]) | Returns a portion of *string*, beginning at character *position*, *substringLength* characters long. SUBSTR calculates lengths using characters as defined by the input character set
 | m | STRCMP(string, string)                         | Returns 0 if both of the strings are same and returns -1 when the first argument is smaller than the second and 1 when the second one is smaller than the first one
 | o | TANH(numeric)                                  | Returns the hyperbolic tangent of *numeric*
+| b | TIME_TRUNC(time, timeUnit)                     | Truncates a *time* value to the granularity of *timeUnit*. The *time* value is always rounded to the beginning of timeUnit, which can be one of the following: MILLISECOND, SECOND, MINUTE, HOUR.
+| b | TIMESTAMP_TRUNC(timestamp, timeUnit)           | Truncates a *timestamp* value to the granularity of *timeUnit*. The *timestamp* value is always rounded to the beginning of the *timeUnit*.

Review Comment:
   move down a few lines, to retain alphabetical order



##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -16,7 +16,809 @@
 # limitations under the License.
 #
 !use scott-big-query
-!set outputformat csv
+!set outputformat mysql

Review Comment:
   when I merge this I'll factor out the big-query.iq change as a separate commit



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -1604,6 +1604,10 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) {
     f.checkScalar("{fn TIMESTAMPDIFF(MONTH,"
         + " TIMESTAMP '2019-09-01 00:00:00',"
         + " TIMESTAMP '2020-03-01 00:00:00')}", "6", "INTEGER NOT NULL");
+    f.checkScalar("{fn TIMESTAMP_TRUNC(TIMESTAMP '2019-09-20 15:30:00', MONTH) }",

Review Comment:
   I don't think JDBC syntax is a requirement. You should remove these lines, and revert the changes to `SqlJdbcFunctionCall.java`.



-- 
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: commits-unsubscribe@calcite.apache.org

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