-
Notifications
You must be signed in to change notification settings - Fork 186
Support date and time function: week #757
Support date and time function: week #757
Conversation
…ime-function-week # Conflicts: # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunction.java # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java # core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunctionTest.java
…ime-function-week
integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/DateTimeFunctionIT.java
Outdated
Show resolved
Hide resolved
integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java
Outdated
Show resolved
Hide resolved
.../main/java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/CalenderLookup.java
Outdated
Show resolved
Hide resolved
…ime-function-week # Conflicts: # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprStringValue.java # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunction.java # core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java # docs/user/dql/functions.rst # integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/DateTimeFunctionIT.java # ppl/src/main/antlr/OpenDistroPPLParser.g4 # sql/src/main/antlr/OpenDistroSQLParser.g4
...java/com/amazon/opendistroforelasticsearch/sql/expression/datetime/DateTimeFunctionTest.java
Show resolved
Hide resolved
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.
LGTM, thanks for the changes!
* Returns week number for date according to mode. | ||
* @param mode Integer for mode. Valid mode values are 0 to 7. | ||
*/ | ||
public int getWeekNumber(int mode) { |
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.
could this function be static to avoid consturct CalendarLookup every time?
e.g. getWeekNumber(ExprValue date, ExprValue mode)
then define getCalendar(int firstDayOfWeek, int minimalDaysInWeek, Supplier dateProvider)
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.
Thanks for the suggestion. added this change.
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.
Thanks for the change.
Issue #709
Description of changes:
Added implementation, unit test, manual IT, and doc updates for function week()
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.