From 636f6557aa37076602276175848bfb01cd591b19 Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 20 May 2021 12:27:21 -0700 Subject: [PATCH 1/5] Disable DELETE clause by defaut and add opensearch.sql.delete.enabled setting Signed-off-by: penghuo --- .../org/opensearch/sql/legacy/DeleteIT.java | 2 + .../org/opensearch/sql/legacy/PluginIT.java | 23 ++++++++++ .../sql/legacy/plugin/RestSqlAction.java | 2 +- .../sql/legacy/plugin/SearchDao.java | 3 +- .../sql/legacy/plugin/SqlSettings.java | 3 ++ .../legacy/query/OpenSearchActionFactory.java | 26 ++++++++--- .../sql/legacy/unittest/JSONRequestTest.java | 45 ++++++++++++++++--- .../legacy/unittest/QueryFunctionsTest.java | 16 ++++--- .../unittest/WhereWithBoolConditionTest.java | 14 ++++-- .../sql/legacy/util/CheckScriptContents.java | 3 +- .../sql/legacy/util/SqlExplainUtils.java | 3 +- 11 files changed, 115 insertions(+), 25 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/DeleteIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/DeleteIT.java index f8aeb1127a..efbcd50b8a 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/DeleteIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/DeleteIT.java @@ -28,6 +28,7 @@ package org.opensearch.sql.legacy; import static org.hamcrest.core.IsEqual.equalTo; +import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_DELETE_ENABLED; import java.io.IOException; import org.json.JSONObject; @@ -40,6 +41,7 @@ public class DeleteIT extends SQLIntegTestCase { protected void init() throws Exception { loadIndex(Index.ACCOUNT); loadIndex(Index.PHRASE); + updateClusterSettings(new ClusterSetting(PERSISTENT, SQL_DELETE_ENABLED, "true")); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java index 72637c15a6..9cb14214ed 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java @@ -31,6 +31,7 @@ import static org.opensearch.sql.legacy.TestUtils.getResponseBody; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; import static org.opensearch.sql.legacy.plugin.RestSqlSettingsAction.SETTINGS_API_ENDPOINT; +import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_DELETE_ENABLED; import java.io.IOException; import java.util.Locale; @@ -40,6 +41,7 @@ import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; import org.opensearch.client.ResponseException; +import org.opensearch.sql.legacy.utils.StringUtils; public class PluginIT extends SQLIntegTestCase { @@ -75,6 +77,27 @@ public void sqlEnableSettingsTest() throws IOException { wipeAllClusterSettings(); } + @Test + public void sqlDeleteSettingsTest() throws IOException { + updateClusterSettings(new ClusterSetting(PERSISTENT, SQL_DELETE_ENABLED, "false")); + + String deleteQuery = StringUtils.format("DELETE FROM %s", TestsConstants.TEST_INDEX_ACCOUNT); + final ResponseException exception = + expectThrows(ResponseException.class, () -> executeQuery(deleteQuery)); + JSONObject actual = new JSONObject(TestUtils.getResponseBody(exception.getResponse())); + JSONObject expected = new JSONObject("{\n" + + " \"error\": {\n" + + " \"reason\": \"Invalid SQL query\",\n" + + " \"details\": \"DELETE clause is disabled by default and will be deprecated. Using the opendistro.sql.delete.enabled setting to enable it\",\n" + + " \"type\": \"SQLFeatureDisabledException\"\n" + + " },\n" + + " \"status\": 400\n" + + "}"); + assertTrue(actual.toString(), actual.similar(expected)); + + wipeAllClusterSettings(); + } + @Test public void sqlTransientOnlySettingTest() throws IOException { // (1) compact form 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 d69b95570a..ca4fa13d7c 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 @@ -214,7 +214,7 @@ private static void logAndPublishMetrics(final Exception e) { } private static QueryAction explainRequest(final NodeClient client, final SqlRequest sqlRequest, Format format) - throws SQLFeatureNotSupportedException, SqlParseException { + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { ColumnTypeProvider typeProvider = performAnalysis(sqlRequest.getSql()); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SearchDao.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SearchDao.java index 7bc7829447..81d2a42e33 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SearchDao.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/SearchDao.java @@ -31,6 +31,7 @@ import java.util.Set; import org.opensearch.client.Client; import org.opensearch.sql.legacy.domain.QueryActionRequest; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.query.OpenSearchActionFactory; import org.opensearch.sql.legacy.query.QueryAction; @@ -67,7 +68,7 @@ public Client getClient() { * @throws SqlParseException */ public QueryAction explain(QueryActionRequest queryActionRequest) - throws SqlParseException, SQLFeatureNotSupportedException { + throws SqlParseException, SQLFeatureNotSupportedException, SQLFeatureDisabledException { return OpenSearchActionFactory.create(client, queryActionRequest); } } 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 404798a7d7..0bc9529fd3 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 @@ -52,6 +52,7 @@ public class SqlSettings { public static final String QUERY_SLOWLOG = "opensearch.sql.query.slowlog"; public static final String METRICS_ROLLING_WINDOW = "opensearch.sql.metrics.rollingwindow"; public static final String METRICS_ROLLING_INTERVAL = "opensearch.sql.metrics.rollinginterval"; + public static final String SQL_DELETE_ENABLED = "opensearch.sql.delete.enabled"; public static final String CURSOR_ENABLED= "opensearch.sql.cursor.enabled"; public static final String CURSOR_FETCH_SIZE = "opensearch.sql.cursor.fetch_size"; @@ -65,6 +66,8 @@ public SqlSettings() { Map> settings = new HashMap<>(); settings.put(SQL_ENABLED, Setting.boolSetting(SQL_ENABLED, true, NodeScope, Dynamic)); settings.put(QUERY_SLOWLOG, Setting.intSetting(QUERY_SLOWLOG, 2, NodeScope, Dynamic)); + settings.put(SQL_DELETE_ENABLED, Setting.boolSetting(SQL_DELETE_ENABLED, false, NodeScope, + Dynamic)); settings.put(METRICS_ROLLING_WINDOW, Setting.longSetting(METRICS_ROLLING_WINDOW, 3600L, 2L, NodeScope, Dynamic)); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/query/OpenSearchActionFactory.java b/legacy/src/main/java/org/opensearch/sql/legacy/query/OpenSearchActionFactory.java index ec7cb986d8..713d7bd37b 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/query/OpenSearchActionFactory.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/query/OpenSearchActionFactory.java @@ -27,6 +27,7 @@ package org.opensearch.sql.legacy.query; import static org.opensearch.sql.legacy.domain.IndexStatement.StatementType; +import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_DELETE_ENABLED; import static org.opensearch.sql.legacy.utils.Util.toSqlExpr; import com.alibaba.druid.sql.ast.expr.SQLAggregateExpr; @@ -56,6 +57,7 @@ import org.opensearch.sql.legacy.domain.QueryActionRequest; import org.opensearch.sql.legacy.domain.Select; import org.opensearch.sql.legacy.esdomain.LocalClusterState; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.executor.ElasticResultHandler; import org.opensearch.sql.legacy.executor.Format; @@ -79,11 +81,12 @@ import org.opensearch.sql.legacy.rewriter.ordinal.OrdinalRewriterRule; import org.opensearch.sql.legacy.rewriter.parent.SQLExprParentSetterRule; import org.opensearch.sql.legacy.rewriter.subquery.SubQueryRewriteRule; +import org.opensearch.sql.legacy.utils.StringUtils; public class OpenSearchActionFactory { public static QueryAction create(Client client, String sql) - throws SqlParseException, SQLFeatureNotSupportedException { + throws SqlParseException, SQLFeatureNotSupportedException, SQLFeatureDisabledException { return create(client, new QueryActionRequest(sql, new ColumnTypeProvider(), Format.JSON)); } @@ -95,7 +98,7 @@ public static QueryAction create(Client client, String sql) * @return Query object. */ public static QueryAction create(Client client, QueryActionRequest request) - throws SqlParseException, SQLFeatureNotSupportedException { + throws SqlParseException, SQLFeatureNotSupportedException, SQLFeatureDisabledException { String sql = request.getSql(); // Remove line breaker anywhere and semicolon at the end sql = sql.replaceAll("\\R", " ").trim(); @@ -138,10 +141,17 @@ public static QueryAction create(Client client, QueryActionRequest request) return handleSelect(client, select); } case "DELETE": - SQLStatementParser parser = createSqlStatementParser(sql); - SQLDeleteStatement deleteStatement = parser.parseDeleteStatement(); - Delete delete = new SqlParser().parseDelete(deleteStatement); - return new DeleteQueryAction(client, delete); + if (isSQLDeleteEnabled()) { + SQLStatementParser parser = createSqlStatementParser(sql); + SQLDeleteStatement deleteStatement = parser.parseDeleteStatement(); + Delete delete = new SqlParser().parseDelete(deleteStatement); + return new DeleteQueryAction(client, delete); + } else { + throw new SQLFeatureDisabledException( + StringUtils.format("DELETE clause is disabled by default and will be " + + "deprecated. Using the %s setting to enable it", + SQL_DELETE_ENABLED)); + } case "SHOW": IndexStatement showStatement = new IndexStatement(StatementType.SHOW, sql); return new ShowQueryAction(client, showStatement); @@ -154,6 +164,10 @@ public static QueryAction create(Client client, QueryActionRequest request) } } + private static boolean isSQLDeleteEnabled() { + return LocalClusterState.state().getSettingValue(SQL_DELETE_ENABLED); + } + private static String getFirstWord(String sql) { int endOfFirstWord = sql.indexOf(' '); return sql.substring(0, endOfFirstWord > 0 ? endOfFirstWord : sql.length()).toUpperCase(); diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/JSONRequestTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/JSONRequestTest.java index 4466e8df0c..ea19572ca4 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/JSONRequestTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/JSONRequestTest.java @@ -29,7 +29,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Matchers.anyInt; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.alibaba.druid.sql.parser.ParserException; @@ -41,14 +43,20 @@ import org.json.JSONObject; import org.junit.Before; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.opensearch.client.Client; +import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.legacy.domain.ColumnTypeProvider; import org.opensearch.sql.legacy.domain.QueryActionRequest; +import org.opensearch.sql.legacy.esdomain.LocalClusterState; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.executor.Format; import org.opensearch.sql.legacy.executor.format.Schema; @@ -306,15 +314,39 @@ public void aggregationQuery() throws IOException { @Test public void deleteSanity() throws IOException { - String result = explain(String.format("{\"query\":\"" + + try (MockedStatic localClusterStateMockedStatic = + Mockito.mockStatic(LocalClusterState.class)) { + LocalClusterState state = mock(LocalClusterState.class); + localClusterStateMockedStatic.when(LocalClusterState::state).thenReturn(state); + when(state.getSettingValue(anyString())).thenReturn(true); + + String result = explain(String.format("{\"query\":\"" + "DELETE " + "FROM %s " + "WHERE firstname LIKE 'A%%' AND age > 20\"}", TestsConstants.TEST_INDEX_ACCOUNT)); - String expectedOutput = Files.toString( + String expectedOutput = Files.toString( new File(getResourcePath() + "src/test/resources/expectedOutput/delete_explain.json"), StandardCharsets.UTF_8) .replaceAll("\r", ""); + assertThat(removeSpaces(result), equalTo(removeSpaces(expectedOutput))); + } + } - assertThat(removeSpaces(result), equalTo(removeSpaces(expectedOutput))); + @Test(expected = SQLFeatureDisabledException.class) + public void deleteShouldThrowExceptionWhenDisabled() + throws SQLFeatureDisabledException, SQLFeatureNotSupportedException, + SqlParseException { + try (MockedStatic localClusterStateMockedStatic = + Mockito.mockStatic(LocalClusterState.class)) { + LocalClusterState state = mock(LocalClusterState.class); + localClusterStateMockedStatic.when(LocalClusterState::state).thenReturn(state); + when(state.getSettingValue(anyString())).thenReturn(false); + + JSONObject jsonRequest = new JSONObject(StringUtils.format("{\"query\":\"" + + "DELETE " + + "FROM %s " + + "WHERE firstname LIKE 'A%%' AND age > 20\"}", TestsConstants.TEST_INDEX_ACCOUNT)); + translate(jsonRequest.getString("query"), jsonRequest); + } } @Test @@ -366,13 +398,14 @@ private String explain(String request) { String sql = jsonRequest.getString("query"); return translate(sql, jsonRequest); - } catch (SqlParseException | SQLFeatureNotSupportedException e) { + } catch (SqlParseException | SQLFeatureNotSupportedException | SQLFeatureDisabledException e) { throw new ParserException("Illegal sql expr in request: " + request); } } - private String translate(String sql, JSONObject jsonRequest) throws SQLFeatureNotSupportedException, SqlParseException { - Client mockClient = Mockito.mock(Client.class); + private String translate(String sql, JSONObject jsonRequest) + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { + Client mockClient = mock(Client.class); CheckScriptContents.stubMockClient(mockClient); QueryAction queryAction = OpenSearchActionFactory diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/QueryFunctionsTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/QueryFunctionsTest.java index a828529755..b22fad1f1f 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/QueryFunctionsTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/QueryFunctionsTest.java @@ -48,6 +48,7 @@ import org.opensearch.index.query.AbstractQueryBuilder; import org.opensearch.index.query.MultiMatchQueryBuilder; import org.opensearch.search.builder.SearchSourceBuilder.ScriptField; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.query.OpenSearchActionFactory; import org.opensearch.sql.legacy.util.CheckScriptContents; @@ -283,27 +284,32 @@ public void isNullWithMathExpr() { } @Test(expected = SQLFeatureNotSupportedException.class) - public void emptyQueryShouldThrowSQLFeatureNotSupportedException() throws SQLFeatureNotSupportedException, SqlParseException { + public void emptyQueryShouldThrowSQLFeatureNotSupportedException() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { OpenSearchActionFactory.create(Mockito.mock(Client.class), ""); } @Test(expected = SQLFeatureNotSupportedException.class) - public void emptyNewLineQueryShouldThrowSQLFeatureNotSupportedException() throws SQLFeatureNotSupportedException, SqlParseException { + public void emptyNewLineQueryShouldThrowSQLFeatureNotSupportedException() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { OpenSearchActionFactory.create(Mockito.mock(Client.class), "\n"); } @Test(expected = SQLFeatureNotSupportedException.class) - public void emptyNewLineQueryShouldThrowSQLFeatureNotSupportedException2() throws SQLFeatureNotSupportedException, SqlParseException { + public void emptyNewLineQueryShouldThrowSQLFeatureNotSupportedException2() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { OpenSearchActionFactory.create(Mockito.mock(Client.class), "\r\n"); } @Test(expected = SQLFeatureNotSupportedException.class) - public void queryWithoutSpaceShouldSQLFeatureNotSupportedException() throws SQLFeatureNotSupportedException, SqlParseException { + public void queryWithoutSpaceShouldSQLFeatureNotSupportedException() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { OpenSearchActionFactory.create(Mockito.mock(Client.class), "SELE"); } @Test(expected = SQLFeatureNotSupportedException.class) - public void spacesOnlyQueryShouldThrowSQLFeatureNotSupportedException() throws SQLFeatureNotSupportedException, SqlParseException { + public void spacesOnlyQueryShouldThrowSQLFeatureNotSupportedException() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { OpenSearchActionFactory.create(Mockito.mock(Client.class), " "); } diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/WhereWithBoolConditionTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/WhereWithBoolConditionTest.java index 61664fe91a..97f6666649 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/WhereWithBoolConditionTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/WhereWithBoolConditionTest.java @@ -37,6 +37,7 @@ import org.junit.Test; import org.mockito.Mockito; import org.opensearch.client.Client; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.query.OpenSearchActionFactory; import org.opensearch.sql.legacy.query.QueryAction; @@ -48,12 +49,15 @@ public class WhereWithBoolConditionTest { @Test - public void whereWithBoolCompilationTest() throws SQLFeatureNotSupportedException, SqlParseException { + public void whereWithBoolCompilationTest() + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { query(StringUtils.format("SELECT * FROM %s WHERE male = false", TestsConstants.TEST_INDEX_BANK)); } @Test - public void selectAllTest() throws SQLFeatureNotSupportedException, SqlParseException, IOException { + public void selectAllTest() + throws SQLFeatureNotSupportedException, SqlParseException, IOException, + SQLFeatureDisabledException { String expectedOutput = Files.toString( new File(getResourcePath() + "src/test/resources/expectedOutput/select_where_true.json"), StandardCharsets.UTF_8) .replaceAll("\r", ""); @@ -70,11 +74,13 @@ public void selectAllTest() throws SQLFeatureNotSupportedException, SqlParseExce ); } - private String query(String query) throws SQLFeatureNotSupportedException, SqlParseException { + private String query(String query) + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { return explain(query); } - private String explain(String sql) throws SQLFeatureNotSupportedException, SqlParseException { + private String explain(String sql) + throws SQLFeatureNotSupportedException, SqlParseException, SQLFeatureDisabledException { Client mockClient = Mockito.mock(Client.class); CheckScriptContents.stubMockClient(mockClient); QueryAction queryAction = OpenSearchActionFactory.create(mockClient, sql); diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/util/CheckScriptContents.java b/legacy/src/test/java/org/opensearch/sql/legacy/util/CheckScriptContents.java index 3c3800133f..83adccc32c 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/util/CheckScriptContents.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/util/CheckScriptContents.java @@ -68,6 +68,7 @@ import org.opensearch.sql.legacy.domain.Select; import org.opensearch.sql.legacy.domain.Where; import org.opensearch.sql.legacy.esdomain.LocalClusterState; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.parser.ElasticSqlExprParser; import org.opensearch.sql.legacy.parser.ScriptFilter; @@ -97,7 +98,7 @@ public static ScriptField getScriptFieldFromQuery(String query) { return scriptFields.get(0); - } catch (SQLFeatureNotSupportedException | SqlParseException e) { + } catch (SQLFeatureNotSupportedException | SqlParseException | SQLFeatureDisabledException e) { throw new ParserException("Unable to parse query: " + query, e); } } diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/util/SqlExplainUtils.java b/legacy/src/test/java/org/opensearch/sql/legacy/util/SqlExplainUtils.java index 9edbabcc01..1d7cfe1020 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/util/SqlExplainUtils.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/util/SqlExplainUtils.java @@ -30,6 +30,7 @@ import java.sql.SQLFeatureNotSupportedException; import org.mockito.Mockito; import org.opensearch.client.Client; +import org.opensearch.sql.legacy.exception.SQLFeatureDisabledException; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.query.OpenSearchActionFactory; import org.opensearch.sql.legacy.query.QueryAction; @@ -46,7 +47,7 @@ public static String explain(String query) { QueryAction queryAction = OpenSearchActionFactory.create(mockClient, query); return queryAction.explain().explain(); - } catch (SqlParseException | SQLFeatureNotSupportedException e) { + } catch (SqlParseException | SQLFeatureNotSupportedException | SQLFeatureDisabledException e) { throw new ParserException("Illegal sql expr in: " + query); } } From a7189cf3d0a991e047f996941b7e09da843cf2cf Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 20 May 2021 13:36:12 -0700 Subject: [PATCH 2/5] Fix PluginIT Signed-off-by: penghuo --- .../org/opensearch/sql/legacy/PluginIT.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java index 9cb14214ed..5e49123855 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java @@ -85,14 +85,17 @@ public void sqlDeleteSettingsTest() throws IOException { final ResponseException exception = expectThrows(ResponseException.class, () -> executeQuery(deleteQuery)); JSONObject actual = new JSONObject(TestUtils.getResponseBody(exception.getResponse())); - JSONObject expected = new JSONObject("{\n" - + " \"error\": {\n" - + " \"reason\": \"Invalid SQL query\",\n" - + " \"details\": \"DELETE clause is disabled by default and will be deprecated. Using the opendistro.sql.delete.enabled setting to enable it\",\n" - + " \"type\": \"SQLFeatureDisabledException\"\n" - + " },\n" - + " \"status\": 400\n" - + "}"); + JSONObject expected = + new JSONObject( + "{\n" + + " \"error\": {\n" + + " \"reason\": \"Invalid SQL query\",\n" + + " \"details\": \"DELETE clause is disabled by default and will be deprecated. Using " + + "the opensearch.sql.delete.enabled setting to enable it\",\n" + + " \"type\": \"SQLFeatureDisabledException\"\n" + + " },\n" + + " \"status\": 400\n" + + "}"); assertTrue(actual.toString(), actual.similar(expected)); wipeAllClusterSettings(); From 6c472558b1c7a01c12a47b423c03680744dd80cb Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 20 May 2021 14:51:13 -0700 Subject: [PATCH 3/5] Update Docs --- docs/user/admin/settings.rst | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index 643c2ef3d9..f3eb3ff59d 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -275,3 +275,66 @@ Result set:: } } +opensearch.sql.delete.enabled +====================== + +Description +----------- + +By default, DELETE clause disabled. You can enable DELETE clause by this setting. + +1. The default value is false. +2. This setting is node scope. +3. This setting can be updated dynamically. + + +Example 1 +--------- + +You can update the setting with a new value like this. + +SQL query:: + + >> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_sql/settings -d '{ + "transient" : { + "opensearch.sql.delete.enabled" : "false" + } + }' + +Result set:: + + { + "acknowledged" : true, + "persistent" : { }, + "transient" : { + "opensearch" : { + "sql" : { + "delete": { + "enabled" : "false" + } + } + } + } + } + +Example 2 +--------- + +Query result after the setting updated is like: + +SQL query:: + + >> curl -H 'Content-Type: application/json' -X POST localhost:9200/_plugins/_sql -d '{ + "query" : "DELETE * FROM accounts" + }' + +Result set:: + + { + "error" : { + "reason" : "Invalid SQL query", + "details" : "DELETE clause is disabled by default and will be deprecated. Using the opensearch.sql.delete.enabled setting to enable it", + "type" : "SQLFeatureDisabledException" + }, + "status" : 400 + } From bdcfdcdfdfbb17380109cfe048be81fd05dc028c Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 20 May 2021 16:08:18 -0700 Subject: [PATCH 4/5] Update docs Signed-off-by: penghuo --- docs/category.json | 3 +- docs/user/admin/settings.rst | 60 +++++++++++++++--------------------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/docs/category.json b/docs/category.json index 26e7a1d954..0aa925daf6 100644 --- a/docs/category.json +++ b/docs/category.json @@ -3,7 +3,8 @@ "experiment/ppl/interfaces/endpoint.rst", "experiment/ppl/interfaces/protocol.rst", "experiment/ppl/admin/settings.rst", - "user/optimization/optimization.rst" + "user/optimization/optimization.rst", + "user/admin/settings.rst" ], "ppl_cli": [ "experiment/ppl/cmd/dedup.rst", diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index f3eb3ff59d..fbfde2dea5 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -295,27 +295,21 @@ You can update the setting with a new value like this. SQL query:: - >> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_sql/settings -d '{ - "transient" : { - "opensearch.sql.delete.enabled" : "false" - } - }' - -Result set:: - - { - "acknowledged" : true, - "persistent" : { }, - "transient" : { - "opensearch" : { - "sql" : { - "delete": { - "enabled" : "false" - } - } - } - } - } + sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_sql/settings \ + ... -d '{"transient":{"opensearch.sql.delete.enabled":"false"}}' + { + "acknowledged": true, + "persistent": {}, + "transient": { + "opensearch": { + "sql": { + "delete": { + "enabled": "false" + } + } + } + } + } Example 2 --------- @@ -324,17 +318,13 @@ Query result after the setting updated is like: SQL query:: - >> curl -H 'Content-Type: application/json' -X POST localhost:9200/_plugins/_sql -d '{ - "query" : "DELETE * FROM accounts" - }' - -Result set:: - - { - "error" : { - "reason" : "Invalid SQL query", - "details" : "DELETE clause is disabled by default and will be deprecated. Using the opensearch.sql.delete.enabled setting to enable it", - "type" : "SQLFeatureDisabledException" - }, - "status" : 400 - } + sh$ curl -sS -H 'Content-Type: application/json' -X POST localhost:9200/_opendistro/_sql \ + ... -d '{"query" : "DELETE * FROM accounts"}' + { + "error": { + "reason": "Invalid SQL query", + "details": "DELETE clause is disabled by default and will be deprecated. Using the opensearch.sql.delete.enabled setting to enable it", + "type": "SQLFeatureDisabledException" + }, + "status": 400 + } From 0780edea1131ecb0f2cab3076d158aef28ec6927 Mon Sep 17 00:00:00 2001 From: penghuo Date: Fri, 21 May 2021 08:51:55 -0700 Subject: [PATCH 5/5] Change endpoint name Signed-off-by: penghuo --- docs/user/admin/settings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index fbfde2dea5..31643865fd 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -318,7 +318,7 @@ Query result after the setting updated is like: SQL query:: - sh$ curl -sS -H 'Content-Type: application/json' -X POST localhost:9200/_opendistro/_sql \ + sh$ curl -sS -H 'Content-Type: application/json' -X POST localhost:9200/_plugins/_sql \ ... -d '{"query" : "DELETE * FROM accounts"}' { "error": {