diff --git a/docs/dev/NewSQLEngine.md b/docs/dev/NewSQLEngine.md index 2b5f99f7eb..0be3370f33 100644 --- a/docs/dev/NewSQLEngine.md +++ b/docs/dev/NewSQLEngine.md @@ -59,10 +59,6 @@ For the following features unsupported in the new engine, the query will be forw You can find all the limitations in [Limitations](/docs/user/limitations/limitations.rst). -### 3.4 What if Something Wrong - -No panic! You can roll back to old query engine easily by a plugin setting change. Simply run the command to disable it by [plugin setting](/docs/user/admin/settings.rst#opendistro-sql-engine-new-enabled). Same as other cluster setting change, no need to restart OpenSearch and the change will take effect on next incoming query. Later on please report the issue to us. - --- ## 4.How it's Implemented diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index bd82e57126..c84c2bfd54 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -510,50 +510,6 @@ Result set:: } } -opensearch.sql.engine.new.enabled -================================= - -Description ------------ - -We are migrating existing functionalities to a new query engine under development. User can choose to enable the new engine if interested or disable if any issue found. - -1. The default value is true. -2. This setting is node scope. -3. This setting can be updated dynamically. - - -Example -------- - -You can update the setting with a new value like this. - -SQL query:: - - >> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_opensearch/_sql/settings -d '{ - "transient" : { - "opensearch.sql.engine.new.enabled" : "false" - } - }' - -Result set:: - - { - "acknowledged" : true, - "persistent" : { }, - "transient" : { - "opensearch" : { - "sql" : { - "engine" : { - "new" : { - "enabled" : "false" - } - } - } - } - } - } - opensearch.query.size_limit =========================== diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index c990d27eea..fa3b737c7c 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -14,8 +14,6 @@ Introduction Expressions, particularly value expressions, are those which return a scalar value. Expressions have different types and forms. For example, there are literal values as atom expression and arithmetic, predicate and function expression built on top of them. And also expressions can be used in different clauses, such as using arithmetic expression in ``SELECT``, ``WHERE`` or ``HAVING`` clause. -Note that before you try out examples using the SQL features in this doc, you need to enable the new query engine by following the steps in ``opensearch.sql.engine.new.enabled`` section in `Plugin Settings <../admin/settings.rst>`_. - Literal Values ============== diff --git a/docs/user/dql/newsql.rst b/docs/user/dql/newsql.rst deleted file mode 100644 index 6b1aa8296d..0000000000 --- a/docs/user/dql/newsql.rst +++ /dev/null @@ -1,35 +0,0 @@ -.. highlight:: sh - -============== -New SQL Engine -============== - -.. rubric:: Table of contents - -.. contents:: - :local: - :depth: 2 - -Introduction -============ - -To use the SQL features present in documentation correctly, you need to enable our new SQL query engine by the following command:: - - sh$ curl -sS -H 'Content-Type: application/json' \ - ... -X PUT localhost:9200/_opensearch/_sql/settings \ - ... -d '{"transient" : {"opensearch.sql.engine.new.enabled" : "true"}}' - { - "acknowledged": true, - "persistent": {}, - "transient": { - "opensearch": { - "sql": { - "engine": { - "new": { - "enabled": "true" - } - } - } - } - } - } diff --git a/integ-test/build.gradle b/integ-test/build.gradle index e1e1122146..d87e68ac9b 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -97,39 +97,8 @@ testClusters.all { plugin ":plugin" } -// Run only legacy SQL ITs with new SQL engine disabled -integTest { - dependsOn (':plugin:bundlePlugin',':integ-test:integTestWithNewEngine') - - systemProperty 'tests.security.manager', 'false' - systemProperty('project.root', project.projectDir.absolutePath) - - systemProperty "https", System.getProperty("https") - systemProperty "user", System.getProperty("user") - systemProperty "password", System.getProperty("password") - - // Enable new SQL engine - systemProperty 'enableNewEngine', 'false' - - // Set default query size limit - systemProperty 'defaultQuerySizeLimit', '10000' - - // Tell the test JVM if the cluster JVM is running under a debugger so that tests can use longer timeouts for - // requests. The 'doFirst' delays reading the debug setting on the cluster till execution time. - doFirst { systemProperty 'cluster.debug', getDebug()} - - if (System.getProperty("test.debug") != null) { - jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005' - } - - exclude 'org/opensearch/sql/ppl/**/*IT.class' - exclude 'org/opensearch/sql/sql/**/*IT.class' - exclude 'org/opensearch/sql/doctest/**/*IT.class' - exclude 'org/opensearch/sql/correctness/**' -} - // Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled -task integTestWithNewEngine(type: RestIntegTestTask) { +integTest { dependsOn ':plugin:bundlePlugin' systemProperty 'tests.security.manager', 'false' @@ -167,8 +136,6 @@ task integTestWithNewEngine(type: RestIntegTestTask) { } - - task docTest(type: RestIntegTestTask) { dependsOn ':plugin:bundlePlugin' diff --git a/integ-test/src/test/java/org/opensearch/sql/doctest/admin/PluginSettingIT.java b/integ-test/src/test/java/org/opensearch/sql/doctest/admin/PluginSettingIT.java index 45e3e0fe3b..bae91029fa 100644 --- a/integ-test/src/test/java/org/opensearch/sql/doctest/admin/PluginSettingIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/doctest/admin/PluginSettingIT.java @@ -45,7 +45,6 @@ import static org.opensearch.sql.legacy.plugin.SqlSettings.QUERY_RESPONSE_FORMAT; import static org.opensearch.sql.legacy.plugin.SqlSettings.QUERY_SLOWLOG; import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_ENABLED; -import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_NEW_ENGINE_ENABLED; import java.util.Arrays; import java.util.EnumSet; @@ -159,16 +158,6 @@ public void cursorDefaultContextKeepAliveSetting() { ); } - @Section(10) - public void sqlNewQueryEngineSetting() { - docSetting( - SQL_NEW_ENGINE_ENABLED, - "We are migrating existing functionalities to a new query engine under development. " + - "User can choose to enable the new engine if interested or disable if any issue found.", - true - ); - } - /** * Generate content for sample queries with setting changed to new value. * Finally setting will be reverted to avoid potential impact on other test cases. diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java index 8dd96b5ee6..703483bb56 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java @@ -94,7 +94,6 @@ public void noGroupKeyAvgOnIntegerShouldPass() { @Test public void hasGroupKeyAvgOnIntegerShouldPass() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest(String.format( "SELECT gender, AVG(age) as avg " + "FROM %s " + @@ -193,8 +192,6 @@ public void AddLiteralOnGroupKeyShouldPass() { @Test public void logWithAddLiteralOnGroupKeyShouldPass() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest(String.format( "SELECT gender, Log(age+10) as logAge, max(balance) as max " + "FROM %s " + @@ -272,8 +269,6 @@ public void groupByDateWithAliasShouldPass() { @Test public void aggregateCastStatementShouldNotReturnZero() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest(String.format( "SELECT SUM(CAST(male AS INT)) AS male_sum FROM %s", Index.BANK.getName())); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java index 94beb32128..f5f6f93bb4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java @@ -56,7 +56,6 @@ import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; -import org.junit.Assume; import org.junit.Ignore; import org.junit.Test; @@ -482,8 +481,6 @@ public void orderByAscTest() { @Test public void orderByAliasAscTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest(String.format("SELECT COUNT(*) as count FROM %s " + "GROUP BY gender ORDER BY count", TEST_INDEX_ACCOUNT)); @@ -506,8 +503,6 @@ public void orderByDescTest() throws IOException { @Test public void orderByAliasDescTest() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest(String.format("SELECT COUNT(*) as count FROM %s " + "GROUP BY gender ORDER BY count DESC", TEST_INDEX_ACCOUNT)); @@ -519,8 +514,6 @@ public void orderByAliasDescTest() throws IOException { @Test public void orderByGroupFieldWithAlias() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); - // ORDER BY field name JSONObject response = executeJdbcRequest(String.format("SELECT gender as g, COUNT(*) as count " + "FROM %s GROUP BY gender ORDER BY gender", TEST_INDEX_ACCOUNT)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/CsvFormatResponseIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/CsvFormatResponseIT.java index 80fb0b145e..d3ce0f0f1b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/CsvFormatResponseIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/CsvFormatResponseIT.java @@ -57,6 +57,7 @@ import org.junit.Assume; import org.junit.Ignore; import org.junit.Test; +import org.junit.jupiter.api.Disabled; import org.opensearch.client.Request; import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; @@ -119,9 +120,8 @@ public void specificPercentilesIntAndDouble() throws IOException { } } - @Test + @Ignore("only work for legacy engine") public void nestedObjectsAndArraysAreQuoted() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); final String query = String.format(Locale.ROOT, "SELECT * FROM %s WHERE _id = 5", TEST_INDEX_NESTED_TYPE); final String result = executeQueryWithStringOutput(query); @@ -135,9 +135,8 @@ public void nestedObjectsAndArraysAreQuoted() throws IOException { Assert.assertThat(result, containsString(expectedMessage)); } - @Test + @Ignore("only work for legacy engine") public void arraysAreQuotedInFlatMode() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); setFlatOption(true); final String query = String.format(Locale.ROOT, "SELECT * FROM %s WHERE _id = 5", @@ -157,7 +156,6 @@ public void arraysAreQuotedInFlatMode() throws IOException { @Test public void doubleQuotesAreEscapedWithDoubleQuotes() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); final String query = "SELECT * FROM " + TEST_INDEX_NESTED_WITH_QUOTES; final CSVResult csvResult = executeCsvRequest(query, false); @@ -190,7 +188,6 @@ public void fieldOrderOther() throws IOException { } @Ignore("Getting parser error") - @Test public void fieldOrderWithScriptFields() throws IOException { final String[] expectedFields = {"email", "script1", "script2", "gender", "address"}; @@ -332,19 +329,17 @@ public void joinSearchResultNotNestedNotFlatNoAggs() throws Exception { @Test public void simpleNumericValueAgg() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select count(*) from %s ", TEST_INDEX_DOG); CSVResult csvResult = executeCsvRequest(query, false); List headers = csvResult.getHeaders(); Assert.assertEquals(1, headers.size()); - Assert.assertEquals("COUNT(*)", headers.get(0)); + Assert.assertEquals("count(*)", headers.get(0)); List lines = csvResult.getLines(); Assert.assertEquals(1, lines.size()); - Assert.assertEquals("2.0", lines.get(0)); - + Assert.assertEquals("2", lines.get(0)); } @Test @@ -364,9 +359,8 @@ public void simpleNumericValueAggWithAlias() throws Exception { } - @Test + @Ignore("only work for legacy engine") public void twoNumericAggWithAlias() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select count(*) as count, avg(age) as myAlias from %s ", TEST_INDEX_DOG); @@ -381,17 +375,11 @@ public void twoNumericAggWithAlias() throws Exception { List lines = csvResult.getLines(); Assert.assertEquals(1, lines.size()); - if (headers.get(0).equals("count")) { - Assert.assertEquals("2.0,3.0", lines.get(0)); - } else { - Assert.assertEquals("3.0,2.0", lines.get(0)); - } - + Assert.assertEquals("2,3.0", lines.get(0)); } @Test public void aggAfterTermsGroupBy() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "SELECT COUNT(*) FROM %s GROUP BY gender", TEST_INDEX_ACCOUNT); CSVResult csvResult = executeCsvRequest(query, false); @@ -401,7 +389,7 @@ public void aggAfterTermsGroupBy() throws Exception { List lines = csvResult.getLines(); Assert.assertEquals(2, lines.size()); - assertThat(lines, containsInAnyOrder(equalTo("507.0"), equalTo("493.0"))); + assertThat(lines, containsInAnyOrder(equalTo("507"), equalTo("493"))); } @Test @@ -522,9 +510,8 @@ public void percentileAggregationTest() throws Exception { Assert.assertEquals("32.0,32.0,34.0,36.0,38.0,40.0,40.0", lines.get(0)); } - @Test + @Ignore("only work for legacy engine") public void includeTypeAndNotScore() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select age , firstname from %s where age > 31 limit 2", TEST_INDEX_ACCOUNT); @@ -541,7 +528,6 @@ public void includeTypeAndNotScore() throws Exception { @Test public void includeScoreAndNotType() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select age , firstname from %s where age > 31 order by _score desc limit 2 ", TEST_INDEX_ACCOUNT); @@ -558,7 +544,6 @@ public void includeScoreAndNotType() throws Exception { @Test public void includeScoreAndType() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select age , firstname from %s where age > 31 order by _score desc limit 2 ", TEST_INDEX_ACCOUNT); @@ -616,9 +601,8 @@ public void twoCharsSeperator() throws Exception { } - @Test + @Ignore("only work for legacy engine") public void includeIdAndNotTypeOrScore() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select age , firstname from %s where lastname = 'Marquez' ", TEST_INDEX_ACCOUNT); CSVResult csvResult = executeCsvRequest(query, false, false, false, true); @@ -631,9 +615,8 @@ public void includeIdAndNotTypeOrScore() throws Exception { Assert.assertTrue(lines.get(0).contains(",437") || lines.get(0).contains("437,")); } - @Test + @Ignore("only work for legacy engine") public void includeIdAndTypeButNoScore() throws Exception { - Assume.assumeFalse(isNewQueryEngineEabled()); String query = String.format(Locale.ROOT, "select age , firstname from %s where lastname = 'Marquez' ", TEST_INDEX_ACCOUNT); CSVResult csvResult = executeCsvRequest(query, false, false, true, true); @@ -648,9 +631,8 @@ public void includeIdAndTypeButNoScore() throws Exception { } //endregion Tests migrated from CSVResultsExtractorTests - @Test + @Ignore("only work for legacy engine") public void sensitiveCharacterSanitizeTest() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); String requestBody = "{" + " \"=cmd|' /C notepad'!_xlbgnm.A1\": \"+cmd|' /C notepad'!_xlbgnm.A1\",\n" + @@ -673,9 +655,8 @@ public void sensitiveCharacterSanitizeTest() throws IOException { Assert.assertTrue(lines.get(0).contains("'@cmd|' /C notepad'!_xlbgnm.A1")); } - @Test + @Ignore("only work for legacy engine") public void sensitiveCharacterSanitizeAndQuotedTest() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); String requestBody = "{" + " \"=cmd|' /C notepad'!_xlbgnm.A1,,\": \",+cmd|' /C notepad'!_xlbgnm.A1\",\n" + diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java index b9e2b1003e..0f8e7403b1 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java @@ -108,7 +108,6 @@ public void testSelectNestedFieldItself() { @Test public void testSelectObjectFieldOfArrayValuesItself() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); // Only the first element of the list of is returned. @@ -117,7 +116,6 @@ public void testSelectObjectFieldOfArrayValuesItself() { @Test public void testSelectObjectFieldOfArrayValuesInnerFields() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = new JSONObject(query("SELECT accounts.id FROM %s")); // Only the first element of the list of is returned. diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OrdinalAliasRewriterIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OrdinalAliasRewriterIT.java index a58b87b495..d61b46f004 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/OrdinalAliasRewriterIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/OrdinalAliasRewriterIT.java @@ -30,7 +30,7 @@ import static org.hamcrest.Matchers.equalTo; import java.io.IOException; -import org.junit.Assume; +import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.legacy.utils.StringUtils; @@ -167,10 +167,9 @@ public void explainSelectFieldiWithBacticksAndTableAliasOrderByOrdinal() throws } // explain ORDER BY IS NULL/NOT NULL - @Test + @Ignore("only work for legacy engine") public void explainSelectFieldiWithBacticksAndTableAliasOrderByOrdinalAndNull() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); String expected = explainQuery(StringUtils.format( "SELECT `b`.`lastname`, age FROM %s AS b ORDER BY `b`.`lastname` IS NOT NULL DESC, age is NULL LIMIT 3", TestsConstants.TEST_INDEX_ACCOUNT)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java index ee3790a5f2..985cb93b2a 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java @@ -46,7 +46,6 @@ import java.util.stream.Stream; import org.json.JSONArray; import org.json.JSONObject; -import org.junit.Assume; import org.junit.Ignore; import org.junit.Test; import org.opensearch.client.Request; @@ -284,9 +283,8 @@ public void groupByMultipleFields() throws IOException { assertContainsData(getDataRows(response), fields); } - @Test + @Ignore("only work for legacy engine") public void testSizeAndTotal() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeQuery( String.format(Locale.ROOT, "SELECT * " + "FROM %s " + @@ -348,10 +346,8 @@ public void aggregationFunctionInSelectCaseCheck() throws IOException { } } - @Test + @Ignore("only work for legacy engine") public void aggregationFunctionInSelectWithAlias() throws IOException { - Assume.assumeFalse(isNewQueryEngineEabled()); - JSONObject response = executeQuery( String.format(Locale.ROOT, "SELECT COUNT(*) AS total FROM %s GROUP BY age", TestsConstants.TEST_INDEX_ACCOUNT)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java index 6edd8393f1..327c5c6e5d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java @@ -231,8 +231,6 @@ public void castIntFieldToStringWithAliasTest() throws IOException { @Test public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest( "SELECT CAST(balance AS FLOAT) AS cast_balance FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY balance DESC LIMIT 1"); @@ -246,8 +244,6 @@ public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { @Test public void castIntFieldToFloatWithAliasJdbcFormatTest() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest( "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY jdbc_float_alias LIMIT 1"); @@ -389,8 +385,6 @@ public void castFieldToDatetimeWithGroupByJdbcFormatTest() { @Test public void castBoolFieldToNumericValueInSelectClause() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest( "SELECT " @@ -418,8 +412,6 @@ public void castBoolFieldToNumericValueInSelectClause() { @Test public void castBoolFieldToNumericValueWithGroupByAlias() { - Assume.assumeTrue(isNewQueryEngineEabled()); - JSONObject response = executeJdbcRequest( "SELECT " @@ -725,7 +717,6 @@ public void right() throws IOException { @Test public void ifFuncShouldPassJDBC() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT IF(age > 30, 'True', 'False') AS Ages FROM " + TEST_INDEX_ACCOUNT + " WHERE age IS NOT NULL GROUP BY Ages"); @@ -764,7 +755,6 @@ public void ifFuncWithNullInputAsConditionTest() throws IOException { @Test public void ifnullShouldPassJDBC() throws IOException { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT IFNULL(lastname, 'unknown') AS name FROM " + TEST_INDEX_ACCOUNT + " GROUP BY name"); @@ -795,7 +785,6 @@ public void ifnullWithNullInputTest() throws IOException { @Test public void isnullShouldPassJDBC() { - Assume.assumeTrue(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest( "SELECT ISNULL(lastname) AS name FROM " + TEST_INDEX_ACCOUNT); 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 36ba0fd612..369e13c931 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 @@ -98,7 +98,6 @@ public void setUpIndices() throws Exception { initClient(); } - configureNewQueryEngine(); resetQuerySizeLimit(); init(); } @@ -158,17 +157,6 @@ public static void cleanUpIndices() throws IOException { wipeAllClusterSettings(); } - private void configureNewQueryEngine() throws IOException { - boolean isEnabled = isNewQueryEngineEabled(); - if (!isEnabled) { - org.opensearch.sql.util.TestUtils.disableNewQueryEngine(client()); - } - } - - protected boolean isNewQueryEngineEabled() { - return Boolean.parseBoolean(System.getProperty("enableNewEngine", "true")); - } - protected void setQuerySizeLimit(Integer limit) throws IOException { updateClusterSettings( new ClusterSetting("transient", Settings.Key.QUERY_SIZE_LIMIT.getKeyValue(), limit.toString())); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java index fc657538fe..60a49a5172 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java @@ -31,7 +31,8 @@ import static org.opensearch.sql.util.MatcherUtils.verifySchema; import org.json.JSONObject; -import org.junit.Assume; + +import org.junit.Ignore; import org.junit.Test; public class TypeInformationIT extends SQLIntegTestCase { @@ -56,12 +57,11 @@ public void testAbsWithIntFieldReturnsInt() { @Test public void testCeilWithLongFieldReturnsLong() { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest("SELECT CEIL(balance) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY balance LIMIT 5"); - verifySchema(response, schema("CEIL(balance)", null, "long")); + verifySchema(response, schema("CEIL(balance)", null, "integer")); } /* @@ -80,20 +80,18 @@ public void testPiReturnsDouble() { */ @Test public void testUpperWithStringFieldReturnsString() { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest("SELECT UPPER(firstname) AS firstname_alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY firstname_alias LIMIT 2"); - verifySchema(response, schema("firstname_alias", null, "text")); + verifySchema(response, schema("UPPER(firstname)", "firstname_alias", "keyword")); } @Test public void testLowerWithTextFieldReturnsText() { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest("SELECT LOWER(firstname) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY firstname LIMIT 2"); - verifySchema(response, schema("LOWER(firstname)", null, "text")); + verifySchema(response, schema("LOWER(firstname)", null, "keyword")); } /* @@ -140,20 +138,18 @@ public void testRadiansWithLongFieldReturnsDouble() { */ @Test public void testAddWithIntReturnsInt() { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest("SELECT (balance + 5) AS balance_add_five FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY firstname LIMIT 2"); - verifySchema(response, schema("balance_add_five", null, "integer")); + verifySchema(response, schema("(balance + 5)", "balance_add_five", "long")); } @Test public void testSubtractLongWithLongReturnsLong() { - Assume.assumeFalse(isNewQueryEngineEabled()); JSONObject response = executeJdbcRequest("SELECT (balance - balance) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY firstname LIMIT 2"); - verifySchema(response, schema("subtract(balance, balance)", null, "long")); + verifySchema(response, schema("(balance - balance)", null, "long")); } /* diff --git a/integ-test/src/test/java/org/opensearch/sql/util/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/util/TestUtils.java index 806629a505..44f8db9020 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/TestUtils.java @@ -862,21 +862,4 @@ public static List> getPermutations(final List items) { return result; } - - /** - * Disable new query engine which is enabled by default. - */ - public static void disableNewQueryEngine(RestClient client) throws IOException { - Request request = new Request("PUT", SETTINGS_API_ENDPOINT); - request.setJsonEntity("{\"transient\" : {\"opensearch.sql.engine.new.enabled\" : \"false\"}}"); - - RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); - restOptionsBuilder.addHeader("Content-Type", "application/json"); - request.setOptions(restOptionsBuilder); - - Response response = client.performRequest(request); - Assert.assertEquals(RestStatus.OK, - RestStatus.fromCode(response.getStatusLine().getStatusCode())); - } - } diff --git a/legacy/build.gradle b/legacy/build.gradle index 13ace7186d..ea36f82bb6 100644 --- a/legacy/build.gradle +++ b/legacy/build.gradle @@ -109,7 +109,7 @@ dependencies { compileOnly group: 'javax.servlet', name: 'servlet-api', version:'2.5' testCompile group: 'org.hamcrest', name: 'hamcrest-core', version:'2.2' - testCompile group: 'org.mockito', name: 'mockito-core', version:'3.5.0' + testCompile group: 'org.mockito', name: 'mockito-inline', version:'3.5.0' testCompile group: 'junit', name: 'junit', version: '4.12' testCompile group: "org.opensearch.client", name: 'transport', version: "${opensearch_version}" diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index f75bbb3899..9a7d2242d5 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -34,7 +34,6 @@ import static org.opensearch.sql.legacy.plugin.SqlSettings.QUERY_ANALYSIS_SEMANTIC_SUGGESTION; import static org.opensearch.sql.legacy.plugin.SqlSettings.QUERY_ANALYSIS_SEMANTIC_THRESHOLD; import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_ENABLED; -import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_NEW_ENGINE_ENABLED; import com.alibaba.druid.sql.parser.ParserException; import com.google.common.collect.ImmutableList; @@ -165,7 +164,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli Format format = SqlRequestParam.getFormat(request.params()); - if (isNewEngineEnabled() && isCursorDisabled()) { + if (isCursorDisabled()) { // Route request to new query engine if it's supported already SQLQueryRequest newSqlRequest = new SQLQueryRequest(sqlRequest.getJsonContent(), sqlRequest.getSql(), request.path(), request.params()); @@ -281,10 +280,6 @@ private boolean isSQLFeatureEnabled() { return allowExplicitIndex && isSqlEnabled; } - private boolean isNewEngineEnabled() { - return LocalClusterState.state().getSettingValue(SQL_NEW_ENGINE_ENABLED); - } - private boolean isCursorDisabled() { Boolean isEnabled = LocalClusterState.state().getSettingValue(CURSOR_ENABLED); return Boolean.FALSE.equals(isEnabled); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SqlSettings.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SqlSettings.java index fdf2f9962c..7db1706aea 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SqlSettings.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SqlSettings.java @@ -49,7 +49,6 @@ public class SqlSettings { * 2) It has separate setting for Query and Fetch phase which are all OpenSearch internal concepts. */ public static final String SQL_ENABLED = "opensearch.sql.enabled"; - public static final String SQL_NEW_ENGINE_ENABLED = "opensearch.sql.engine.new.enabled"; public static final String QUERY_SLOWLOG = "opensearch.sql.query.slowlog"; public static final String QUERY_RESPONSE_FORMAT = "opensearch.sql.query.response.format"; public static final String QUERY_ANALYSIS_ENABLED = "opensearch.sql.query.analysis.enabled"; @@ -67,7 +66,6 @@ public class SqlSettings { public SqlSettings() { Map> settings = new HashMap<>(); settings.put(SQL_ENABLED, Setting.boolSetting(SQL_ENABLED, true, NodeScope, Dynamic)); - settings.put(SQL_NEW_ENGINE_ENABLED, Setting.boolSetting(SQL_NEW_ENGINE_ENABLED, true, NodeScope, Dynamic)); settings.put(QUERY_SLOWLOG, Setting.intSetting(QUERY_SLOWLOG, 2, NodeScope, Dynamic)); settings.put(QUERY_RESPONSE_FORMAT, Setting.simpleString(QUERY_RESPONSE_FORMAT, Format.JDBC.getFormatName(), NodeScope, Dynamic)); diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/planner/QueryPlannerTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/planner/QueryPlannerTest.java index 46895a4db7..235b489c97 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/planner/QueryPlannerTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/planner/QueryPlannerTest.java @@ -46,6 +46,8 @@ import org.junit.Before; import org.junit.Ignore; import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -68,6 +70,7 @@ import org.opensearch.sql.legacy.plugin.SqlSettings; import org.opensearch.sql.legacy.query.QueryAction; import org.opensearch.sql.legacy.query.SqlElasticRequestBuilder; +import org.opensearch.sql.legacy.query.join.BackOffRetryStrategy; import org.opensearch.sql.legacy.query.join.OpenSearchJoinQueryActionFactory; import org.opensearch.sql.legacy.query.planner.HashJoinQueryPlanRequestBuilder; import org.opensearch.sql.legacy.query.planner.core.QueryPlanner; @@ -166,19 +169,24 @@ protected SearchHits query(String sql, MockSearchHits mockHits1, MockSearchHits doAnswer(mockHits1).when(response1).getHits(); doAnswer(mockHits2).when(response2).getHits(); - ClearScrollRequestBuilder mockReqBuilder = mock(ClearScrollRequestBuilder.class); - when(client.prepareClearScroll()).thenReturn(mockReqBuilder); - when(mockReqBuilder.addScrollId(any())).thenReturn(mockReqBuilder); - when(mockReqBuilder.get()).thenAnswer(new Answer() { - @Override - public ClearScrollResponse answer(InvocationOnMock invocation) throws Throwable { - mockHits2.reset(); - return new ClearScrollResponse(true, 0); - } - }); - - List hits = plan(sql).execute(); - return new SearchHits(hits.toArray(new SearchHit[0]), new TotalHits(hits.size(), Relation.EQUAL_TO), 0); + try (MockedStatic backOffRetryStrategyMocked = + Mockito.mockStatic(BackOffRetryStrategy.class)) { + backOffRetryStrategyMocked.when(BackOffRetryStrategy::isHealthy).thenReturn(true); + + ClearScrollRequestBuilder mockReqBuilder = mock(ClearScrollRequestBuilder.class); + when(client.prepareClearScroll()).thenReturn(mockReqBuilder); + when(mockReqBuilder.addScrollId(any())).thenReturn(mockReqBuilder); + when(mockReqBuilder.get()).thenAnswer(new Answer() { + @Override + public ClearScrollResponse answer(InvocationOnMock invocation) throws Throwable { + mockHits2.reset(); + return new ClearScrollResponse(true, 0); + } + }); + + List hits = plan(sql).execute(); + return new SearchHits(hits.toArray(new SearchHit[0]), new TotalHits(hits.size(), Relation.EQUAL_TO), 0); + } } protected QueryPlanner plan(String sql) {