From f5b40aae06617cdb1ec50ced2402f0d1dfbe16aa Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Mon, 17 Mar 2025 19:39:07 -0700 Subject: [PATCH 1/7] Fix issue 2489 Signed-off-by: Peng Huo --- .../data/value/OpenSearchExprValueFactory.java | 9 +++++---- .../value/OpenSearchExprValueFactoryTest.java | 17 +++-------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 68c6fda617f..91b679104d5 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -17,7 +17,6 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; import static org.opensearch.sql.data.type.ExprCoreType.TIME; import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER; import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_HOUR_MINUTE_SECOND_FORMATTER; import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_YEAR_MONTH_DAY_FORMATTER; @@ -44,6 +43,7 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateFormatters; import org.opensearch.common.time.FormatNames; +import org.opensearch.index.mapper.DateFieldMapper; import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprByteValue; import org.opensearch.sql.data.model.ExprCollectionValue; @@ -262,10 +262,11 @@ private static ExprValue parseDateTimeString(String value, OpenSearchDateType da DateFormatters.from(STRICT_YEAR_MONTH_DAY_FORMATTER.parse(value)).toLocalDate()); default: return new ExprTimestampValue( - DateFormatters.from(DATE_TIME_FORMATTER.parse(value)).toInstant()); + DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(value)) + .toInstant()); } - } catch (DateTimeParseException ignored) { - // ignored + } catch (DateTimeParseException | IllegalArgumentException ignored) { + // nothing to do, try another format } throw new IllegalArgumentException( diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 89dfd4dbdb2..42281044c82 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -78,6 +78,7 @@ class OpenSearchExprValueFactoryTest { .put("dateV", OpenSearchDateType.of(DATE)) .put("timeV", OpenSearchDateType.of(TIME)) .put("timestampV", OpenSearchDateType.of(TIMESTAMP)) + .put("timeonlyV", OpenSearchDateType.of("HH:mm:ss")) .put("datetimeDefaultV", OpenSearchDateType.of()) .put("dateStringV", OpenSearchDateType.of("date")) .put("timeStringV", OpenSearchDateType.of("time")) @@ -312,10 +313,6 @@ public void constructDatetime() { assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), tupleValue("{\"timestampV\":\"2015-01-01T12:10:30\"}").get("timestampV")), - () -> - assertEquals( - new ExprTimestampValue("2015-01-01 12:10:30"), - tupleValue("{\"timestampV\":\"2015-01-01 12:10:30\"}").get("timestampV")), () -> assertEquals( new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), @@ -348,10 +345,6 @@ public void constructDatetime() { assertEquals( new ExprTimestampValue("1984-05-10 20:30:40"), tupleValue("{ \"dateTimeCustomV\" : 19840510203040 }").get("dateTimeCustomV")), - () -> - assertEquals( - new ExprTimestampValue("2015-01-01 12:10:30"), - constructFromObject("timestampV", "2015-01-01 12:10:30")), () -> assertEquals( new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), @@ -361,7 +354,7 @@ public void constructDatetime() { () -> assertEquals( new ExprTimeValue("19:36:22"), - tupleValue("{\"timestampV\":\"19:36:22\"}").get("timestampV")), + tupleValue("{\"timeonlyV\":\"19:36:22\"}").get("timeonlyV")), // case: timestamp-formatted field, but it only gets a date: should match a date () -> @@ -384,10 +377,6 @@ public void constructDatetime_fromCustomFormat() { "Construct TIMESTAMP from \"2015-01-01 12-10-30\" failed, unsupported format.", exception.getMessage()); - assertEquals( - new ExprTimestampValue("2015-01-01 12:10:30"), - constructFromObject("customAndEpochMillisV", "2015-01-01 12:10:30")); - assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), constructFromObject("customAndEpochMillisV", "2015-01-01-12-10-30")); @@ -700,7 +689,7 @@ public void constructArrayOfCustomEpochMillisReturnsAll() { List.of( new ExprTimestampValue("2015-01-01 12:10:30"), new ExprTimestampValue("1999-11-09 01:09:44"))), - tupleValue("{\"customAndEpochMillisV\":[\"2015-01-01 12:10:30\",\"1999-11-09 01:09:44\"]}") + tupleValue("{\"customAndEpochMillisV\":[\"2015-01-01-12-10-30\",\"1999-11-09-01-09-44\"]}") .get("customAndEpochMillisV")); } From 9d8383629a1ce59bf0d5866a0191f165725c46ad Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Mon, 17 Mar 2025 19:45:11 -0700 Subject: [PATCH 2/7] Update comments Signed-off-by: Peng Huo --- .../sql/opensearch/data/value/OpenSearchExprValueFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 91b679104d5..21551f086cb 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -266,7 +266,7 @@ private static ExprValue parseDateTimeString(String value, OpenSearchDateType da .toInstant()); } } catch (DateTimeParseException | IllegalArgumentException ignored) { - // nothing to do, try another format + // ignored } throw new IllegalArgumentException( From c895fa4a7321186c83ae33b203d8d255bee3521f Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Tue, 15 Apr 2025 08:42:05 -0700 Subject: [PATCH 3/7] Integrate with YamlRestTest Signed-off-by: Peng Huo --- DEVELOPER_GUIDE.rst | 2 + integ-test/build.gradle | 20 ++++++++-- .../RestHandlerClientYamlTestSuiteIT.java | 23 +++++++++++ .../resources/rest-api-spec/api/ppl.json | 22 ++++++++++ .../rest-api-spec/api/query.settings.json | 22 ++++++++++ .../rest-api-spec/test/issues/2489.yml | 40 +++++++++++++++++++ 6 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 integ-test/src/yamlRestTest/java/org/opensearch/sql/rest/RestHandlerClientYamlTestSuiteIT.java create mode 100644 integ-test/src/yamlRestTest/resources/rest-api-spec/api/ppl.json create mode 100644 integ-test/src/yamlRestTest/resources/rest-api-spec/api/query.settings.json create mode 100644 integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml diff --git a/DEVELOPER_GUIDE.rst b/DEVELOPER_GUIDE.rst index 7f34e7bab31..f7798c9e921 100644 --- a/DEVELOPER_GUIDE.rst +++ b/DEVELOPER_GUIDE.rst @@ -231,6 +231,8 @@ Most of the time you just need to run ./gradlew build which will make sure you p - Run all unit tests. * - ./gradlew :integ-test:integTest - Run all integration test (this takes time). + * - ./gradlew :integ-test:yamlRestTest + - Run rest integration test. * - ./gradlew :doctest:doctest - Run doctests * - ./gradlew build diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 33f2e120bc2..d3a606b766a 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -40,6 +40,7 @@ plugins { apply plugin: 'opensearch.build' apply plugin: 'opensearch.rest-test' +apply plugin: 'opensearch.yaml-rest-test' apply plugin: 'java' apply plugin: 'io.freefair.lombok' apply plugin: 'com.wiredforcode.spawn' @@ -258,6 +259,13 @@ testClusters { plugin ":opensearch-sql-plugin" setting "plugins.query.datasources.encryption.masterkey", "1234567812345678" } + yamlRestTest { + testDistribution = 'archive' + plugin(getJobSchedulerPlugin()) + plugin(getGeoSpatialPlugin()) + plugin ":opensearch-sql-plugin" + setting "plugins.query.datasources.encryption.masterkey", "1234567812345678" + } remoteCluster { testDistribution = 'archive' plugin(getJobSchedulerPlugin()) @@ -435,10 +443,10 @@ integTest { } dependsOn ':opensearch-sql-plugin:bundlePlugin' - if(getOSFamilyType() != "windows") { - dependsOn startPrometheus - finalizedBy stopPrometheus - } +// if(getOSFamilyType() != "windows") { +// dependsOn startPrometheus +// finalizedBy stopPrometheus +// } // enable calcite codegen in IT systemProperty 'calcite.debug', 'false' @@ -501,6 +509,10 @@ integTest { // Exclude this IT, because they executed in another task (:integTestWithSecurity) exclude 'org/opensearch/sql/security/**' + + exclude 'org/opensearch/sql/ppl/**' + exclude 'org/opensearch/sql/datasource/**' + exclude 'org/opensearch/sql/calcite/**' } diff --git a/integ-test/src/yamlRestTest/java/org/opensearch/sql/rest/RestHandlerClientYamlTestSuiteIT.java b/integ-test/src/yamlRestTest/java/org/opensearch/sql/rest/RestHandlerClientYamlTestSuiteIT.java new file mode 100644 index 00000000000..3f30cf8dafa --- /dev/null +++ b/integ-test/src/yamlRestTest/java/org/opensearch/sql/rest/RestHandlerClientYamlTestSuiteIT.java @@ -0,0 +1,23 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.rest; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import org.opensearch.test.rest.yaml.ClientYamlTestCandidate; +import org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase; + +public class RestHandlerClientYamlTestSuiteIT extends OpenSearchClientYamlSuiteTestCase { + + public RestHandlerClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate) { + super(testCandidate); + } + + @ParametersFactory + public static Iterable parameters() throws Exception { + return OpenSearchClientYamlSuiteTestCase.createParameters(); + } +} diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/api/ppl.json b/integ-test/src/yamlRestTest/resources/rest-api-spec/api/ppl.json new file mode 100644 index 00000000000..6b6e56307c4 --- /dev/null +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/api/ppl.json @@ -0,0 +1,22 @@ +{ + "ppl": { + "documentation": { + "url": "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/index.rst", + "description": "OpenSearch PPL Reference Manual" + }, + "stability" : "stable", + "url": { + "paths": [ + { + "path" : "/_plugins/_ppl", + "methods" : ["POST"] + } + ] + }, + "params": {}, + "body": { + "description": "PPL Query", + "required":true + } + } +} diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/api/query.settings.json b/integ-test/src/yamlRestTest/resources/rest-api-spec/api/query.settings.json new file mode 100644 index 00000000000..82829d4e40b --- /dev/null +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/api/query.settings.json @@ -0,0 +1,22 @@ +{ + "query.settings": { + "documentation": { + "url": "https://github.com/opensearch-project/sql/blob/main/docs/user/admin/settings.rst", + "description": "Query Settings" + }, + "stability" : "stable", + "url": { + "paths": [ + { + "path" : "/_plugins/_query/settings", + "methods" : ["PUT"] + } + ] + }, + "params": {}, + "body": { + "description": "Query Settings", + "required":true + } + } +} diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml new file mode 100644 index 00000000000..520a7891b3d --- /dev/null +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml @@ -0,0 +1,40 @@ +setup: + - do: + indices.create: + index: test + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + properties: + timestamp: + type: date + - do: + query.settings: + body: + transient: + plugins.calcite.enabled : true + plugins.calcite.fallback.allowed : false +--- +"Handle epoch field in string format": + - skip: + features: + - headers + - do: + bulk: + index: test_1 + refresh: true + body: + - '{"index": {}}' + - '{"timestamp": "1705642934886"}' + - do: + headers: + Content-Type: 'application/json' + ppl: + body: + query: 'source=test | fields timestamp' + - match: {"total": 1} + - match: {"schema": [{"name": "timestamp", "type": "timestamp"}]} + - match: {"datarows": [["2024-01-19 05:42:14.886"]]} + From da257580f7fce6bdb3ac1f79dfe66870246e6f78 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Tue, 15 Apr 2025 08:47:46 -0700 Subject: [PATCH 4/7] Revert change Signed-off-by: Peng Huo --- integ-test/build.gradle | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/integ-test/build.gradle b/integ-test/build.gradle index d3a606b766a..6981a3c99ec 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -443,10 +443,10 @@ integTest { } dependsOn ':opensearch-sql-plugin:bundlePlugin' -// if(getOSFamilyType() != "windows") { -// dependsOn startPrometheus -// finalizedBy stopPrometheus -// } + if(getOSFamilyType() != "windows") { + dependsOn startPrometheus + finalizedBy stopPrometheus + } // enable calcite codegen in IT systemProperty 'calcite.debug', 'false' @@ -509,10 +509,6 @@ integTest { // Exclude this IT, because they executed in another task (:integTestWithSecurity) exclude 'org/opensearch/sql/security/**' - - exclude 'org/opensearch/sql/ppl/**' - exclude 'org/opensearch/sql/datasource/**' - exclude 'org/opensearch/sql/calcite/**' } From 8d0207c88d4a03a38c62d2eba5c161a463b4d342 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Tue, 15 Apr 2025 08:55:14 -0700 Subject: [PATCH 5/7] Fix YamlTest Signed-off-by: Peng Huo --- .../resources/rest-api-spec/test/issues/2489.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml index 520a7891b3d..58b1b204d48 100644 --- a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/2489.yml @@ -16,6 +16,16 @@ setup: transient: plugins.calcite.enabled : true plugins.calcite.fallback.allowed : false + +--- +teardown: + - do: + query.settings: + body: + transient: + plugins.calcite.enabled : false + plugins.calcite.fallback.allowed : true + --- "Handle epoch field in string format": - skip: @@ -23,7 +33,7 @@ setup: - headers - do: bulk: - index: test_1 + index: test refresh: true body: - '{"index": {}}' From e2733a438c7976013a0b94418ace1cefaa851db9 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Tue, 15 Apr 2025 09:57:30 -0700 Subject: [PATCH 6/7] Disable security.manager Signed-off-by: Peng Huo --- integ-test/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 6981a3c99ec..7b5bade79ce 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -419,6 +419,10 @@ task integTestWithSecurity(type: RestIntegTestTask) { } } +yamlRestTest { + systemProperty 'tests.security.manager', 'false' +} + // Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled integTest { useCluster testClusters.remoteCluster From 801ca25f6e29425acf556eca2bb7517dd1cab774 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Wed, 16 Apr 2025 08:21:23 -0700 Subject: [PATCH 7/7] Remove unused code Signed-off-by: Peng Huo --- .../java/org/opensearch/sql/utils/DateTimeFormatters.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java b/core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java index 18e6541514e..a5702b4f77a 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java +++ b/core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java @@ -109,13 +109,6 @@ public class DateTimeFormatters { public static final DateTimeFormatter SQL_LITERAL_DATE_TIME_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); - public static final DateTimeFormatter DATE_TIME_FORMATTER = - new DateTimeFormatterBuilder() - .appendOptional(SQL_LITERAL_DATE_TIME_FORMAT) - .appendOptional(STRICT_DATE_OPTIONAL_TIME_FORMATTER) - .appendOptional(STRICT_HOUR_MINUTE_SECOND_FORMATTER) - .toFormatter(); - /** todo. only support timestamp in format yyyy-MM-dd HH:mm:ss. */ public static final DateTimeFormatter DATE_TIME_FORMATTER_WITHOUT_NANO = SQL_LITERAL_DATE_TIME_FORMAT;