-
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 Hour_Of_Day Function As An Alias Of Hour #195
Add Hour_Of_Day Function As An Alias Of Hour #195
Conversation
Signed-off-by: GabeFernandez310 <[email protected]>
Signed-off-by: GabeFernandez310 <[email protected]>
Signed-off-by: GabeFernandez310 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## integ-add-hour_of_day-function #195 +/- ##
=================================================================
Coverage ? 95.82%
Complexity ? 3542
=================================================================
Files ? 356
Lines ? 9400
Branches ? 674
=================================================================
Hits ? 9008
Misses ? 334
Partials ? 58
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: GabeFernandez310 <[email protected]>
@@ -245,6 +245,7 @@ datetimeConstantLiteral | |||
| CURRENT_TIME | |||
| CURRENT_TIMESTAMP | |||
| DAY_OF_YEAR | |||
| HOUR_OF_DAY |
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.
datetimeConstantLiteral
is a function which could be called without ()
too, e.g.
select HOUR_OF_DAY, HOUR_OF_DAY();
HOUR_OF_DAY
should be in this list. DAY_OF_YEAR
, WEEK_OF_YEAR
and MONTH_OF_YEAR
too, actually. See dateTimeFunctionName
.
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.
HOUR_OF_DAY
should be in this list.DAY_OF_YEAR
,WEEK_OF_YEAR
andMONTH_OF_YEAR
too, actually. See dateTimeFunctionName.
They are currently in this list. Did you mean to say that they shouldn't be in this list?
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.
Right, sorry. My mistake, but you understand everything correct.
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.
No worries. Thanks Yury! Have a good vacation!
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 4255ec9
Signed-off-by: GabeFernandez310 <[email protected]>
Signed-off-by: GabeFernandez310 <[email protected]>
core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: GabeFernandez310 <[email protected]>
Signed-off-by: GabeFernandez310 <[email protected]>
@@ -1597,21 +1597,21 @@ Description | |||
>>>>>>>>>>> | |||
|
|||
Usage: hour(time) extracts the hour value for time. Different from the time of day value, the time value has a large range and can be greater than 23, so the return value of hour(time) can be also greater than 23. | |||
The function `hour_of_day` is also provided as an alias. |
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.
do we need an underscore to use it as a link?
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.
Yes we do, but it does not have its own entry in the doc so I don't believe it would be linking to anything. Should I just remove the backticks altogether?
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Show resolved
Hide resolved
core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: GabeFernandez310 <[email protected]>
Description
Adds support for the
hour_of_day
function as an alias for thehour
function which currently exists in opensearchIssues Resolved
#722
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.