[CALCITE-6527] Add DATE_ADD function (enabled in Spark library) #3969
Conversation
| @@ -2150,8 +2152,11 @@ private static class TimestampAddConvertlet implements SqlRexConvertlet { | |||
| switch (call.operandCount()) { | |||
| case 2: | |||
| // Oracle-style 'ADD_MONTHS(date, integer months)' | |||
| if (call.getOperator() == SqlLibraryOperators.ADD_MONTHS) { | |||
| qualifier = new SqlIntervalQualifier(TimeUnit.MONTH, null, SqlParserPos.ZERO); | |||
| if (call.getOperator() == SqlLibraryOperators.ADD_MONTHS | |||
There was a problem hiding this comment.
Use another condition to do this. Don't need to merge it.
There was a problem hiding this comment.
optimized the code to make it more readable. Do you think it's acceptable?
| f.checkScalar("date_add(timestamp '2016-02-22 13:00:01', '-2.0')", | ||
| "2016-02-20", | ||
| "DATE NOT NULL"); | ||
| f.checkScalar("date_add(timestamp '2016-02-22 13:00:01', -2)", |
There was a problem hiding this comment.
Add test like:
spark-sql> select date_add('2016-02-22 13:00:01', -2);
2016-02-20
spark-sql> select date_add('13:00:01', -2);
NULL
Time taken: 0.195 seconds, Fetched 1 row(s)
spark-sql> select date_add('1', -2);
NULL
spark-sql> select date_add(1, -2);
Error in query: cannot resolve 'date_add(1, -2)' due to data type mismatch: argument 1 requires date type, however, '1' is of int type.; line 1 pos 7;
'Project [unresolvedalias(date_add(1, -2), None)]
+- OneRowRelation
There was a problem hiding this comment.
@NobiGo Calcite has its own date type rules, which are different from Spark. I am wondering whether it is necessary to return null in select date_add('13:00:01', -2)🤔.
The dataricks documentation states that only date expressions are accepted，https://docs.databricks.com/en/sql/language-manual/functions/date_add.html.
I think you can refer to the NVL2 function I adapted before, https://issues.apache.org/jira/browse/CALCITE-6397
|
@NobiGo no other comment. squashed |
site/_docs/reference.md
Outdated
| @@ -2804,6 +2804,7 @@ In the following: | |||
| | b s | DATE_FROM_UNIX_DATE(integer) | Returns the DATE that is *integer* days after 1970-01-01 | |||
| | p r | DATE_PART(timeUnit, datetime) | Equivalent to `EXTRACT(timeUnit FROM datetime)` | |||
| | b | DATE_ADD(date, interval) | Returns the DATE value that occurs *interval* after *date* | |||
| | s | DATE_ADD(date, num_days) | Returns the DATE that is *num_days* after *date* | |||
There was a problem hiding this comment.
use camelCase for arguments, not snake_case.
|
@julianhyde @NobiGo Thanks for your review |
|
NobiGo
added
the
LGTM-will-merge-soon
The following is my recent review of why the BC date is equal to 0000.
In calcite, the date function will go through a series of conversions, such as the bigquery date_add function will be converted to DATETIME_PLUS later,and the current adapted DATE_ADD is no exception.
After being converted to DATETIME_PLUS, it seems that it will be converted to PLUS operator operation later, and finally enter
RexlmpTable.DatetimeArithmeticImplementorto perform parameter type conversion and partial calculation,return normalize(typeName, Expressions.add(trop0, trop1)). Of course, there may be higher levels of abstraction and conversion later, but unfortunately I haven't found it yet.In this process, I found that calcite did not restrict the result set of the date function to the date BC date and 0000. I raised jira and emailed to seek answers, there is Julian's explanation in jira, and calcite has its own DATE data rules.
Why 0000 and BC dates are not allowed as parameters? This is because calcite has already made restrictions on dates when parsing.