-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add YEARWEEK
Function To OpenSearch SQL
#236
Changes from 6 commits
f637b02
6995700
5bbb28b
e6e324a
9e22a53
b533e4b
a853b6a
7eac2c6
5ee1687
f5c480c
84dec18
271bf2d
c302e05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,6 +196,7 @@ public void register(BuiltinFunctionRepository repository) { | |
repository.register(week(BuiltinFunctionName.WEEKOFYEAR)); | ||
repository.register(week(BuiltinFunctionName.WEEK_OF_YEAR)); | ||
repository.register(year()); | ||
repository.register(yearweek()); | ||
} | ||
|
||
/** | ||
|
@@ -900,6 +901,30 @@ private DefaultFunctionResolver year() { | |
); | ||
} | ||
|
||
/** | ||
* YEARWEEK(DATE[,mode]). return the week number for date. | ||
*/ | ||
private DefaultFunctionResolver yearweek() { | ||
return define(BuiltinFunctionName.YEARWEEK.getName(), | ||
implWithProperties(nullMissingHandlingWithProperties((functionProperties, arg) | ||
-> DateTimeFunction.yearweekToday( | ||
DEFAULT_WEEK_OF_YEAR_MODE, | ||
functionProperties.getQueryStartClock())), INTEGER, TIME), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, DATE), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, DATETIME), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, TIMESTAMP), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, STRING), | ||
implWithProperties(nullMissingHandlingWithProperties((functionProperties, time, modeArg) | ||
-> DateTimeFunction.yearweekToday( | ||
modeArg, | ||
functionProperties.getQueryStartClock())), INTEGER, TIME, INTEGER), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, DATE, INTEGER), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, DATETIME, INTEGER), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, TIMESTAMP, INTEGER), | ||
impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, STRING, INTEGER) | ||
); | ||
} | ||
|
||
/** | ||
* Formats date according to format specifier. First argument is date, second is format. | ||
* Detailed supported signatures: | ||
|
@@ -1731,6 +1756,72 @@ private ExprValue exprYear(ExprValue date) { | |
return new ExprIntegerValue(date.dateValue().getYear()); | ||
} | ||
|
||
/** | ||
* Convert mode argument passed into our CalendarLookup class to a different mode. | ||
* Needed to align with MySQL for the yearweek function due to different behaviour for modes. | ||
* Note, this misalignment only exists for yearweek. | ||
* Our current mode behavior works as intended for other functions. | ||
* | ||
* @param mode is an integer containing the initial mode arg | ||
* @return an integer containing the new mode | ||
*/ | ||
private int convertWeekModeFromMySqlToJava(LocalDate date, int mode) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you move this function under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider naming your parameter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved under |
||
// Needed to align with MySQL. Due to how modes for this function work. | ||
// See description of modes here ... | ||
// https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_week | ||
|
||
if (CalendarLookup.getWeekNumber(mode, date) == 0) { | ||
if (mode == 0 || mode == 1) { | ||
mode = 2; | ||
} else if (mode == 5) { | ||
mode = 7; | ||
} | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return mode; | ||
} | ||
|
||
/** | ||
* Helper function to extract the yearweek output from a given date. | ||
* | ||
* @param date is a LocalDate input argument. | ||
* @param mode is an integer containing the mode used to parse the LocalDate. | ||
* @return is a long containing the formatted output for the yearweek function. | ||
*/ | ||
private int extractYearweek(LocalDate date, int mode) { | ||
mode = convertWeekModeFromMySqlToJava(date, mode); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create a new var called modeJava. It's best not to update parameters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 271bf2d |
||
int formatted = CalendarLookup.getYearNumber(mode, date) * 100 | ||
+ CalendarLookup.getWeekNumber(mode, date); | ||
|
||
return formatted; | ||
} | ||
|
||
/** | ||
* Yearweek for date implementation for ExprValue. | ||
* | ||
* @param date ExprValue of Date/Datetime/Time/Timestamp/String type. | ||
* @param mode ExprValue of Integer type. | ||
*/ | ||
private ExprValue exprYearweek(ExprValue date, ExprValue mode) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 271bf2d |
||
return new ExprIntegerValue(extractYearweek(date.dateValue(), mode.integerValue())); | ||
} | ||
|
||
/** | ||
* Yearweek for date implementation for ExprValue. | ||
* When mode is not specified default value mode 0 is used. | ||
* | ||
* @param date ExprValue of Date/Datetime/Time/Timestamp/String type. | ||
* @return ExprValue. | ||
*/ | ||
private ExprValue exprYearweekWithoutMode(ExprValue date) { | ||
return exprYearweek(date, new ExprIntegerValue(0)); | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private ExprValue yearweekToday(ExprValue mode, Clock clock) { | ||
return new ExprIntegerValue( | ||
extractYearweek(LocalDateTime.now(clock).toLocalDate(), mode.integerValue())); | ||
} | ||
|
||
private ExprValue monthOfYearToday(Clock clock) { | ||
return new ExprIntegerValue(LocalDateTime.now(clock).getMonthValue()); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
|
||
package org.opensearch.sql.expression.datetime; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.ExtendWith; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.Arguments; | ||
import org.junit.jupiter.params.provider.MethodSource; | ||
import org.mockito.Mock; | ||
import org.mockito.junit.jupiter.MockitoExtension; | ||
import org.opensearch.sql.data.model.ExprDateValue; | ||
import org.opensearch.sql.data.model.ExprTimeValue; | ||
import org.opensearch.sql.data.model.ExprValue; | ||
import org.opensearch.sql.exception.SemanticCheckException; | ||
import org.opensearch.sql.expression.DSL; | ||
import org.opensearch.sql.expression.Expression; | ||
import org.opensearch.sql.expression.ExpressionTestBase; | ||
import org.opensearch.sql.expression.FunctionExpression; | ||
import org.opensearch.sql.expression.env.Environment; | ||
|
||
import java.time.LocalDate; | ||
import java.util.stream.Stream; | ||
|
||
import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR; | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import static org.junit.jupiter.api.Assertions.assertAll; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; | ||
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; | ||
|
||
@ExtendWith(MockitoExtension.class) | ||
class YearweekTest extends ExpressionTestBase { | ||
|
||
@Mock | ||
Environment<Expression, ExprValue> env; | ||
|
||
private void yearweekQuery(String date, int mode, int expectedResult) { | ||
FunctionExpression expression = DSL | ||
.yearweek( | ||
functionProperties, | ||
DSL.literal(new ExprDateValue(date)), DSL.literal(mode)); | ||
assertEquals(INTEGER, expression.type()); | ||
assertEquals(String.format("yearweek(DATE '%s', %d)", date, mode), expression.toString()); | ||
assertEquals(integerValue(expectedResult), eval(expression)); | ||
} | ||
|
||
private static Stream<Arguments> getTestDataForYearweek() { | ||
//Test the behavior of different modes passed into the 'yearweek' function | ||
return Stream.of( | ||
Arguments.of("2019-01-05", 0, 201852), | ||
Arguments.of("2019-01-05", 1, 201901), | ||
Arguments.of("2019-01-05", 2, 201852), | ||
Arguments.of("2019-01-05", 3, 201901), | ||
Arguments.of("2019-01-05", 4, 201901), | ||
Arguments.of("2019-01-05", 5, 201853), | ||
Arguments.of("2019-01-05", 6, 201901), | ||
Arguments.of("2019-01-05", 7, 201853), | ||
Arguments.of("2019-01-06", 0, 201901), | ||
Arguments.of("2019-01-06", 1, 201901), | ||
Arguments.of("2019-01-06", 2, 201901), | ||
Arguments.of("2019-01-06", 3, 201901), | ||
Arguments.of("2019-01-06", 4, 201902), | ||
Arguments.of("2019-01-06", 5, 201853), | ||
Arguments.of("2019-01-06", 6, 201902), | ||
Arguments.of("2019-01-06", 7, 201853), | ||
Arguments.of("2019-01-07", 0, 201901), | ||
Arguments.of("2019-01-07", 1, 201902), | ||
Arguments.of("2019-01-07", 2, 201901), | ||
Arguments.of("2019-01-07", 3, 201902), | ||
Arguments.of("2019-01-07", 4, 201902), | ||
Arguments.of("2019-01-07", 5, 201901), | ||
Arguments.of("2019-01-07", 6, 201902), | ||
Arguments.of("2019-01-07", 7, 201901), | ||
Arguments.of("2000-01-01", 0, 199952), | ||
Arguments.of("2000-01-01", 2, 199952), | ||
Arguments.of("1999-12-31", 0, 199952) | ||
); | ||
} | ||
|
||
@ParameterizedTest(name = "{0} | {1}") | ||
@MethodSource("getTestDataForYearweek") | ||
public void testWeekyear(String date, int mode, int expected) { | ||
yearweekQuery(date, mode, expected); | ||
} | ||
|
||
@Test | ||
public void testYearWeekWithTimeType() { | ||
int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR); | ||
int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); | ||
int expected = Integer.parseInt(String.format("%d%02d", year, week)); | ||
|
||
FunctionExpression expression = DSL | ||
.yearweek( | ||
functionProperties, | ||
DSL.literal(new ExprTimeValue("10:11:12")), DSL.literal(0)); | ||
|
||
assertEquals(expected, eval(expression).integerValue()); | ||
} | ||
|
||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Test | ||
public void testInvalidYearWeek() { | ||
assertAll( | ||
//test invalid month | ||
() -> assertThrows( | ||
SemanticCheckException.class, | ||
() -> yearweekQuery("2019-13-05 01:02:03", 0, 0)), | ||
//test invalid day | ||
() -> assertThrows( | ||
SemanticCheckException.class, | ||
() -> yearweekQuery("2019-01-50 01:02:03", 0, 0)), | ||
//test invalid leap year | ||
() -> assertThrows( | ||
SemanticCheckException.class, | ||
() -> yearweekQuery("2019-02-29 01:02:03", 0, 0)) | ||
); | ||
} | ||
private ExprValue eval(Expression expression) { | ||
return expression.valueOf(env); | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -482,6 +482,7 @@ dateTimeFunctionName | |
| WEEK_OF_YEAR | ||
| WEEKOFYEAR | ||
| YEAR | ||
| YEARWEEK | ||
; | ||
|
||
textFunctionName | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
DateTimeFunction.
needed ..?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 271bf2d