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


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -870,13 +870,22 @@ private SqlLibraryOperators() {
    * INTERVAL int64 date_part)" but in Calcite the second argument can be any
    * interval expression, not just an interval literal. */
   @LibraryOperator(libraries = {BIG_QUERY})
-  public static final SqlFunction TIMESTAMP_SUB =
+  public static final SqlBasicFunction TIMESTAMP_SUB =
       SqlBasicFunction.create(SqlKind.TIMESTAMP_SUB, ReturnTypes.ARG0_NULLABLE,
           OperandTypes.TIMESTAMP_INTERVAL)
           .withFunctionType(SqlFunctionCategory.TIMEDATE);
 
-  /** The "TIMESTAMP_TRUNC(timestamp, timeUnit[, timeZone])" function 
(BigQuery);
-   * truncates a TIMESTAMP value to the beginning of a timeUnit. */
+  /** BigQuery's {@code DATETIME_SUB(timestamp, interval)} function
+   * is a synonym for TIMESTAMP_SUB because in Calcite, DATETIME
+   * is an alias for TIMESTAMP. */
+  @LibraryOperator(libraries = {BIG_QUERY})
+  public static final SqlFunction DATETIME_SUB =
+      TIMESTAMP_SUB.withName("DATETIME_SUB");
+
+  /** The "TIMESTAMP_TRUNC(timestamp_expression, date_time_part[, time_zone])"

Review Comment:
   Can you restore my original edits, so that the signature reads 
`TIMESTAMP_TRUNC(timestamp, timeUnit[, timeZone])`. `timestamp_expression` is 
copy-pasted from the bigQuery doc but is inconsistent with how other functions 
are documented in this file.



##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -197,6 +197,8 @@ private StandardConvertletTable() {
         new TimestampDiffConvertlet());
     registerOp(SqlLibraryOperators.TIMESTAMP_SUB,
         new TimestampSubConvertlet());
+    registerOp(SqlLibraryOperators.DATETIME_SUB,
+        new TimestampSubConvertlet());

Review Comment:
   move a few lines up, so that it is alphabetical. alphabetical reduces merge 
conflicts.



##########
site/_docs/reference.md:
##########
@@ -2649,6 +2650,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | DATETIME(date)                                 | Converts *date* to a 
TIMESTAMP value (at midnight)
 | b | DATETIME(date, timeZone)                       | Converts *date* to a 
TIMESTAMP value (at midnight), in *timeZone*
 | b | DATETIME(year, month, day, hour, minute, second) | Creates a TIMESTAMP 
for *year*, *month*, *day*, *hour*, *minute*, *second* (all of type INTEGER)
+| b | DATETIME_SUB(datetime, interval)               | Returns the DATETIME 
value that occurs *interval* before *datetime*

Review Comment:
   calcite has no `DATETIME` type. Use Calcite terminology.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to