Skip to content

Commit b049ac1

Browse files
SwiddisSwiddis
authored
Bugfix: SQL type mapping for legacy JDBC output (#3613)
1 parent ff98560 commit b049ac1

File tree

7 files changed

+191
-3
lines changed

7 files changed

+191
-3
lines changed

‎integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,11 @@ public enum Index {
650650
"_doc",
651651
getDateTimeIndexMapping(),
652652
"src/test/resources/datetime.json"),
653+
DATETIME_NESTED(
654+
TestsConstants.TEST_INDEX_DATE_TIME_NESTED,
655+
"_doc",
656+
getDateTimeNestedIndexMapping(),
657+
"src/test/resources/datetime_nested.json"),
653658
NESTED_SIMPLE(
654659
TestsConstants.TEST_INDEX_NESTED_SIMPLE,
655660
"_doc",

‎integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,11 @@ public static String getDateTimeIndexMapping() {
230230
return getMappingFile(mappingFile);
231231
}
232232

233+
public static String getDateTimeNestedIndexMapping() {
234+
String mappingFile = "date_time_nested_index_mapping.json";
235+
return getMappingFile(mappingFile);
236+
}
237+
233238
public static String getNestedSimpleIndexMapping() {
234239
String mappingFile = "nested_simple_index_mapping.json";
235240
return getMappingFile(mappingFile);

‎integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public class TestsConstants {
4646
public static final String TEST_INDEX_WEBLOGS = TEST_INDEX + "_weblogs";
4747
public static final String TEST_INDEX_DATE = TEST_INDEX + "_date";
4848
public static final String TEST_INDEX_DATE_TIME = TEST_INDEX + "_datetime";
49+
public static final String TEST_INDEX_DATE_TIME_NESTED = TEST_INDEX + "_datetime_nested";
4950

5051
/** A test index with @timestamp field */
5152
public static final String TEST_INDEX_TIME_DATA = TEST_INDEX + "_time_data";

‎integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.sql;
7+
8+
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME;
9+
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME_NESTED;
10+
11+
import java.io.IOException;
12+
import java.util.Locale;
13+
import org.json.JSONArray;
14+
import org.json.JSONObject;
15+
import org.junit.Assert;
16+
import org.junit.Ignore;
17+
import org.junit.Test;
18+
import org.opensearch.sql.legacy.SQLIntegTestCase;
19+
20+
public class ComplexTimestampQueryIT extends SQLIntegTestCase {
21+
@Override
22+
protected void init() throws Exception {
23+
loadIndex(SQLIntegTestCase.Index.DATETIME);
24+
loadIndex(SQLIntegTestCase.Index.DATETIME_NESTED);
25+
}
26+
27+
/** See: <a href="https://github.com/opensearch-project/sql/issues/3159">3159</a> */
28+
@Test
29+
public void joinWithTimestampFieldsSchema() throws IOException {
30+
String query =
31+
String.format(
32+
Locale.ROOT,
33+
"SELECT one.login_time, two.login_time "
34+
+ "FROM %s AS one JOIN %s AS two "
35+
+ "ON one._id = two._id",
36+
TEST_INDEX_DATE_TIME,
37+
TEST_INDEX_DATE_TIME);
38+
39+
JSONObject result = executeQuery(query);
40+
JSONArray schema = result.getJSONArray("schema");
41+
42+
Assert.assertFalse(schema.isEmpty());
43+
for (int i = 0; i < schema.length(); i++) {
44+
JSONObject column = schema.getJSONObject(i);
45+
Assert.assertEquals("timestamp", column.getString("type"));
46+
}
47+
}
48+
49+
/** Control for joinWithTimestampFieldsSchema */
50+
@Test
51+
public void nonJoinTimestampFieldsSchema() throws IOException {
52+
String query =
53+
String.format(
54+
Locale.ROOT, "SELECT one.login_time " + "FROM %s AS one", TEST_INDEX_DATE_TIME);
55+
56+
JSONObject result = executeQuery(query);
57+
JSONArray schema = result.getJSONArray("schema");
58+
59+
Assert.assertFalse(schema.isEmpty());
60+
for (int i = 0; i < schema.length(); i++) {
61+
JSONObject column = schema.getJSONObject(i);
62+
Assert.assertEquals("timestamp", column.getString("type"));
63+
}
64+
}
65+
66+
/** See: <a href="https://github.com/opensearch-project/sql/issues/3204">3204</a> */
67+
// TODO currently out of scope due to V1/V2 engine feature mismatch. Should be fixed with Calcite.
68+
@Test
69+
@Ignore
70+
public void joinTimestampComparison() throws IOException {
71+
String query =
72+
String.format(
73+
Locale.ROOT,
74+
"SELECT one.login_time, two.login_time "
75+
+ "FROM %s AS one JOIN %s AS two "
76+
+ "ON one._id = two._id "
77+
+ "WHERE one.login_time > timestamp('2018-05-07 00:00:00')",
78+
TEST_INDEX_DATE_TIME,
79+
TEST_INDEX_DATE_TIME);
80+
81+
JSONObject result = executeQuery(query);
82+
Assert.assertEquals(1, result.getJSONArray("datarows").length());
83+
}
84+
85+
/** Control for joinTimestampComparison */
86+
@Test
87+
public void nonJoinTimestampComparison() throws IOException {
88+
String query =
89+
String.format(
90+
Locale.ROOT,
91+
"SELECT login_time "
92+
+ "FROM %s "
93+
+ "WHERE login_time > timestamp('2018-05-07 00:00:00')",
94+
TEST_INDEX_DATE_TIME);
95+
96+
JSONObject result = executeQuery(query);
97+
System.err.println(result.getJSONArray("datarows").toString());
98+
Assert.assertEquals(1, result.getJSONArray("datarows").length());
99+
}
100+
101+
/** See: <a href="https://github.com/opensearch-project/sql/issues/1545">1545</a> */
102+
@Test
103+
public void selectDatetimeWithNested() throws IOException {
104+
String query =
105+
String.format(
106+
Locale.ROOT,
107+
"SELECT tab.login_time " + "FROM %s AS tab, tab.projects AS pro",
108+
TEST_INDEX_DATE_TIME_NESTED);
109+
110+
JSONObject result = executeQuery(query);
111+
JSONArray schema = result.getJSONArray("schema");
112+
113+
Assert.assertFalse(schema.isEmpty());
114+
for (int i = 0; i < schema.length(); i++) {
115+
JSONObject column = schema.getJSONObject(i);
116+
Assert.assertEquals("timestamp", column.getString("type"));
117+
}
118+
}
119+
120+
/** Control for selectDatetimeWithNested */
121+
@Test
122+
public void selectDatetimeWithoutNested() throws IOException {
123+
String query =
124+
String.format(
125+
Locale.ROOT, "SELECT tab.login_time " + "FROM %s AS tab", TEST_INDEX_DATE_TIME_NESTED);
126+
127+
JSONObject result = executeQuery(query);
128+
JSONArray schema = result.getJSONArray("schema");
129+
130+
Assert.assertFalse(schema.isEmpty());
131+
for (int i = 0; i < schema.length(); i++) {
132+
JSONObject column = schema.getJSONObject(i);
133+
Assert.assertEquals("timestamp", column.getString("type"));
134+
}
135+
}
136+
}

‎integ-test/src/test/resources/datetime_nested.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{"index":{"_id":"1"}}
2+
{"login_time":"2015-01-01", "projects": [{"name": "abc"}]}
3+
{"index":{"_id":"2"}}
4+
{"login_time":"2015-01-01T12:10:30Z", "projects": [{"name": "def"}, {"name": "ghi"}]}
5+
{"index":{"_id":"3"}}
6+
{"login_time":"1585882955000", "projects": []}
7+
{"index":{"_id":"4"}}
8+
{"login_time":"2020-04-08T11:10:30+05:00", "projects": [{"name": "jkl"}]}

‎integ-test/src/test/resources/indexDefinitions/date_time_nested_index_mapping.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"mappings": {
3+
"properties": {
4+
"login_time": {
5+
"type": "date"
6+
},
7+
"projects": {
8+
"type": "nested"
9+
}
10+
}
11+
}
12+
}

‎legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,19 +202,40 @@ private String cursorOutputInJDBCFormat() {
202202

203203
private String rawEntry(Row row, Schema schema) {
204204
// TODO String separator is being kept to "|" for the time being as using "\t" will require
205-
// formatting since
206-
// TODO tabs are occurring in multiple of 4 (one option is Guava's Strings.padEnd() method)
205+
// formatting since tabs are occurring in multiple of 4 (one option is Guava's Strings.padEnd()
206+
// method)
207207
return StreamSupport.stream(schema.spliterator(), false)
208208
.map(column -> row.getDataOrDefault(column.getName(), "NULL").toString())
209209
.collect(Collectors.joining("|"));
210210
}
211211

212+
/**
213+
* Apply the V2 type mapping described in docs/user/general/datatypes.rst#data-types-mapping. The
214+
* legacy engine works directly on OpenSearch types internally (trying to map on reading the OS
215+
* schema fails when other parts call Type.valueOf(...)), so we need to apply this mapping at the
216+
* serialization stage.
217+
*
218+
* @param column The column to fetch the type for
219+
* @return The type mapped to the appropriate OpenSearch SQL type
220+
*/
221+
private String v2MappedType(Column column) {
222+
String type = column.getType();
223+
224+
return switch (type) {
225+
case "date", "date_nanos" -> "timestamp";
226+
case "half_float", "scaled_float" -> "float";
227+
case "nested" -> "array";
228+
case "object" -> "struct";
229+
default -> type;
230+
};
231+
}
232+
212233
private JSONArray getSchemaAsJson() {
213234
Schema schema = resultSet.getSchema();
214235
JSONArray schemaJson = new JSONArray();
215236

216237
for (Column column : schema) {
217-
schemaJson.put(schemaEntry(column.getName(), column.getAlias(), column.getType()));
238+
schemaJson.put(schemaEntry(column.getName(), column.getAlias(), v2MappedType(column)));
218239
}
219240

220241
return schemaJson;

0 commit comments

Comments
 (0)