Skip to content
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

[BUG] Fix CAST ( ... AS DATETIME) #853

Open
Yury-Fridlyand opened this issue Sep 21, 2022 · 4 comments
Open

[BUG] Fix CAST ( ... AS DATETIME) #853

Yury-Fridlyand opened this issue Sep 21, 2022 · 4 comments
Assignees
Labels
bug Something isn't working legacy Issues related to legacy query engine to be deprecated SQL wontfix This will not be worked on

Comments

@Yury-Fridlyand
Copy link
Collaborator

Yury-Fridlyand commented Sep 21, 2022

How can one reproduce the bug?

opensearchsql> SELECT CAST(TIMESTAMP('2000-01-02 00:00:00') AS DATETIME);
{'reason': 'Invalid SQL query', 'details': '', 'type': 'NullPointerException'}
opensearchsql> SELECT CAST('2000-01-02 00:00:00' AS DATETIME);
{'reason': 'Invalid SQL query', 'details': '', 'type': 'NullPointerException'}

Exception comes from legacy engine. V2 error:

line 1:37 mismatched input 'DATETIME' expecting {'BOOLEAN', 'DOUBLE', 'FLOAT', 'INT', 'INTEGER', 'LONG', 'STRING', 'DATE', 'TIME', 'TIMESTAMP'}

convertedDataType
: typeName=DATE
| typeName=TIME
| typeName=TIMESTAMP
| typeName=INT
| typeName=INTEGER
| typeName=DOUBLE
| typeName=LONG
| typeName=FLOAT
| typeName=STRING
| typeName=BOOLEAN
;

I tried to fix this

diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4
index f94834bb..054a16db 100644
--- a/sql/src/main/antlr/OpenSearchSQLParser.g4
+++ b/sql/src/main/antlr/OpenSearchSQLParser.g4
@@ -341,6 +341,7 @@ multiFieldRelevanceFunction

 convertedDataType
     : typeName=DATE
+    | typeName=DATETIME
     | typeName=TIME
     | typeName=TIMESTAMP
     | typeName=INT

Unfortunately, this fix damages few ITs (test report):

SQLFunctionsIT.castFieldToDatetimeWithWhereClauseJdbcFormatTest
SQLFunctionsIT.castKeywordFieldToDatetimeWithAliasJdbcFormatTest
SQLFunctionsIT.castKeywordFieldToDatetimeWithoutAliasJdbcFormatTest
SQLFunctionsIT.castStatementInWhereClauseDatetimeCastTest

For example, error log from one of these tests:

org.opensearch.sql.legacy.SQLFunctionsIT > castKeywordFieldToDatetimeWithAliasJdbcFormatTest FAILED
    java.lang.RuntimeException: org.opensearch.client.ResponseException: method [POST], host [http://[::1]:42733], URI [/_plugins/_sql?format=jdbc], status line [HTTP/1.1 500 Internal Server Error]
    {
      "error": {
        "type": "SemanticCheckException",
        "reason": "Invalid Query",
        "details": "datetime:2014-08-19T07:09:13.434Z in unsupported format, please use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]"
      },
      "status": 400
    }

The test itself:

executeJdbcRequest("SELECT CAST(date_keyword AS DATETIME) AS test_alias FROM "

So, CAST( ... AS DATETIME) partially works even now, but proposed fix might be a breaking change.

What is the expected behavior?

opensearchsql> SELECT CAST('2000-01-02 00:00:00' AS DATETIME);
fetched rows / total rows = 1/1
+-------------------------------------------+
| CAST('2000-01-02 00:00:00' AS DATETIME)   |
|-------------------------------------------|
| 2000-01-02 00:00:00                       |
+-------------------------------------------+
opensearchsql> SELECT CAST(TIMESTAMP('2000-01-02 00:00:00') AS DATETIME);
fetched rows / total rows = 1/1
+------------------------------------------------------+
| CAST(TIMESTAMP('2000-01-02 00:00:00') AS DATETIME)   |
|------------------------------------------------------|
| 2000-01-02 00:00:00                                  |
+------------------------------------------------------+

What is your host/environment?

2.x @ fa8d5bd

@dai-chen
Copy link
Collaborator

dai-chen commented Feb 6, 2023

@Yury-Fridlyand I can send a small PR for this if you're working on other issues.

@Yury-Fridlyand
Copy link
Collaborator Author

@dai-chen, you're welcome!

@dai-chen dai-chen mentioned this issue Feb 8, 2023
6 tasks
@dai-chen dai-chen linked a pull request Feb 8, 2023 that will close this issue
6 tasks
@dai-chen
Copy link
Collaborator

dai-chen commented Feb 16, 2023

@Yury-Fridlyand @MaxKsyunz Thanks for preview the draft PR! However when I tried to add comparison test, I found out and recall that there is actually no DATETIME type in other database. So I'm thinking we may close this when DATETIME removed in #1176 instead of fixing this now.

@dai-chen dai-chen added wontfix This will not be worked on and removed v2.6.0 labels Feb 16, 2023
@Yury-Fridlyand
Copy link
Collaborator Author

@dai-chen, just to recall that this works in legacy engine.
Maybe we have to migrate it (fix) and then deprecate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working legacy Issues related to legacy query engine to be deprecated SQL wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants