Remove cursor enabling and fetch size setting (#75)

* Remove cursor enabling and fetch size setting Signed-off-by: Chen Dai <[email protected]> * Update cursor docs with the changes Signed-off-by: Chen Dai <[email protected]> * Update cursor docs with the changes Signed-off-by: Chen Dai <[email protected]>
opensearch-project · May 20, 2021 · c1f3cc8 · c1f3cc8
1 parent ae0c482
commit c1f3cc8
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 272 deletions.
diff --git a/docs/dev/Pagination.md b/docs/dev/Pagination.md
@@ -425,8 +425,8 @@ Right now there is inconsistency in results for `csv` and `jdbc` format. This is
 
 - By default all requests will be a cursor request - meaning the response will contain `cursor` key to fetch next page of result. This is true for all queries which cursor is supported.
 - Cursor is supported only via `POST` HTTP request.
-- If `fetch_size` is omitted from request, it will default to **1000**, unless overridden by cluster settings (See below).
-- A `fetch_size` value of **0**, will imply no cursor and query will fallback to non-cursor behavior. This will allow to use/not-use cursor on a per query basis.
+- If `fetch_size` is omitted from request, the query will fallback to non-cursor behavior.
+- A `fetch_size` value of **0**, will imply no cursor and query will fallback to non-cursor behavior too. This will allow to use/not-use cursor on a per query basis.
 - If SQL query limit is less than `fetch_size`, no cursor context will be open and all results will be fetched in first page.
 - Negative or non-numeric values of `fetch_size` will throw `400` exception.
 - If `cursor` is given as JSON field in request, other fields like `fetch_size` , `query`, `filter`, `parameters` will be ignored. 
@@ -439,84 +439,6 @@ When OpenSearch bootstraps, SQL plugin will register a few settings in OpenSearc
 Most of the settings are able to change dynamically so you can control the behavior of SQL plugin without need to bounce your cluster.
 For cursors we will be exposing the following settings:
 
-####  opensearch.sql.cursor.enabled
-
-You can disable cursor for all SQL queries which support pagination.
-
-- The default value is **true**.
-- This setting is node scope.
-- This setting can be updated dynamically.
-- This can be `persistent` and `transient`.
-
-Example:
-
-```
->> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings -d '{
-  "transient" : {
-    "opensearch.sql.cursor.enabled" : "false"
-  }
-}'
-```
-
-Response:
-
-```
-{
-  "acknowledged" : true,
-  "persistent" : { },
-  "transient" : {
-    "opensearch" : {
-      "sql" : {
-        "cursor" : {
-          "enabled" : "false"
-        }
-      }
-    }
-  }
-}
-
-```
-
-####  opensearch.sql.cursor.fetch_size
-
-This setting controls the default page size for all cursor requests.
-
-- The default value is **1000**.
-- The minimum value is **1**.
-- The effective max value is controlled by `index.max_result_window` setting. Increase the fetch_size above this will give a 500 error from OpenSearch.
-- This setting is node scope.
-- This setting can be updated dynamically.
-- This can be `persistent` and `transient`.
-
-Example:
-
-```
->> curl -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings -d '{
-  "persistent" : {
-    "opensearch.sql.cursor.fetch_size" : "100"
-  }
-}'
-```
-
-Response:
-
-```
-{
-  "acknowledged" : true,
-  "transient" : { },
-  "persistent" : {
-    "opensearch" : {
-      "sql" : {
-        "cursor" : {
-          "fetch_size" : "100"
-        }
-      }
-    }
-  }
-}
-
-```
-
 ####  opensearch.sql.cursor.keep_alive
 
 This setting controls the how long the cursor context is open for all cursor requests.

diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst
@@ -121,90 +121,6 @@ Result set::
 	  }
 	}
 
-opensearch.sql.cursor.enabled
-=============================
-
-Description
------------
-
-User can enable/disable pagination for all queries that are supported.
-
-1. The default value is false.
-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/_plugins/_sql/settings -d '{
-	  "transient" : {
-	    "opensearch.sql.cursor.enabled" : "true"
-	  }
-	}'
-
-Result set::
-
-	{
-	  "acknowledged" : true,
-	  "persistent" : { },
-	  "transient" : {
-	    "opensearch" : {
-	      "sql" : {
-	        "cursor" : {
-	          "enabled" : "true"
-	        }
-	      }
-	    }
-	  }
-	}
-
-opensearch.sql.cursor.fetch_size
-================================
-
-Description
------------
-
-User can set the default fetch_size for all queries that are supported by pagination. Explicit `fetch_size` passed in request will override this value
-
-1. The default value is 1000.
-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/_plugins/_sql/settings -d '{
-	  "transient" : {
-	    "opensearch.sql.cursor.fetch_size" : "50"
-	  }
-	}'
-
-Result set::
-
-	{
-	  "acknowledged" : true,
-	  "persistent" : { },
-	  "transient" : {
-	    "opensearch" : {
-	      "sql" : {
-	        "cursor" : {
-	          "fetch_size" : "50"
-	        }
-	      }
-	    }
-	  }
-	}
-
 opensearch.sql.cursor.keep_alive
 ================================
 

diff --git a/docs/user/interfaces/endpoint.rst b/docs/user/interfaces/endpoint.rst
@@ -97,7 +97,7 @@ Cursor
 Description
 -----------
 
-To get paginated response for a query, user needs to provide `fetch_size` parameter as part of normal query. The value of `fetch_size` should be greater than `0`. In absence of `fetch_size`, default value of 1000 is used. A value of `0` will fallback to non-paginated response. This feature is only available over `jdbc` format for now.
+To get paginated response for a query, user needs to provide `fetch_size` parameter as part of normal query. The value of `fetch_size` should be greater than `0`. In absence of `fetch_size` or a value of `0`, it will fallback to non-paginated response. This feature is only available over `jdbc` format for now.
 
 Example
 -------

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
@@ -36,22 +36,17 @@
 import static org.opensearch.sql.doctest.core.request.SqlRequestFormat.IGNORE_REQUEST;
 import static org.opensearch.sql.doctest.core.response.SqlResponseFormat.IGNORE_RESPONSE;
 import static org.opensearch.sql.doctest.core.response.SqlResponseFormat.PRETTY_JSON_RESPONSE;
-import static org.opensearch.sql.legacy.plugin.SqlSettings.CURSOR_ENABLED;
-import static org.opensearch.sql.legacy.plugin.SqlSettings.CURSOR_FETCH_SIZE;
 import static org.opensearch.sql.legacy.plugin.SqlSettings.CURSOR_KEEPALIVE;
 import static org.opensearch.sql.legacy.plugin.SqlSettings.QUERY_SLOWLOG;
 import static org.opensearch.sql.legacy.plugin.SqlSettings.SQL_ENABLED;
 
-import java.util.Arrays;
 import java.util.EnumSet;
-import java.util.stream.Collectors;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.sql.doctest.core.DocTest;
 import org.opensearch.sql.doctest.core.annotation.DocTestConfig;
 import org.opensearch.sql.doctest.core.annotation.Section;
 import org.opensearch.sql.doctest.core.builder.Example;
 import org.opensearch.sql.doctest.core.builder.ListItems;
-import org.opensearch.sql.legacy.executor.Format;
 import org.opensearch.sql.legacy.plugin.SqlSettings;
 import org.opensearch.sql.legacy.utils.StringUtils;
 
@@ -83,25 +78,6 @@ public void slowLogSetting() {
     );
   }
 
-  @Section(7)
-  public void cursorEnabledSetting() {
-    docSetting(
-        CURSOR_ENABLED,
-        "User can enable/disable pagination for all queries that are supported.",
-        true
-    );
-  }
-
-  @Section(8)
-  public void cursorDefaultFetchSizeSetting() {
-    docSetting(
-        CURSOR_FETCH_SIZE,
-        "User can set the default fetch_size for all queries that are supported by pagination. " +
-            "Explicit `fetch_size` passed in request will override this value",
-        50
-    );
-  }
-
   @Section(9)
   public void cursorDefaultContextKeepAliveSetting() {
     docSetting(

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java
@@ -41,6 +41,7 @@
 import java.util.List;
 import org.json.JSONArray;
 import org.json.JSONObject;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
@@ -58,7 +59,6 @@ public class CursorIT extends SQLIntegTestCase {
   @Override
   protected void init() throws Exception {
     loadIndex(Index.ACCOUNT);
-    enableCursorClusterSetting();
   }
 
   /**
@@ -132,7 +132,7 @@ public void noPaginationWhenFetchSizeZero() throws IOException {
     String selectQuery = StringUtils.format("SELECT firstname, state FROM %s", TEST_INDEX_ACCOUNT);
     JSONObject response = new JSONObject(executeFetchQuery(selectQuery, 0, JDBC));
     assertFalse(response.has(CURSOR));
-    assertThat(response.getJSONArray(DATAROWS).length(), equalTo(200));
+    assertThat(response.getJSONArray(DATAROWS).length(), equalTo(1000)); // Default limit is 1000 in new engine
   }
 
   /**
@@ -282,6 +282,7 @@ public void testRegressionOnDateFormatChange() throws IOException {
   }
 
 
+  @Ignore("Breaking change for OpenSearch: deprecate and enable cursor always")
   @Test
   public void defaultBehaviorWhenCursorSettingIsDisabled() throws IOException {
     updateClusterSettings(new ClusterSetting(PERSISTENT, "opensearch.sql.cursor.enabled", "false"));
@@ -300,32 +301,22 @@ public void defaultBehaviorWhenCursorSettingIsDisabled() throws IOException {
 
   @Test
   public void testCursorSettings() throws IOException {
-    // reverting enableCursorClusterSetting() in init() method before checking defaults
-    updateClusterSettings(new ClusterSetting(PERSISTENT, "opensearch.sql.cursor.enabled", null));
-
     // Assert default cursor settings
     JSONObject clusterSettings = getAllClusterSettings();
-    assertThat(clusterSettings.query("/defaults/opensearch.sql.cursor.enabled"), equalTo("false"));
-    assertThat(clusterSettings.query("/defaults/opensearch.sql.cursor.fetch_size"),
-        equalTo("1000"));
     assertThat(clusterSettings.query("/defaults/opensearch.sql.cursor.keep_alive"), equalTo("1m"));
 
-    updateClusterSettings(new ClusterSetting(PERSISTENT, "opensearch.sql.cursor.enabled", "true"));
-    updateClusterSettings(new ClusterSetting(TRANSIENT, "opensearch.sql.cursor.fetch_size", "400"));
     updateClusterSettings(
         new ClusterSetting(PERSISTENT, "opensearch.sql.cursor.keep_alive", "200s"));
 
     clusterSettings = getAllClusterSettings();
-    assertThat(clusterSettings.query("/persistent/opensearch.sql.cursor.enabled"), equalTo("true"));
-    assertThat(clusterSettings.query("/transient/opensearch.sql.cursor.fetch_size"),
-        equalTo("400"));
     assertThat(clusterSettings.query("/persistent/opensearch.sql.cursor.keep_alive"),
         equalTo("200s"));
 
     wipeAllClusterSettings();
   }
 
 
+  @Ignore("Breaking change for OpenSearch: no pagination if fetch_size field absent in request")
   @Test
   public void testDefaultFetchSizeFromClusterSettings() throws IOException {
     // the default fetch size is 1000
@@ -481,11 +472,6 @@ public void verifyDataRows(JSONArray dataRowsOne, JSONArray dataRowsTwo) {
     assertTrue(dataRowsOne.similar(dataRowsTwo));
   }
 
-  private void enableCursorClusterSetting() throws IOException {
-    updateClusterSettings(
-        new ClusterSetting("persistent", "opensearch.sql.cursor.enabled", "true"));
-  }
-
   public String executeFetchAsStringQuery(String query, String fetchSize, String requestType)
       throws IOException {
     String endpoint = QUERY_API_ENDPOINT + "?format=" + requestType;

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java
@@ -60,23 +60,15 @@ public void explain() throws IOException {
 
   @Test
   public void closeCursor() throws IOException {
-    updateClusterSettings(
-        new ClusterSetting("transient", "opensearch.sql.cursor.enabled", "true"));
+    String sql = StringUtils.format(
+        "SELECT firstname FROM %s WHERE balance > 100", TEST_INDEX_ACCOUNT);
+    JSONObject result = new JSONObject(executeFetchQuery(sql, 50, "jdbc"));
 
-    try {
-      String sql = StringUtils.format(
-          "SELECT firstname FROM %s WHERE balance > 100", TEST_INDEX_ACCOUNT);
-      JSONObject result = new JSONObject(executeFetchQuery(sql, 50, "jdbc"));
-
-      Request request = new Request("POST", LEGACY_CURSOR_CLOSE_ENDPOINT);
-      request.setJsonEntity(makeCursorRequest(result.getString("cursor")));
-      request.setOptions(buildJsonOption());
-      JSONObject response = new JSONObject(executeRequest(request));
-      assertThat(response.getBoolean("succeeded"), equalTo(true));
-    } finally {
-      updateClusterSettings(
-          new ClusterSetting("transient", "opensearch.sql.cursor.enabled", "false"));
-    }
+    Request request = new Request("POST", LEGACY_CURSOR_CLOSE_ENDPOINT);
+    request.setJsonEntity(makeCursorRequest(result.getString("cursor")));
+    request.setOptions(buildJsonOption());
+    JSONObject response = new JSONObject(executeRequest(request));
+    assertThat(response.getBoolean("succeeded"), equalTo(true));
   }
 
   @Test
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,7 +97,7 @@ Cursor @@
     Description
     -----------
-    To get paginated response for a query, user needs to provide `fetch_size` parameter as part of normal query. The value of `fetch_size` should be greater than `0`. In absence of `fetch_size`, default value of 1000 is used. A value of `0` will fallback to non-paginated response. This feature is only available over `jdbc` format for now.
+    To get paginated response for a query, user needs to provide `fetch_size` parameter as part of normal query. The value of `fetch_size` should be greater than `0`. In absence of `fetch_size` or a value of `0`, it will fallback to non-paginated response. This feature is only available over `jdbc` format for now.
     Example
     -------
@@ Expand Down @@