From b43233c24bde9ab7c5faf16f3cbf8a17b53b6090 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 6 May 2025 20:27:46 +0000 Subject: [PATCH 01/10] Add reproducers for 1545, 3159, 3204 Signed-off-by: Simeon Widdis --- .../sql/legacy/SQLIntegTestCase.java | 45 +----- .../org/opensearch/sql/legacy/TestUtils.java | 5 + .../opensearch/sql/legacy/TestsConstants.java | 1 + .../sql/sql/ComplexTimestampQueryIT.java | 147 ++++++++++++++++++ integ-test/src/test/resources/datetime.json | 2 +- .../src/test/resources/datetime_nested.json | 8 + .../date_time_nested_index_mapping.json | 12 ++ 7 files changed, 180 insertions(+), 40 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java create mode 100644 integ-test/src/test/resources/datetime_nested.json create mode 100644 integ-test/src/test/resources/indexDefinitions/date_time_nested_index_mapping.json diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 1ee1932bfcc..194340734ea 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -6,45 +6,7 @@ package org.opensearch.sql.legacy; import static com.google.common.base.Strings.isNullOrEmpty; -import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; -import static org.opensearch.sql.legacy.TestUtils.getAccountIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getAliasIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getBankIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getBankWithNullValuesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDataTypeNonnumericIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDataTypeNumericIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDateIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDateTimeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDeepNestedIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogs2IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogs3IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDuplicationNullableIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getEmployeeNestedTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGameOfThronesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGeoIpIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGeopointIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getHobbiesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getJoinTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getJsonTestIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getLocationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getMappingFile; -import static org.opensearch.sql.legacy.TestUtils.getNestedSimpleIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getNestedTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOccupationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOdbcIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOrderIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getPeople2IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getPhraseIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getResponseBody; -import static org.opensearch.sql.legacy.TestUtils.getStateCountryIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getStringIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getUnexpandedObjectIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWeblogsIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWorkInformationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWorkerIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.isIndexExist; -import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient; +import static org.opensearch.sql.legacy.TestUtils.*; import static org.opensearch.sql.legacy.plugin.RestSqlAction.CURSOR_CLOSE_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT; @@ -716,6 +678,11 @@ public enum Index { "_doc", getDateTimeIndexMapping(), "src/test/resources/datetime.json"), + DATETIME_NESTED( + TestsConstants.TEST_INDEX_DATE_TIME_NESTED, + "_doc", + getDateTimeNestedIndexMapping(), + "src/test/resources/datetime_nested.json"), NESTED_SIMPLE( TestsConstants.TEST_INDEX_NESTED_SIMPLE, "_doc", diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index f07c390cc4a..bf9ba195029 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -230,6 +230,11 @@ public static String getDateTimeIndexMapping() { return getMappingFile(mappingFile); } + public static String getDateTimeNestedIndexMapping() { + String mappingFile = "date_time_nested_index_mapping.json"; + return getMappingFile(mappingFile); + } + public static String getNestedSimpleIndexMapping() { String mappingFile = "nested_simple_index_mapping.json"; return getMappingFile(mappingFile); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index e2057ee1637..ff6ca74e764 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -46,6 +46,7 @@ public class TestsConstants { public static final String TEST_INDEX_WEBLOGS = TEST_INDEX + "_weblogs"; public static final String TEST_INDEX_DATE = TEST_INDEX + "_date"; public static final String TEST_INDEX_DATE_TIME = TEST_INDEX + "_datetime"; + public static final String TEST_INDEX_DATE_TIME_NESTED = TEST_INDEX + "_datetime_nested"; public static final String TEST_INDEX_DEEP_NESTED = TEST_INDEX + "_deep_nested"; public static final String TEST_INDEX_STRINGS = TEST_INDEX + "_strings"; public static final String TEST_INDEX_DATATYPE_NUMERIC = TEST_INDEX + "_datatypes_numeric"; diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java new file mode 100644 index 00000000000..5cf38bc8412 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java @@ -0,0 +1,147 @@ +package org.opensearch.sql.sql; + +import org.opensearch.sql.legacy.SQLIntegTestCase; + +import java.io.IOException; +import java.util.Locale; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Assert; +import org.junit.Test; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME_NESTED; + +public class ComplexTimestampQueryIT extends SQLIntegTestCase { + @Override + protected void init() throws Exception { + loadIndex(SQLIntegTestCase.Index.DATETIME); + loadIndex(SQLIntegTestCase.Index.DATETIME_NESTED); + } + + /** + * See: 3159 + */ + @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: 3204 + */ + @Test + 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(2, 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(2, result.getJSONArray("datarows").length()); + } + + /** + * See: 1545 + */ + @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()); + for (int i = 0; i < schema.length(); i++) { + JSONObject column = schema.getJSONObject(i); + Assert.assertEquals("timestamp", column.getString("type")); + } + } +} diff --git a/integ-test/src/test/resources/datetime.json b/integ-test/src/test/resources/datetime.json index 3898da61a76..96a2749318e 100644 --- a/integ-test/src/test/resources/datetime.json +++ b/integ-test/src/test/resources/datetime.json @@ -3,6 +3,6 @@ {"index":{"_id":"2"}} {"login_time":"2015-01-01T12:10:30Z"} {"index":{"_id":"3"}} -{"login_time":"1585882955"} +{"login_time":"1585882955000"} {"index":{"_id":"4"}} {"login_time":"2020-04-08T11:10:30+05:00"} diff --git a/integ-test/src/test/resources/datetime_nested.json b/integ-test/src/test/resources/datetime_nested.json new file mode 100644 index 00000000000..4da608a3d4d --- /dev/null +++ b/integ-test/src/test/resources/datetime_nested.json @@ -0,0 +1,8 @@ +{"index":{"_id":"1"}} +{"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"}]} diff --git a/integ-test/src/test/resources/indexDefinitions/date_time_nested_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/date_time_nested_index_mapping.json new file mode 100644 index 00000000000..d113b95060b --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/date_time_nested_index_mapping.json @@ -0,0 +1,12 @@ +{ + "mappings": { + "properties": { + "login_time": { + "type": "date" + }, + "projects": { + "type": "nested" + } + } + } +} From 4bec578b99aaff1680d916fd1597d3e45126be6c Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 6 May 2025 20:30:34 +0000 Subject: [PATCH 02/10] Undo collapse to star import Signed-off-by: Simeon Widdis --- .../sql/legacy/SQLIntegTestCase.java | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 194340734ea..ebae11b8a96 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -6,7 +6,46 @@ package org.opensearch.sql.legacy; import static com.google.common.base.Strings.isNullOrEmpty; -import static org.opensearch.sql.legacy.TestUtils.*; +import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; +import static org.opensearch.sql.legacy.TestUtils.getAccountIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getAliasIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getBankIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getBankWithNullValuesIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDataTypeNonnumericIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDataTypeNumericIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDateIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDateTimeIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDateTimeNestedIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDeepNestedIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDogIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDogs2IndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDogs3IndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getDuplicationNullableIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getEmployeeNestedTypeIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getGameOfThronesIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getGeoIpIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getGeopointIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getHobbiesIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getJoinTypeIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getJsonTestIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getLocationIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getMappingFile; +import static org.opensearch.sql.legacy.TestUtils.getNestedSimpleIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getNestedTypeIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getOccupationIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getOdbcIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getOrderIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getPeople2IndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getPhraseIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getResponseBody; +import static org.opensearch.sql.legacy.TestUtils.getStateCountryIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getStringIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getUnexpandedObjectIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getWeblogsIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getWorkInformationIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getWorkerIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.isIndexExist; +import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient; import static org.opensearch.sql.legacy.plugin.RestSqlAction.CURSOR_CLOSE_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT; From aa66519c39462cfa3f26bede251d8004ad42d014 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 6 May 2025 22:41:23 +0000 Subject: [PATCH 03/10] Fix timestamp comparison query Signed-off-by: Simeon Widdis --- .../java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java index 5cf38bc8412..870bb48f814 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java @@ -74,7 +74,7 @@ public void joinTimestampComparison() throws IOException { Locale.ROOT, "SELECT one.login_time, two.login_time " + "FROM %s AS one JOIN %s AS two " + - "ON one._id = two._id" + + "ON one._id = two._id " + "WHERE one.login_time > timestamp('2018-05-07 00:00:00')", TEST_INDEX_DATE_TIME, TEST_INDEX_DATE_TIME ); From 4a83cd753c37e90c490e94e937b8c49c9ba3c712 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 8 May 2025 17:24:33 +0000 Subject: [PATCH 04/10] Ignore 3204 test Signed-off-by: Simeon Widdis --- .../java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java index 870bb48f814..d97511aae54 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java @@ -1,5 +1,6 @@ package org.opensearch.sql.sql; +import org.junit.Ignore; import org.opensearch.sql.legacy.SQLIntegTestCase; import java.io.IOException; @@ -68,7 +69,9 @@ public void nonJoinTimestampFieldsSchema() throws IOException { /** * See: 3204 */ + // 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, From e1e4f13ee8123e6c0666738733ef1798627de83c Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 8 May 2025 17:42:58 +0000 Subject: [PATCH 05/10] Hack Protocol to fix the timestamp type issue Signed-off-by: Simeon Widdis --- .../opensearch/sql/legacy/executor/format/Protocol.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java index 3c3bbdc51be..2a85a9ba525 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java @@ -214,7 +214,14 @@ private JSONArray getSchemaAsJson() { JSONArray schemaJson = new JSONArray(); for (Column column : schema) { - schemaJson.put(schemaEntry(column.getName(), column.getAlias(), column.getType())); + // Hacky workaround for #3159, #1545: Legacy sometimes falsely converts `timestamp`s to `date`s, which causes + // breakage for consumers like JDBC. Until Calcite's done and we can delete legacy, just reset the type. + String t = column.getType(); + if (t.equals("date")) { + schemaJson.put(schemaEntry(column.getName(), column.getAlias(), "timestamp")); + } else { + schemaJson.put(schemaEntry(column.getName(), column.getAlias(), t)); + } } return schemaJson; From f0653bba2c81c38ba7f9ca9610c9f78730ae9b68 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 8 May 2025 18:15:47 +0000 Subject: [PATCH 06/10] Apply spotless Signed-off-by: Simeon Widdis --- .../sql/legacy/SQLIntegTestCase.java | 8 +- .../sql/sql/ComplexTimestampQueryIT.java | 254 +++++++++--------- .../sql/legacy/executor/format/Protocol.java | 6 +- 3 files changed, 128 insertions(+), 140 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index ebae11b8a96..1eb9166cd55 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -718,10 +718,10 @@ public enum Index { getDateTimeIndexMapping(), "src/test/resources/datetime.json"), DATETIME_NESTED( - TestsConstants.TEST_INDEX_DATE_TIME_NESTED, - "_doc", - getDateTimeNestedIndexMapping(), - "src/test/resources/datetime_nested.json"), + TestsConstants.TEST_INDEX_DATE_TIME_NESTED, + "_doc", + getDateTimeNestedIndexMapping(), + "src/test/resources/datetime_nested.json"), NESTED_SIMPLE( TestsConstants.TEST_INDEX_NESTED_SIMPLE, "_doc", diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java index d97511aae54..ba9ea1c0aea 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java @@ -1,150 +1,136 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.sql; -import org.junit.Ignore; -import org.opensearch.sql.legacy.SQLIntegTestCase; +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 static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_TIME_NESTED; +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); + @Override + protected void init() throws Exception { + loadIndex(SQLIntegTestCase.Index.DATETIME); + loadIndex(SQLIntegTestCase.Index.DATETIME_NESTED); + } + + /** See: 3159 */ + @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")); } - - /** - * See: 3159 - */ - @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")); } - - /** - * 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: 3204 */ + // 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(2, 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(2, result.getJSONArray("datarows").length()); + } + + /** See: 1545 */ + @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")); } - - /** - * See: 3204 - */ - // 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(2, 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(2, result.getJSONArray("datarows").length()); - } - - /** - * See: 1545 - */ - @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()); - 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()); + for (int i = 0; i < schema.length(); i++) { + JSONObject column = schema.getJSONObject(i); + Assert.assertEquals("timestamp", column.getString("type")); } + } } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java index 2a85a9ba525..78e062a10ae 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java @@ -214,8 +214,10 @@ private JSONArray getSchemaAsJson() { JSONArray schemaJson = new JSONArray(); for (Column column : schema) { - // Hacky workaround for #3159, #1545: Legacy sometimes falsely converts `timestamp`s to `date`s, which causes - // breakage for consumers like JDBC. Until Calcite's done and we can delete legacy, just reset the type. + // Hacky workaround for #3159, #1545: Legacy sometimes falsely converts `timestamp`s to + // `date`s, which causes + // breakage for consumers like JDBC. Until Calcite's done and we can delete legacy, just reset + // the type. String t = column.getType(); if (t.equals("date")) { schemaJson.put(schemaEntry(column.getName(), column.getAlias(), "timestamp")); From abdb24884b26b77ee66b02414cb41ee56ef42c5f Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 2 Jun 2025 20:59:01 +0000 Subject: [PATCH 07/10] Make protocol mapping more robust Signed-off-by: Simeon Widdis --- .../sql/legacy/executor/format/Protocol.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java index 78e062a10ae..7eae98fc3e0 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java @@ -202,28 +202,39 @@ private String cursorOutputInJDBCFormat() { private String rawEntry(Row row, Schema schema) { // TODO String separator is being kept to "|" for the time being as using "\t" will require - // formatting since - // TODO tabs are occurring in multiple of 4 (one option is Guava's Strings.padEnd() method) + // formatting since tabs are occurring in multiple of 4 (one option is Guava's Strings.padEnd() + // method) return StreamSupport.stream(schema.spliterator(), false) .map(column -> row.getDataOrDefault(column.getName(), "NULL").toString()) .collect(Collectors.joining("|")); } + /** + * Apply the V2 type mapping described in docs/user/general/datatypes.rst#data-types-mapping. The + * legacy engine works directly on OpenSearch types internally (trying to map on reading the OS + * schema fails when other parts call Type.valueOf(...)), so we need to apply this mapping at the + * serialization stage. + * + * @param column The column to fetch the type for + * @return The type mapped to the appropriate OpenSearch SQL type + */ + private String v2MappedType(Column column) { + String type = column.getType(); + + return switch (type) { + case "half_float", "scaled_float" -> "float"; + case "date", "date_nanos" -> "timestamp"; + case "object" -> "struct"; + default -> type; + }; + } + private JSONArray getSchemaAsJson() { Schema schema = resultSet.getSchema(); JSONArray schemaJson = new JSONArray(); for (Column column : schema) { - // Hacky workaround for #3159, #1545: Legacy sometimes falsely converts `timestamp`s to - // `date`s, which causes - // breakage for consumers like JDBC. Until Calcite's done and we can delete legacy, just reset - // the type. - String t = column.getType(); - if (t.equals("date")) { - schemaJson.put(schemaEntry(column.getName(), column.getAlias(), "timestamp")); - } else { - schemaJson.put(schemaEntry(column.getName(), column.getAlias(), t)); - } + schemaJson.put(schemaEntry(column.getName(), column.getAlias(), v2MappedType(column))); } return schemaJson; From b9bd84d668392d03f7c8a402414b9b2ccbf92e22 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 3 Jun 2025 20:56:43 +0000 Subject: [PATCH 08/10] Add nested -> array type mapping, too Signed-off-by: Simeon Widdis --- .../org/opensearch/sql/legacy/executor/format/Protocol.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java index 7eae98fc3e0..0deb2fd9ccd 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/Protocol.java @@ -222,8 +222,9 @@ private String v2MappedType(Column column) { String type = column.getType(); return switch (type) { - case "half_float", "scaled_float" -> "float"; case "date", "date_nanos" -> "timestamp"; + case "half_float", "scaled_float" -> "float"; + case "nested" -> "array"; case "object" -> "struct"; default -> type; }; From bff0a9db56d410eced1c25fdafe1890ecdfb3433 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 3 Jun 2025 21:17:18 +0000 Subject: [PATCH 09/10] Empty commit (test retry) Signed-off-by: Simeon Widdis From 33d0ff4061ce14e52d7343e40ca175bc4766cb30 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 17 Sep 2025 23:12:02 +0000 Subject: [PATCH 10/10] Update because new tests depend on broken timestamp Signed-off-by: Simeon Widdis --- .../java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java | 4 ++-- integ-test/src/test/resources/datetime.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java index ba9ea1c0aea..c0eb800c103 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java @@ -79,7 +79,7 @@ public void joinTimestampComparison() throws IOException { TEST_INDEX_DATE_TIME); JSONObject result = executeQuery(query); - Assert.assertEquals(2, result.getJSONArray("datarows").length()); + Assert.assertEquals(1, result.getJSONArray("datarows").length()); } /** Control for joinTimestampComparison */ @@ -95,7 +95,7 @@ public void nonJoinTimestampComparison() throws IOException { JSONObject result = executeQuery(query); System.err.println(result.getJSONArray("datarows").toString()); - Assert.assertEquals(2, result.getJSONArray("datarows").length()); + Assert.assertEquals(1, result.getJSONArray("datarows").length()); } /** See: 1545 */ diff --git a/integ-test/src/test/resources/datetime.json b/integ-test/src/test/resources/datetime.json index 96a2749318e..3898da61a76 100644 --- a/integ-test/src/test/resources/datetime.json +++ b/integ-test/src/test/resources/datetime.json @@ -3,6 +3,6 @@ {"index":{"_id":"2"}} {"login_time":"2015-01-01T12:10:30Z"} {"index":{"_id":"3"}} -{"login_time":"1585882955000"} +{"login_time":"1585882955"} {"index":{"_id":"4"}} {"login_time":"2020-04-08T11:10:30+05:00"}