-
Notifications
You must be signed in to change notification settings - Fork 180
Bugfix: SQL type mapping for legacy JDBC output #3613
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
Changes from all commits
b43233c
4bec578
aa66519
4a83cd7
e1e4f13
f0653bb
abdb248
57c4168
b9bd84d
bff0a9d
f52c763
f9c2df3
33d0ff4
4ba80b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.sql; | ||
|
|
||
| import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME; | ||
| import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME_NESTED; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Locale; | ||
| import org.json.JSONArray; | ||
| import org.json.JSONObject; | ||
| import org.junit.Assert; | ||
| import org.junit.Ignore; | ||
| import org.junit.Test; | ||
| import org.opensearch.sql.legacy.SQLIntegTestCase; | ||
|
|
||
| public class ComplexTimestampQueryIT extends SQLIntegTestCase { | ||
| @Override | ||
| protected void init() throws Exception { | ||
| loadIndex(SQLIntegTestCase.Index.DATETIME); | ||
| loadIndex(SQLIntegTestCase.Index.DATETIME_NESTED); | ||
| } | ||
|
|
||
| /** See: <a href="https://github.com/opensearch-project/sql/issues/3159">3159</a> */ | ||
| @Test | ||
| public void joinWithTimestampFieldsSchema() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, | ||
| "SELECT one.login_time, two.login_time " | ||
| + "FROM %s AS one JOIN %s AS two " | ||
| + "ON one._id = two._id", | ||
| TEST_INDEX_DATE_TIME, | ||
| TEST_INDEX_DATE_TIME); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| JSONArray schema = result.getJSONArray("schema"); | ||
|
|
||
| Assert.assertFalse(schema.isEmpty()); | ||
| for (int i = 0; i < schema.length(); i++) { | ||
| JSONObject column = schema.getJSONObject(i); | ||
| Assert.assertEquals("timestamp", column.getString("type")); | ||
| } | ||
| } | ||
|
|
||
| /** Control for joinWithTimestampFieldsSchema */ | ||
| @Test | ||
| public void nonJoinTimestampFieldsSchema() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, "SELECT one.login_time " + "FROM %s AS one", TEST_INDEX_DATE_TIME); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| JSONArray schema = result.getJSONArray("schema"); | ||
|
|
||
| Assert.assertFalse(schema.isEmpty()); | ||
| for (int i = 0; i < schema.length(); i++) { | ||
| JSONObject column = schema.getJSONObject(i); | ||
| Assert.assertEquals("timestamp", column.getString("type")); | ||
| } | ||
| } | ||
|
|
||
| /** See: <a href="https://github.com/opensearch-project/sql/issues/3204">3204</a> */ | ||
| // TODO currently out of scope due to V1/V2 engine feature mismatch. Should be fixed with Calcite. | ||
| @Test | ||
| @Ignore | ||
| public void joinTimestampComparison() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, | ||
| "SELECT one.login_time, two.login_time " | ||
| + "FROM %s AS one JOIN %s AS two " | ||
| + "ON one._id = two._id " | ||
| + "WHERE one.login_time > timestamp('2018-05-07 00:00:00')", | ||
| TEST_INDEX_DATE_TIME, | ||
| TEST_INDEX_DATE_TIME); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| Assert.assertEquals(1, result.getJSONArray("datarows").length()); | ||
| } | ||
|
|
||
| /** Control for joinTimestampComparison */ | ||
| @Test | ||
| public void nonJoinTimestampComparison() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, | ||
| "SELECT login_time " | ||
| + "FROM %s " | ||
| + "WHERE login_time > timestamp('2018-05-07 00:00:00')", | ||
| TEST_INDEX_DATE_TIME); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| System.err.println(result.getJSONArray("datarows").toString()); | ||
| Assert.assertEquals(1, result.getJSONArray("datarows").length()); | ||
| } | ||
|
|
||
| /** See: <a href="https://github.com/opensearch-project/sql/issues/1545">1545</a> */ | ||
| @Test | ||
| public void selectDatetimeWithNested() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, | ||
| "SELECT tab.login_time " + "FROM %s AS tab, tab.projects AS pro", | ||
| TEST_INDEX_DATE_TIME_NESTED); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| JSONArray schema = result.getJSONArray("schema"); | ||
|
|
||
| Assert.assertFalse(schema.isEmpty()); | ||
| for (int i = 0; i < schema.length(); i++) { | ||
| JSONObject column = schema.getJSONObject(i); | ||
| Assert.assertEquals("timestamp", column.getString("type")); | ||
| } | ||
| } | ||
|
|
||
| /** Control for selectDatetimeWithNested */ | ||
| @Test | ||
| public void selectDatetimeWithoutNested() throws IOException { | ||
| String query = | ||
| String.format( | ||
| Locale.ROOT, "SELECT tab.login_time " + "FROM %s AS tab", TEST_INDEX_DATE_TIME_NESTED); | ||
|
|
||
| JSONObject result = executeQuery(query); | ||
| JSONArray schema = result.getJSONArray("schema"); | ||
|
|
||
| Assert.assertFalse(schema.isEmpty()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit - Consider parameterizing the schema type check logic to reduce duplication: private void assertSchemaTypeIsTimestamp(JSONArray schema) {
Assert.assertFalse(schema.isEmpty());
for (int i = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- |
||
| for (int i = 0; i < schema.length(); i++) { | ||
| JSONObject column = schema.getJSONObject(i); | ||
| Assert.assertEquals("timestamp", column.getString("type")); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| {"index":{"_id":"1"}} | ||
Swiddis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| {"login_time":"2015-01-01", "projects": [{"name": "abc"}]} | ||
| {"index":{"_id":"2"}} | ||
| {"login_time":"2015-01-01T12:10:30Z", "projects": [{"name": "def"}, {"name": "ghi"}]} | ||
| {"index":{"_id":"3"}} | ||
| {"login_time":"1585882955000", "projects": []} | ||
| {"index":{"_id":"4"}} | ||
| {"login_time":"2020-04-08T11:10:30+05:00", "projects": [{"name": "jkl"}]} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "mappings": { | ||
| "properties": { | ||
| "login_time": { | ||
| "type": "date" | ||
| }, | ||
| "projects": { | ||
| "type": "nested" | ||
| } | ||
| } | ||
| } | ||
| } |
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.
have u try enable calcite, issue still exist?
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 tried porting the test to YamlRest w/ Calcite, and got this
Doesn't seem like it's ready yet, will leave it as ignored/todo