-
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 datetime functions FROM_UNIXTIME
and UNIX_TIMESTAMP
#114
Add datetime functions FROM_UNIXTIME
and UNIX_TIMESTAMP
#114
Conversation
…, UT and IT. 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]>
@@ -6,6 +6,7 @@ | |||
|
|||
package org.opensearch.sql.expression.datetime; | |||
|
|||
import static java.time.temporal.ChronoField.YEAR; |
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 there no YEAR field in ExprCoreType?
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.. Why it should be? Aren't you messing ChronoField
and ExprCoreType
? ExprCoreType
is a enum of STRING
, DOUBLE
, etc
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Show resolved
Hide resolved
Signed-off-by: Yury-Fridlyand <[email protected]>
This comment was marked as spam.
This comment was marked as spam.
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
Show resolved
Hide resolved
core/src/test/java/org/opensearch/sql/expression/datetime/FromUnixTimeTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/opensearch/sql/expression/datetime/FromUnixTimeTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/opensearch/sql/expression/datetime/FromUnixTimeTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: MaxKsyunz <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
MySQL:
OpenSearchSQL:
Missing trailing zeros in FROM_UNIXTIME |
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.
select UNIX_TIMESTAMP(TIMESTAMP('1996-13-15 17:05:42'));
Should return null (similar with other invalid dates (or times!)) instead of
TransportError(500, 'SemanticCheckException', {'error': {'type': 'SemanticCheckException', 'reason': 'Invalid Query', 'details': 'timestamp:1996-13-15 17:05:42 in unsupported format, please use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'}, 'status': 400})
| 3404817525.0 | | ||
+----------------------------------+ | ||
|
||
os> select UNIX_TIMESTAMP(TIMESTAMP('1996-11-15 17:05:42')) |
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.
MySQL shows this as returning without a floating point.
mysql> select UNIX_TIMESTAMP(TIMESTAMP('1996-11-15 17:05:42'));
+--------------------------------------------------+
| UNIX_TIMESTAMP(TIMESTAMP('1996-11-15 17:05:42')) |
+--------------------------------------------------+
| 848077542 |
+--------------------------------------------------+
1 row in set (0.00 sec)
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.
It is added by the double
serializer, it is out of my control. We can discuss to fix this globally in scope of another fix.
@@ -375,7 +375,7 @@ trigonometricFunctionName | |||
dateAndTimeFunctionBase | |||
: ADDDATE | DATE | DATE_ADD | DATE_SUB | DAY | DAYNAME | DAYOFMONTH | DAYOFWEEK | DAYOFYEAR | FROM_DAYS | |||
| HOUR | MICROSECOND | MINUTE | MONTH | MONTHNAME | QUARTER | SECOND | SUBDATE | TIME | TIME_TO_SEC | |||
| TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | MAKETIME | MAKEDATE | |||
| TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | MAKETIME | MAKEDATE | FROM_UNIXTIME | UNIX_TIMESTAMP |
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.
Add in alphabetical order please.
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.
Thank you, fixed in faf0372.
@@ -385,7 +385,7 @@ trigonometricFunctionName | |||
dateTimeFunctionName | |||
: ADDDATE | DATE | DATE_ADD | DATE_SUB | DAY | DAYNAME | DAYOFMONTH | DAYOFWEEK | DAYOFYEAR | FROM_DAYS | |||
| HOUR | MICROSECOND | MINUTE | MONTH | MONTHNAME | QUARTER | SECOND | SUBDATE | TIME | TIME_TO_SEC | |||
| TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | MAKETIME | MAKEDATE | |||
| TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | MAKETIME | MAKEDATE | FROM_UNIXTIME | UNIX_TIMESTAMP |
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.
Add in alphabetical order.
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.
Thank you, fixed in faf0372.
Signed-off-by: Yury-Fridlyand <[email protected]>
Interesting. It is impossible to track this in our framework. We convert input to |
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.
@Yury-Fridlyand please update FromUnixTimeTest
, UnixTimeStampTeset
, and UnixTwoWayConversionTest
to match existing expression tests in core/src/test/java/org/opensearch/sql/expression
.
The following belong in core/src/main/java/org/opensearch/sql/expression/DSL.java
FromUnixTimeTest.fromUnixTime(Expression)
FromUnixTimeTest.fromUnixTime(Expression, Expression)
UnixTimeStampTest.unixTimeStampExpr()
UnixTimeStampTest.unixTimeStampOf(Expression)
Once you do that, duplicated functions from UnixTwoWayConversionTest
can be removed.
core/src/test/java/org/opensearch/sql/expression/datetime/UnixTwoWayConversionTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/opensearch/sql/expression/datetime/UnixTwoWayConversionTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java
Outdated
Show resolved
Hide resolved
…TimeTestBase.java Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Max Ksyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
* Add implementation for `FROM_UNIXTIME` and `UNIX_TIMESTAMP` functions, UT and IT. Signed-off-by: Yury-Fridlyand <[email protected]>
* Add implementation for `FROM_UNIXTIME` and `UNIX_TIMESTAMP` functions, UT and IT. Signed-off-by: Yury-Fridlyand <[email protected]>
…ch-project#835) * Add datetime functions `FROM_UNIXTIME` and `UNIX_TIMESTAMP` (#114) * Add implementation for `FROM_UNIXTIME` and `UNIX_TIMESTAMP` functions, UT and IT. Signed-off-by: Yury-Fridlyand <[email protected]> * Collent all DateTime formatters into one place. Signed-off-by: Yury-Fridlyand <[email protected]> * Rename `DateFormatters` -> `DateTimeFormatters`. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
…ch-project#835) * Add datetime functions `FROM_UNIXTIME` and `UNIX_TIMESTAMP` (#114) * Add implementation for `FROM_UNIXTIME` and `UNIX_TIMESTAMP` functions, UT and IT. Signed-off-by: Yury-Fridlyand <[email protected]> * Collent all DateTime formatters into one place. Signed-off-by: Yury-Fridlyand <[email protected]> * Rename `DateFormatters` -> `DateTimeFormatters`. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
FROM_UNIXTIME
(DOUBLE) -> DATETIME
(DOUBLE, STRING) -> STRING
Implementation details
DATE_FORMAT
function is used1970-01-01 00:00:00
-3001-01-18 23:59:59.999999
NOTES
DOUBLE
affected by Floating Point ImprecisionExample
Test samples
SQL
PPL
UNIX_TIMESTAMP
() -> LONG
DATE -> DOUBLE
DATETIME -> DOUBLE
TIMESTAMP -> DOUBLE
DOUBLE -> DOUBLE
Implementation details
LocalDateTime.now()
without caching the value. Sequential calls in one query might return different values.1970-01-01 00:00:00
-3001-01-18 23:59:59.999999
.NOTES
DOUBLE
affected by Floating Point ImprecisionExample
Test samples
SQL
PPL
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.