Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-6396] Add ADD_MONTHS function (enabled in Spark library) #3784

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
import static org.apache.calcite.sql.fun.SqlInternalOperators.LITERAL_AGG;
import static org.apache.calcite.sql.fun.SqlInternalOperators.THROW_UNLESS;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.ACOSH;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.ADD_MONTHS;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAYS_OVERLAP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAYS_ZIP;
Expand Down Expand Up @@ -863,6 +864,7 @@ Builder populate2() {
defineMethod(MAP_VALUES, BuiltInMethod.MAP_VALUES.method, NullPolicy.STRICT);
defineMethod(MAP_FROM_ARRAYS, BuiltInMethod.MAP_FROM_ARRAYS.method, NullPolicy.ANY);
defineMethod(MAP_FROM_ENTRIES, BuiltInMethod.MAP_FROM_ENTRIES.method, NullPolicy.STRICT);
defineMethod(ADD_MONTHS, BuiltInMethod.ADD_MONTHS2.method, NullPolicy.ANY);
map.put(STR_TO_MAP, new StringToMapImplementor());
map.put(ARRAY_CONCAT, new ArrayConcatImplementor());
map.put(SORT_ARRAY, new SortArrayImplementor());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5556,6 +5556,12 @@ public static Map mapFromArrays(List keysArray, List valuesArray) {
return map;
}

public static String addMonths(String date, int months) {
LocalDate localDate = LocalDate.parse(date);
LocalDate result = localDate.plusMonths(months);
return result.toString();
}

/** Support the MAP function.
*
* <p>odd-indexed elements are keys and even-indexed elements are values.
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/SqlKind.java
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,9 @@ public enum SqlKind {
/** {@code LEAST} function (Oracle). */
LEAST,

/** {@code ADD_MONTHS} function (Spark). */
ADD_MONTHS,

/** {@code DATE_ADD} function (BigQuery Semantics). */
DATE_ADD,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,13 @@ private static SqlCall transformConvert(SqlValidator validator, SqlCall call) {
.andThen(SqlTypeTransforms.TO_NULLABLE_ALL),
OperandTypes.SAME_SAME);

/** The "NVL(value, value)" function. */
@LibraryOperator(libraries = {SPARK})
public static final SqlBasicFunction ADD_MONTHS =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Databricks documentation ADD_MONTHS takes an expression that evaluates to a date, and not a string: https://docs.databricks.com/en/sql/language-manual/functions/add_months.html

The result is also supposed to be a DATE.

I have already asked this question in a prior review. Reviews take a lot of energy and time, so please listen to the comments if you expect people to review your PRs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I encountered a weird bug that seems to take some time

SqlBasicFunction.create(SqlKind.ADD_MONTHS,
ReturnTypes.VARCHAR_NULLABLE,
OperandTypes.STRING_INTEGER);

/** The "IFNULL(value, value)" function. */
@LibraryOperator(libraries = {BIG_QUERY, SPARK})
public static final SqlFunction IFNULL = NVL.withName("IFNULL");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ public enum BuiltInMethod {
FLOOR_DIV(Math.class, "floorDiv", long.class, long.class),
FLOOR_MOD(Math.class, "floorMod", long.class, long.class),
ADD_MONTHS(DateTimeUtils.class, "addMonths", long.class, int.class),
ADD_MONTHS2(SqlFunctions.class, "addMonths", String.class, int.class),
ADD_MONTHS_INT(DateTimeUtils.class, "addMonths", int.class, int.class),
SUBTRACT_MONTHS(DateTimeUtils.class, "subtractMonths", long.class,
long.class),
Expand Down
1 change: 1 addition & 0 deletions site/_docs/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2674,6 +2674,7 @@ In the following:
| p | expr :: type | Casts *expr* to *type*
| m | expr1 <=> expr2 | Whether two values are equal, treating null values as the same, and it's similar to `IS NOT DISTINCT FROM`
| * | ACOSH(numeric) | Returns the inverse hyperbolic cosine of *numeric*
| s | ADD_MONTHS(string, months) | Returns the date that is *months* after *string*
| s | ARRAY([expr [, expr ]*]) | Construct an array in Apache Spark. The function allows users to use `ARRAY()` to create an empty array
| s | ARRAY_APPEND(array, element) | Appends an *element* to the end of the *array* and returns the result. Type of *element* should be similar to type of the elements of the *array*. If the *array* is null, the function will return null. If an *element* that is null, the null *element* will be added to the end of the *array*
| s | ARRAY_COMPACT(array) | Removes null values from the *array*
Expand Down
17 changes: 17 additions & 0 deletions testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10418,6 +10418,23 @@ void checkNvl(SqlOperatorFixture f0, FunctionAlias functionAlias) {
f0.forEachLibrary(list(functionAlias.libraries), consumer);
}

@Test void testAddMonthsFunc() {
final SqlOperatorFixture f0 = fixture();
f0.setFor(SqlLibraryOperators.ADD_MONTHS);
f0.checkFails("^add_months('2016-08-31', 1)^",
"No match found for function signature "
+ "ADD_MONTHS\\(<CHARACTER>, <NUMERIC>\\)", false);

final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("ADD_MONTHS('2016-08-31', 1)", "2016-09-30",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should test with invalid dates too, and with dates prior to the Gregorian calendar introduction.
You should also test with negative values for the number of months.

"VARCHAR NOT NULL");
f.checkScalar("ADD_MONTHS('2016-08-31', 5)", "2017-01-31",
"VARCHAR NOT NULL");
f.checkScalar("ADD_MONTHS('2016-08-31', 18)", "2018-02-28",
"VARCHAR NOT NULL");
f.checkNull("ADD_MONTHS(CAST(NULL AS VARCHAR(200)), 18)");
}

@Test void testDecodeFunc() {
checkDecodeFunc(fixture().withLibrary(SqlLibrary.ORACLE));
checkDecodeFunc(fixture().withLibrary(SqlLibrary.SPARK));
Expand Down
Loading