julianhyde commented on code in PR #2998:
URL: https://github.com/apache/calcite/pull/2998#discussion_r1040631072
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4824,7 +4824,7 @@ SqlNode IntervalLiteralOrExpression() :
|
e = CompoundIdentifier()
)
- intervalQualifier = IntervalQualifierStart() {
+ intervalQualifier = TimeUnitOrName() {
if (sign == -1) {
Review Comment:
Is this change still required?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7913,7 +7913,50 @@ private static void
checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
"2015-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
}
- @Test void testDenseRankFunc() {
+ @Test void testBigQueryTimestampAdd() {
+ final SqlOperatorFixture nonBigQuery = fixture()
+ .setFor(SqlLibraryOperators.TIMESTAMP_ADD_BIG_QUERY);
+ nonBigQuery.checkFails("^timestamp_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 5 minute)^",
+ "No match found for function signature "
+ + "TIMESTAMP_ADD\\(<TIMESTAMP>, <INTERVAL_DAY_TIME>\\)", false);
+
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlLibraryOperators.TIMESTAMP_ADD_BIG_QUERY);
+ f.checkScalar("timestamp_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 5000000000 nanosecond)",
+ "2008-12-25 15:30:05",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar(
+ "timestamp_add(timestamp '2008-12-25 15:30:00', interval 100000000000
microsecond)",
+ "2008-12-26 19:16:40",
+ "TIMESTAMP(3) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2008-12-25 15:30:00', interval
100000000 millisecond)",
+ "2008-12-26 19:16:40",
+ "TIMESTAMP(3) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval 2
second)",
+ "2016-02-24 12:42:27",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval 2
minute)",
+ "2016-02-24 12:44:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval
-2000 hour)",
+ "2015-12-03 04:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval 1
day)",
+ "2016-02-25 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval 1
month)",
+ "2016-03-24 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("timestamp_add(timestamp '2016-02-24 12:42:25', interval 1
year)",
+ "2017-02-24 12:42:25",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkNull("timestamp_add(CAST(NULL AS TIMESTAMP), interval 5 minute)");
+ }
+
+ @Test void testDenseRankFunc() {
Review Comment:
fix the indent of `testDenseRankFunc`. then the diff will look better, and
checkstyle will pass
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -634,6 +634,13 @@ private SqlLibraryOperators() {
ImmutableSet.<TimeUnitRange>builder()
.addAll(MONTH_UNITS).addAll(DATE_UNITS).addAll(TIME_UNITS).build();
+ /** The "TIMESTAMP_ADD(timestamp_expression, INTERVAL int64_expression
date_part)"
+ * function (BigQuery); Adds int64_expression units of date_part to the
timestamp,
+ * independent of any time zone.*/
+ @LibraryOperator(libraries = {BIG_QUERY})
+ public static final SqlFunction TIMESTAMP_ADD_BIG_QUERY =
+ new SqlTimestampAddBigQueryFunction("TIMESTAMP_ADD");
+
Review Comment:
Interval is built-in in Calcite, so the syntax description is just
"TIMESTAMP_ADD(timestamp, interval)".
I would name the function TIMESTAMP_ADD2 and describe it as 'The 2-argument
TIMESTAMP_ADD function, as in BigQuery, as opposed to the 3-argument
TIMESTAMPADD JDBC and builtin function"
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -634,6 +634,13 @@ private SqlLibraryOperators() {
ImmutableSet.<TimeUnitRange>builder()
.addAll(MONTH_UNITS).addAll(DATE_UNITS).addAll(TIME_UNITS).build();
+ /** The "TIMESTAMP_ADD(timestamp_expression, INTERVAL int64_expression
date_part)"
+ * function (BigQuery); Adds int64_expression units of date_part to the
timestamp,
+ * independent of any time zone.*/
+ @LibraryOperator(libraries = {BIG_QUERY})
+ public static final SqlFunction TIMESTAMP_ADD_BIG_QUERY =
+ new SqlTimestampAddBigQueryFunction("TIMESTAMP_ADD");
Review Comment:
I don't think you need `class SqlTimestampAddBigQueryFunction` at all.
Because this function doesn't support custom time frames, you can remove the
logic for those, and the type resolution gets simpler. I think
```
SqlBasicFunction.create(SqlKind.TIMESTAMP_ADD, ReturnTypes.ARG0_NULLABLE,
OperandTypes.TIMESTAMP_INTERVAL)
.withFunctionType(SqlFunctionCategory.TIMEDATE)
```
is a sufficient definition.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7913,7 +7913,50 @@ private static void
checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
"2015-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
}
- @Test void testDenseRankFunc() {
+ @Test void testBigQueryTimestampAdd() {
+ final SqlOperatorFixture nonBigQuery = fixture()
+ .setFor(SqlLibraryOperators.TIMESTAMP_ADD_BIG_QUERY);
+ nonBigQuery.checkFails("^timestamp_add(timestamp '2008-12-25 15:30:00', "
+ + "interval 5 minute)^",
+ "No match found for function signature "
+ + "TIMESTAMP_ADD\\(<TIMESTAMP>, <INTERVAL_DAY_TIME>\\)", false);
+
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlLibraryOperators.TIMESTAMP_ADD_BIG_QUERY);
+ f.checkScalar("timestamp_add(timestamp '2008-12-25 15:30:00', "
Review Comment:
I don't think Calcite supports NANOSECOND intervals right now (maybe not
MICROSECOND either). You should disable this statement with an `if` and log a
Calcite bug to track.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7913,7 +7913,50 @@ private static void
checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
"2015-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
}
- @Test void testDenseRankFunc() {
+ @Test void testBigQueryTimestampAdd() {
Review Comment:
In addition to this test, you should remove the `!if (false) {` ... `}`
around the `TIMESTAMP_ADD` test in `big-query.iq`.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
