-
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 typeof
function.
#123
Add typeof
function.
#123
Conversation
This comment was marked as abuse.
This comment was marked as abuse.
…ions (opensearch-project#754) * Add implementation of `now`, `sysdate`, `localtime` and similar functions (#92) Signed-off-by: Yury-Fridlyand <[email protected]> * Rework on `now` function implementation (#113) Signed-off-by: Yury-Fridlyand <[email protected]> * Minor SQL ANTLR clean-up. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: MaxKsyunz <[email protected]>
…n-2.13.4 Update com.fasterxml.jackson to 2.13.4 to match opensearch repo.
@@ -60,6 +62,7 @@ public static void register(BuiltinFunctionRepository repository) { | |||
repository.register(castToTime()); | |||
repository.register(castToTimestamp()); | |||
repository.register(castToDatetime()); | |||
repository.register(typeof()); |
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 you please move typeof
into a separate class?
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.
How can I call it? Is Misc
OK?
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.
TypeOfOperator
seems right to me.
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 9296c62.
…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]>
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 - just a few comments
@@ -361,4 +372,27 @@ void castToDatetime() { | |||
assertEquals(new ExprDatetimeValue("2012-08-07 00:00:00"), expression.valueOf(null)); | |||
} | |||
|
|||
@Test | |||
void typeof() { | |||
assertEquals("UNDEFINED", typeofGetValue(ExprNullValue.of())); |
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 it possible to test UNKNOWN?
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, see typeof(NULL)
in integration tests
// Auxiliary function useful for debugging | ||
private static DefaultFunctionResolver typeof() { | ||
return FunctionDSL.define(BuiltinFunctionName.TYPEOF.getName(), | ||
impl(TypeOfOperator::exprTypeOf, STRING, UNKNOWN), |
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.
nit: would be nice to have these in alphabetic order like the list above in the same file.
+ " | fields `datetime`, `timestamp`, `time`, `date`", | ||
TEST_INDEX_DATATYPE_NUMERIC)); | ||
verifyDataRows(response, | ||
rows("DATETIME", "TIMESTAMP", "TIME", "DATE")); |
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.
please add a test for complex data types, like ARRAY, STRUCT, and INTERVAL.
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.
To test with ARRAY
and STRUCT
I have to update parser and AstBuilder
/AstExpressionBuilder
. I can't imagine where it could be used especially as a return value of another function.
typeof(somefunc(...)) -> STRUCT
.
Signed-off-by: Yury-Fridlyand <[email protected]>
9296c62
to
cd9570d
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
return Stream.of( | ||
Arguments.of(840101d, LocalDateTime.of(1984, 1, 1, 0, 0, 0)), | ||
Arguments.of(840101112233d, LocalDateTime.of(1984, 1, 1, 11,22,33)), | ||
Arguments.of(840101112233.123456, LocalDateTime.of(1984, 1, 1, 11, 22, 33, 123456000)), |
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 the argument.of
's need a d
at the end too?
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.
d
for Double, otherwise it is interpreted as int/long and test crashes, because the test method expect double
as the first arg.
| value_1 | value_2 | | ||
|----------------------------+----------------------------| | ||
| 2022-08-02 15:39:05.173069 | 2022-08-02 15:39:05.173069 | | ||
+----------------------------+----------------------------+ |
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.
I get this result from mySQL for this query. Should NOW
return with .nnnnnn
?
mysql> SELECT NOW() as value_1, NOW() as value_2;
+---------------------+---------------------+
| value_1 | value_2 |
+---------------------+---------------------+
| 2022-09-29 07:30:55 | 2022-09-29 07:30:55 |
+---------------------+---------------------+
1 row in set (0.00 sec)
| CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP | | ||
|----------------------------+----------------------------| | ||
| 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19.209361 | | ||
+----------------------------+----------------------------+ |
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> SELECT CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP;
+---------------------+---------------------+
| CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP |
+---------------------+---------------------+
| 2022-09-29 07:32:49 | 2022-09-29 07:32:49 |
+---------------------+
MySQL return
Add XYPoint Query Processor
Signed-off-by: Yury-Fridlyand [email protected]
Description
TYPEOF
function is useful for debugging to check types of other functions.I also fixed cast toDATETIME
.Usage
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.