You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "julianhyde (via GitHub)" <gi...@apache.org> on 2023/01/30 04:31:59 UTC
[GitHub] [calcite] julianhyde commented on a diff in pull request #3024: [CALCITE-5469] Implement BigQuery DATETIME_ADD/DATETIME_DIFF
julianhyde commented on code in PR #3024:
URL: https://github.com/apache/calcite/pull/3024#discussion_r1090160372
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4677,6 +4677,28 @@ SqlNode DatetimeFunctionCall() :
}
}
+/**
+ * Parses BigQuery's built-in DATETIME_DIFF() function.
Review Comment:
It's not 'BigQuery's'. Just "Parses the DATETIME_DIFF function".
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -764,6 +764,22 @@ private SqlLibraryOperators() {
ReturnTypes.BIGINT_NULLABLE, OperandTypes.TIMESTAMP,
SqlFunctionCategory.TIMEDATE);
+ /** BigQuery's "DATETIME_ADD(timestamp, interval) function; Behaves similarly
Review Comment:
I wouldn't describe it as 'BigQuery's'. Other DBs might support it one day.
The "because in Calcite, datetime is a type alias for timestamp" is confusing, and I'd remove it. The fact is, both accept TIMESTAMP or TIMESTAMP WITH LOCAL TIME ZONE arguments, and both return the same type as their first argument.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -8122,6 +8122,170 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
"2015-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
}
+ /** Tests BigQuery's DATETIME_ADD(timestamp, interval) function. In Calcite,
+ * datetime is a type alias for timestamp and this function follows the same
+ * behavior as TIMESTAMP_ADD(timestamp, interval). The tests below use
+ * timestamps rather than the datetime alias because the operator fixture
+ * does not currently support type aliases. */
+ @Test void testDatetimeAdd() {
+ final SqlOperatorFixture f0 = fixture()
+ .setFor(SqlLibraryOperators.DATETIME_ADD);
+ f0.checkFails("^datetime_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 5 minute)^",
+ "No match found for function signature "
+ + "DATETIME_ADD\\(<TIMESTAMP>, <INTERVAL_DAY_TIME>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ if (Bug.CALCITE_5422_FIXED) {
+ f.checkScalar("datetime_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 100000000000 microsecond)",
+ "2008-12-26 19:16:40",
+ "TIMESTAMP(3) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 100000000 millisecond)",
+ "2008-12-26 19:16:40",
+ "TIMESTAMP(3) NOT NULL");
+ }
+
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval 2 second)",
+ "2016-02-24 12:42:27",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval 2 minute)",
+ "2016-02-24 12:44:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval -2000 hour)",
+ "2015-12-03 04:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval 1 day)",
+ "2016-02-25 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval 1 month)",
+ "2016-03-24 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("datetime_add(timestamp '2016-02-24 12:42:25', interval 1 year)",
+ "2017-02-24 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkNull("datetime_add(CAST(NULL AS TIMESTAMP), interval 5 minute)");
+
+ }
+
+ /** Tests BigQuery's DATETIME_DIFF(timestamp, timestamp2, timeUnit) function. In Calcite,
Review Comment:
same remarks as for DATETIME_ADD
##########
site/_docs/reference.md:
##########
@@ -2620,6 +2622,8 @@ semantics.
| b | DATETIME(date, time) | Returns a datetime object, given a date and a time.
| b | DATETIME(timestamp) | Converts a timestamp object to a datetime, assuming UTC.
| b | DATETIME(timestamp, timezone) | Converts a timestamp object to a datetime, in a given time zone.
+| b | DATETIME_ADD(timestamp, interval) | Synonym for TIMESTAMP_ADD.
Review Comment:
remove '.'
Not really a synonym. The implementation happens to allow both TIMESTAMP and TIMESTAMP WITH LOCAL TIME ZONE, and therefore DATETIME_ADD shares an implementation with TIMESTAMP_ADD. But the user doesn't have to know that.
--
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