-
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
Update TIMESTAMP
function
#139
Conversation
This comment was marked as spam.
This comment was marked as spam.
TIMESTAMP
functionTIMESTAMP
function
9d3f6be
to
e6843e2
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
…orresponding tests. Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
557b0c1
to
84a1d4d
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
TIMESTAMP
functionTIMESTAMP
function
|
You can also reference opensearch-project#722 as part of this PR in that this PR does partially fix opensearch-project#722 |
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.
Quick question about getting timestampValue
FunctionProperties functionProperties) { | ||
return value instanceof ExprTimeValue | ||
? ((ExprTimeValue) value).timestampValue(functionProperties) | ||
: value.timestampValue(); |
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.
Oooh. I do not like this logic is required here.
It would be better if the logic was in Expr*Value classes.
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.
Can you give an example, please?
Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]>
Description
Depends on #132/opensearch-project#1194
I referred to MySQL docs and tried to reproduce MySQL v.8.0.30 behavior as a reference.
Updated function:
TIMESTAMP
Signatures
Notes:
Unlike MySQL,
TIMESTAMP
function returns aTIMESTAMP
object (Instant
), not aDATETIME
.Test queries:
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.