Deprecated legacy settings instead of removing (#140)

* Deprecated cursor enabling and fetch size setting

Signed-off-by: penghuo <penghuo@gmail.com>

* update doc

Signed-off-by: penghuo <penghuo@gmail.com>

* add more deprecated settings

Signed-off-by: penghuo <penghuo@gmail.com>

common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java

@@ -56,7 +56,18 @@ public enum Key {
     /**
      * Legacy Common Settings.
      */
-    QUERY_SIZE_LIMIT("opendistro.query.size_limit");
+    QUERY_SIZE_LIMIT("opendistro.query.size_limit"),
+
+    /**
+     * Deprecated Settings.
+     */
+    SQL_NEW_ENGINE_ENABLED("opendistro.sql.engine.new.enabled"),
+    QUERY_ANALYSIS_ENABLED("opendistro.sql.query.analysis.enabled"),
+    QUERY_ANALYSIS_SEMANTIC_SUGGESTION("opendistro.sql.query.analysis.semantic.suggestion"),
+    QUERY_ANALYSIS_SEMANTIC_THRESHOLD("opendistro.sql.query.analysis.semantic.threshold"),
+    QUERY_RESPONSE_FORMAT("opendistro.sql.query.response.format"),
+    SQL_CURSOR_ENABLED("opendistro.sql.cursor.enabled"),
+    SQL_CURSOR_FETCH_SIZE("opendistro.sql.cursor.fetch_size");
 
     @Getter
     private final String keyValue;

docs/user/admin/settings.rst

@@ -16,6 +16,35 @@ Introduction
 
 When OpenSearch bootstraps, SQL plugin will register a few settings in OpenSearch cluster settings. Most of the settings are able to change dynamically so you can control the behavior of SQL plugin without need to bounce your cluster. You can update the settings by sending requests to either ``_cluster/settings`` or ``_plugins/_query/settings`` endpoint, though the examples are sending to the latter.
 
+Breaking Change
+===============
+opendistro.sql.engine.new.enabled
+---------------------------------
+The opendistro.sql.engine.new.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the new engine is always enabled.
+
+opendistro.sql.query.analysis.enabled
+-------------------------------------
+The opendistro.sql.query.analysis.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis in legacy engine is disabled.
+
+opendistro.sql.query.analysis.semantic.suggestion
+-------------------------------------------------
+The opendistro.sql.query.analysis.semantic.suggestion setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis suggestion in legacy engine is disabled.
+
+opendistro.sql.query.analysis.semantic.threshold
+------------------------------------------------
+The opendistro.sql.query.analysis.semantic.threshold setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis threshold in legacy engine is disabled.
+
+opendistro.sql.query.response.format
+------------------------------------
+The opendistro.sql.query.response.format setting is deprecated and will be removed then. From OpenSearch 1.0, the query response format is default to JDBC format. `You can change the format by using query parameters<../interfaces/protocol.rst>`_.
+
+opendistro.sql.cursor.enabled
+-----------------------------
+The opendistro.sql.cursor.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the cursor feature is enabled by default.
+
+opendistro.sql.cursor.fetch_size
+--------------------------------
+The opendistro.sql.cursor.fetch_size setting is deprecated and will be removed then. From OpenSearch 1.0, the fetch_size in query body will decide whether create the cursor context. No cursor will be created if the fetch_size = 0.
 
 plugins.sql.enabled
 ======================

opensearch/src/main/java/org/opensearch/sql/opensearch/setting/LegacyOpenDistroSettings.java

@@ -13,6 +13,8 @@
 
 import static org.opensearch.common.unit.TimeValue.timeValueMinutes;
 
+import com.google.common.collect.ImmutableList;
+import java.util.List;
 import lombok.experimental.UtilityClass;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.unit.ByteSizeValue;
@@ -81,4 +83,108 @@ public class LegacyOpenDistroSettings {
       Setting.Property.Dynamic,
       Setting.Property.Deprecated);
 
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the new engine is always enabled.
+   */
+  public static final Setting<Boolean> SQL_NEW_ENGINE_ENABLED_SETTING = Setting.boolSetting(
+      LegacySettings.Key.SQL_NEW_ENGINE_ENABLED.getKeyValue(),
+      true,
+      Setting.Property.NodeScope,
+      Setting.Property.Dynamic,
+      Setting.Property.Deprecated);
+
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the query analysis in legacy engine is disabled.
+   */
+  public static final Setting<Boolean> QUERY_ANALYSIS_ENABLED_SETTING = Setting.boolSetting(
+      LegacySettings.Key.QUERY_ANALYSIS_ENABLED.getKeyValue(),
+      false,
+      Setting.Property.NodeScope,
+      Setting.Property.Dynamic,
+      Setting.Property.Deprecated);
+
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the query analysis suggestion in legacy engine is disabled.
+   */
+  public static final Setting<Boolean> QUERY_ANALYSIS_SEMANTIC_SUGGESTION_SETTING =
+      Setting.boolSetting(
+      LegacySettings.Key.QUERY_ANALYSIS_SEMANTIC_SUGGESTION.getKeyValue(),
+      false,
+      Setting.Property.NodeScope,
+      Setting.Property.Dynamic,
+      Setting.Property.Deprecated);
+
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the query analysis threshold in legacy engine is disabled.
+   */
+  public static final Setting<Integer> QUERY_ANALYSIS_SEMANTIC_THRESHOLD_SETTING =
+      Setting.intSetting(
+          LegacySettings.Key.QUERY_ANALYSIS_SEMANTIC_THRESHOLD.getKeyValue(),
+          200,
+          Setting.Property.NodeScope,
+          Setting.Property.Dynamic,
+          Setting.Property.Deprecated);
+
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the query response format is default to JDBC format.
+   */
+  public static final Setting<String> QUERY_RESPONSE_FORMAT_SETTING =
+      Setting.simpleString(
+          LegacySettings.Key.QUERY_RESPONSE_FORMAT.getKeyValue(),
+          "jdbc",
+          Setting.Property.NodeScope,
+          Setting.Property.Dynamic,
+          Setting.Property.Deprecated);
+
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the cursor feature is enabled by default.
+   */
+  public static final Setting<Boolean> SQL_CURSOR_ENABLED_SETTING =
+      Setting.boolSetting(
+          LegacySettings.Key.SQL_CURSOR_ENABLED.getKeyValue(),
+          true,
+          Setting.Property.NodeScope,
+          Setting.Property.Dynamic,
+          Setting.Property.Deprecated);
+  /**
+   * Deprecated and will be removed then.
+   * From OpenSearch 1.0, the fetch_size in query body will decide whether create the cursor
+   * context. No cursor will be created if the fetch_size = 0.
+   */
+  public static final Setting<Integer> SQL_CURSOR_FETCH_SIZE_SETTING =
+      Setting.intSetting(
+          LegacySettings.Key.SQL_CURSOR_FETCH_SIZE.getKeyValue(),
+          1000,
+          Setting.Property.NodeScope,
+          Setting.Property.Dynamic,
+          Setting.Property.Deprecated);
+
+  /**
+   * Used by Plugin to init Setting.
+   */
+  public static List<Setting<?>> legacySettings() {
+    return new ImmutableList.Builder<Setting<?>>()
+        .add(SQL_ENABLED_SETTING)
+        .add(SQL_QUERY_SLOWLOG_SETTING)
+        .add(SQL_CURSOR_KEEPALIVE_SETTING)
+        .add(METRICS_ROLLING_WINDOW_SETTING)
+        .add(METRICS_ROLLING_INTERVAL_SETTING)
+        .add(PPL_ENABLED_SETTING)
+        .add(PPL_QUERY_MEMORY_LIMIT_SETTING)
+        .add(QUERY_SIZE_LIMIT_SETTING)
+        .add(SQL_NEW_ENGINE_ENABLED_SETTING)
+        .add(QUERY_ANALYSIS_ENABLED_SETTING)
+        .add(QUERY_ANALYSIS_SEMANTIC_SUGGESTION_SETTING)
+        .add(QUERY_ANALYSIS_SEMANTIC_THRESHOLD_SETTING)
+        .add(QUERY_RESPONSE_FORMAT_SETTING)
+        .add(SQL_CURSOR_ENABLED_SETTING)
+        .add(SQL_CURSOR_FETCH_SIZE_SETTING)
+        .build();
+  }
 }

opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java

@@ -33,6 +33,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.opensearch.common.unit.TimeValue.timeValueMinutes;
+import static org.opensearch.sql.opensearch.setting.LegacyOpenDistroSettings.legacySettings;
 
 import java.util.List;
 import org.junit.jupiter.api.Test;
@@ -148,4 +149,10 @@ public void updateLegacySettingsFallback() {
     assertEquals(OpenSearchSettings.METRICS_ROLLING_WINDOW_SETTING.get(settings), 2000L);
     assertEquals(OpenSearchSettings.METRICS_ROLLING_INTERVAL_SETTING.get(settings), 100L);
   }
+
+
+  @Test
+  void legacySettingsShouldBeDeprecatedBeforeRemove() {
+    assertEquals(15, legacySettings().size());
+  }
 }

plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

@@ -152,13 +152,7 @@ public List<ExecutorBuilder<?>> getExecutorBuilders(Settings settings) {
   @Override
   public List<Setting<?>> getSettings() {
     return new ImmutableList.Builder<Setting<?>>()
-        .add(LegacyOpenDistroSettings.SQL_ENABLED_SETTING)
-        .add(LegacyOpenDistroSettings.SQL_QUERY_SLOWLOG_SETTING)
-        .add(LegacyOpenDistroSettings.METRICS_ROLLING_WINDOW_SETTING)
-        .add(LegacyOpenDistroSettings.METRICS_ROLLING_INTERVAL_SETTING)
-        .add(LegacyOpenDistroSettings.PPL_ENABLED_SETTING)
-        .add(LegacyOpenDistroSettings.PPL_QUERY_MEMORY_LIMIT_SETTING)
-        .add(LegacyOpenDistroSettings.QUERY_SIZE_LIMIT_SETTING)
+        .addAll(LegacyOpenDistroSettings.legacySettings())
         .addAll(OpenSearchSettings.pluginSettings())
         .build();
   }