Merge remote-tracking branch 'upstream/main' into dev-datetime-now

Signed-off-by: Yury Fridlyand <[email protected]>
Bit-Quill · Aug 12, 2022 · 8272c93 · 8272c93
2 parents 2170b42 + deececb
commit 8272c93
Show file tree

Hide file tree

Showing 34 changed files with 917 additions and 197 deletions.
diff --git a/build.gradle b/build.gradle
@@ -7,6 +7,8 @@
 buildscript {
     ext {
         opensearch_version = System.getProperty("opensearch.version", "2.2.0-SNAPSHOT")
+        spring_version = "5.3.22"
+        jackson_version = "2.13.3"
         isSnapshot = "true" == System.getProperty("build.snapshot", "true")
         buildVersionQualifier = System.getProperty("build.version_qualifier", "")
         version_tokens = opensearch_version.tokenize('-')

diff --git a/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java b/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java
@@ -24,8 +24,6 @@ public class QueryContext {
    */
   private static final String REQUEST_ID_KEY = "request_id";
 
-  private static final String EMPTY_ID = "ID";
-
   /**
    * Timestamp when SQL plugin started to process current request.
    */
@@ -40,16 +38,22 @@ public class QueryContext {
    * call this method twice on the same thread within the lifetime of the request.
    * </p>
    */
-  public static void addRequestId() {
-    ThreadContext.put(REQUEST_ID_KEY, UUID.randomUUID().toString());
+  public static String addRequestId() {
+    var id = UUID.randomUUID().toString();
+    ThreadContext.put(REQUEST_ID_KEY, id);
+    return id;
   }
 
   /**
    * Get RequestID.
    * @return the current request id from {@link ThreadContext}.
    */
   public static String getRequestId() {
-    return Optional.ofNullable(ThreadContext.get(REQUEST_ID_KEY)).orElse(EMPTY_ID);
+    var id = ThreadContext.get(REQUEST_ID_KEY);
+    if (null == id) {
+      id = addRequestId();
+    }
+    return id;
   }
 
   public static void recordProcessingStarted() {

diff --git a/core/build.gradle b/core/build.gradle
@@ -40,16 +40,16 @@ repositories {
 
 dependencies {
     api group: 'com.google.guava', name: 'guava', version: '31.0.1-jre'
-    api group: 'org.springframework', name: 'spring-context', version: '5.3.22'
-    api group: 'org.springframework', name: 'spring-beans', version: '5.3.22'
+    api group: 'org.springframework', name: 'spring-context', version: "${spring_version}"
+    api group: 'org.springframework', name: 'spring-beans', version: "${spring_version}"
     api group: 'org.apache.commons', name: 'commons-lang3', version: '3.10'
     api group: 'com.facebook.presto', name: 'presto-matching', version: '0.240'
     api group: 'org.apache.commons', name: 'commons-math3', version: '3.6.1'
     api project(':common')
 
     testImplementation('org.junit.jupiter:junit-jupiter:5.6.2')
     testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1'
-    testImplementation group: 'org.springframework', name: 'spring-test', version: '5.3.22'
+    testImplementation group: 'org.springframework', name: 'spring-test', version: "${spring_version}"
     testImplementation group: 'org.mockito', name: 'mockito-core', version: '3.12.4'
     testImplementation group: 'org.mockito', name: 'mockito-junit-jupiter', version: '3.12.4'
 }

diff --git a/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java b/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java
@@ -18,7 +18,6 @@
 import org.opensearch.sql.expression.Expression;
 import org.opensearch.sql.expression.LiteralExpression;
 import org.opensearch.sql.expression.NamedExpression;
-import org.opensearch.sql.expression.ParseExpression;
 import org.opensearch.sql.expression.ReferenceExpression;
 import org.opensearch.sql.expression.aggregation.NamedAggregator;
 import org.opensearch.sql.expression.window.WindowDefinition;

diff --git a/integ-test/build.gradle b/integ-test/build.gradle
@@ -53,9 +53,9 @@ configurations.all {
     // enforce 1.1.3, https://www.whitesourcesoftware.com/vulnerability-database/WS-2019-0379
     resolutionStrategy.force 'commons-codec:commons-codec:1.13'
     resolutionStrategy.force 'com.google.guava:guava:31.0.1-jre'
-    resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-core:2.13.3'
-    resolutionStrategy.force 'com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.13.3'
-    resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-databind:2.13.3'
+    resolutionStrategy.force "com.fasterxml.jackson.core:jackson-core:${jackson_version}"
+    resolutionStrategy.force "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jackson_version}"
+    resolutionStrategy.force "com.fasterxml.jackson.core:jackson-databind:${jackson_version}"
 }
 
 dependencies {

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
@@ -31,6 +31,7 @@
 import java.nio.file.Paths;
 import java.util.Locale;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient;
 import static org.opensearch.sql.legacy.TestUtils.getAccountIndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getBankIndexMapping;
@@ -71,6 +72,8 @@ public abstract class SQLIntegTestCase extends OpenSearchSQLRestTestCase {
   public static final String TRANSIENT = "transient";
   public static final Integer DEFAULT_QUERY_SIZE_LIMIT =
       Integer.parseInt(System.getProperty("defaultQuerySizeLimit", "200"));
+  public static final Integer DEFAULT_MAX_RESULT_WINDOW =
+      Integer.parseInt(System.getProperty("defaultMaxResultWindow", "10000"));
 
   public boolean shouldResetQuerySizeLimit() {
     return true;
@@ -161,6 +164,15 @@ protected static void wipeAllClusterSettings() throws IOException {
     updateClusterSettings(new ClusterSetting("transient", "*", null));
   }
 
+  protected void setMaxResultWindow(String indexName, Integer window) throws IOException {
+    updateIndexSettings(indexName, "{ \"index\": { \"max_result_window\":" + window + " } }");
+  }
+
+  protected void resetMaxResultWindow(String indexName) throws IOException {
+    updateIndexSettings(indexName,
+        "{ \"index\": { \"max_result_window\": " + DEFAULT_MAX_RESULT_WINDOW + " } }");
+  }
+
   /**
    * Provide for each test to load test index, data and other setup work
    */
@@ -378,6 +390,18 @@ public String toString() {
     }
   }
 
+  protected static JSONObject updateIndexSettings(String indexName, String setting)
+      throws IOException {
+    Request request = new Request("PUT", "/" + indexName + "/_settings");
+    if (!isNullOrEmpty(setting)) {
+      request.setJsonEntity(setting);
+    }
+    RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
+    restOptionsBuilder.addHeader("Content-Type", "application/json");
+    request.setOptions(restOptionsBuilder);
+    return new JSONObject(executeRequest(request));
+  }
+
   protected String makeRequest(String query) {
     return makeRequest(query, 0);
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java
@@ -14,6 +14,7 @@
 import org.json.JSONObject;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.jupiter.api.Test;
 
 public class HeadCommandIT extends PPLIntegTestCase {
@@ -26,6 +27,7 @@ public void beforeTest() throws IOException {
   @After
   public void afterTest() throws IOException {
     resetQuerySizeLimit();
+    resetMaxResultWindow(TEST_INDEX_ACCOUNT);
   }
 
   @Override
@@ -60,6 +62,76 @@ public void testHeadWithNumber() throws IOException {
         rows("Nanette", 28));
   }
 
+  @Ignore("Fix https://github.com/opensearch-project/sql/issues/703#issuecomment-1211422130")
+  @Test
+  public void testHeadWithNumberLargerThanQuerySizeLimit() throws IOException {
+    setQuerySizeLimit(5);
+    JSONObject result =
+        executeQuery(String.format(
+            "source=%s | fields firstname, age | head 10", TEST_INDEX_ACCOUNT));
+    verifyDataRows(result,
+        rows("Amber", 32),
+        rows("Hattie", 36),
+        rows("Nanette", 28),
+        rows("Dale", 33),
+        rows("Elinor", 36),
+        rows("Virginia", 39),
+        rows("Dillard", 34),
+        rows("Mcgee", 39),
+        rows("Aurelia", 37),
+        rows("Fulton", 23));
+  }
+
+  @Test
+  public void testHeadWithNumberLargerThanMaxResultWindow() throws IOException {
+    setMaxResultWindow(TEST_INDEX_ACCOUNT, 10);
+    JSONObject result =
+        executeQuery(String.format(
+            "source=%s | fields firstname, age | head 15", TEST_INDEX_ACCOUNT));
+    verifyDataRows(result,
+        rows("Amber", 32),
+        rows("Hattie", 36),
+        rows("Nanette", 28),
+        rows("Dale", 33),
+        rows("Elinor", 36),
+        rows("Virginia", 39),
+        rows("Dillard", 34),
+        rows("Mcgee", 39),
+        rows("Aurelia", 37),
+        rows("Fulton", 23),
+        rows("Burton", 31),
+        rows("Josie", 32),
+        rows("Hughes", 30),
+        rows("Hall", 25),
+        rows("Deidre", 33));
+  }
+
+  @Ignore("Fix https://github.com/opensearch-project/sql/issues/703#issuecomment-1211422130")
+  @Test
+  public void testHeadWithLargeNumber() throws IOException {
+    setQuerySizeLimit(5);
+    setMaxResultWindow(TEST_INDEX_ACCOUNT, 10);
+    JSONObject result =
+        executeQuery(String.format(
+            "source=%s | fields firstname, age | head 15", TEST_INDEX_ACCOUNT));
+    verifyDataRows(result,
+        rows("Amber", 32),
+        rows("Hattie", 36),
+        rows("Nanette", 28),
+        rows("Dale", 33),
+        rows("Elinor", 36),
+        rows("Virginia", 39),
+        rows("Dillard", 34),
+        rows("Mcgee", 39),
+        rows("Aurelia", 37),
+        rows("Fulton", 23),
+        rows("Burton", 31),
+        rows("Josie", 32),
+        rows("Hughes", 30),
+        rows("Hall", 25),
+        rows("Deidre", 33));
+  }
+
   @Test
   public void testHeadWithNumberAndFrom() throws IOException {
     JSONObject result =

diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java
@@ -8,7 +8,7 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 
 import org.apache.logging.log4j.ThreadContext;
 import org.junit.After;
@@ -47,7 +47,7 @@ public void addRequestId_alreadyExists() {
 
     @Test
     public void getRequestId_doesNotExist() {
-        assertEquals("ID", QueryContext.getRequestId());
+        assertNotNull(QueryContext.getRequestId());
     }
 
     @Test

diff --git a/opensearch/build.gradle b/opensearch/build.gradle
@@ -32,9 +32,9 @@ dependencies {
     api project(':core')
     api group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}"
     implementation "io.github.resilience4j:resilience4j-retry:1.5.0"
-    implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: '2.13.3'
-    implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.13.3'
-    implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: '2.13.3'
+    implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: "${jackson_version}"
+    implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${jackson_version}"
+    implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: "${jackson_version}"
     implementation group: 'org.json', name: 'json', version:'20180813'
     compileOnly group: 'org.opensearch.client', name: 'opensearch-rest-high-level-client', version: "${opensearch_version}"
     implementation group: 'org.opensearch', name:'opensearch-ml-client', version: "${opensearch_build}"

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java
@@ -30,6 +30,14 @@ public interface OpenSearchClient {
    */
   Map<String, IndexMapping> getIndexMappings(String... indexExpression);
 
+  /**
+   * Fetch index.max_result_window settings according to index expression given.
+   *
+   * @param indexExpression index expression
+   * @return map from index name to its max result window
+   */
+  Map<String, Integer> getIndexMaxResultWindows(String... indexExpression);
+
   /**
    * Perform search query in the search request.
    *

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java
@@ -24,11 +24,14 @@
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.AliasMetadata;
+import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.metadata.MappingMetadata;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.collect.ImmutableOpenMap;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
+import org.opensearch.index.IndexSettings;
 import org.opensearch.sql.opensearch.mapping.IndexMapping;
 import org.opensearch.sql.opensearch.request.OpenSearchRequest;
 import org.opensearch.sql.opensearch.response.OpenSearchResponse;
@@ -86,6 +89,29 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
     }
   }
 
+  /**
+   * Fetch index.max_result_window settings according to index expression given.
+   *
+   * @param indexExpression index expression
+   * @return map from index name to its max result window
+   */
+  @Override
+  public Map<String, Integer> getIndexMaxResultWindows(String... indexExpression) {
+    ClusterState state = clusterService.state();
+    ImmutableOpenMap<String, IndexMetadata> indicesMetadata = state.metadata().getIndices();
+    String[] concreteIndices = resolveIndexExpression(state, indexExpression);
+
+    ImmutableMap.Builder<String, Integer> result = ImmutableMap.builder();
+    for (String index : concreteIndices) {
+      Settings settings = indicesMetadata.get(index).getSettings();
+      Integer maxResultWindow = settings.getAsInt("index.max_result_window",
+          IndexSettings.MAX_RESULT_WINDOW_SETTING.getDefault(settings));
+      result.put(index, maxResultWindow);
+    }
+
+    return result.build();
+  }
+
   /**
    * TODO: Scroll doesn't work for aggregation. Support aggregation later.
    */

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java
@@ -11,12 +11,15 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import lombok.RequiredArgsConstructor;
 import org.opensearch.action.admin.cluster.settings.ClusterGetSettingsRequest;
+import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest;
+import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.opensearch.action.search.ClearScrollRequest;
 import org.opensearch.client.RequestOptions;
 import org.opensearch.client.RestHighLevelClient;
@@ -26,6 +29,7 @@
 import org.opensearch.client.indices.GetMappingsResponse;
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.metadata.AliasMetadata;
+import org.opensearch.common.collect.ImmutableOpenMap;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.sql.opensearch.mapping.IndexMapping;
 import org.opensearch.sql.opensearch.request.OpenSearchRequest;
@@ -54,6 +58,36 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
     }
   }
 
+  @Override
+  public Map<String, Integer> getIndexMaxResultWindows(String... indexExpression) {
+    GetSettingsRequest request = new GetSettingsRequest()
+        .indices(indexExpression).includeDefaults(true);
+    try {
+      GetSettingsResponse response = client.indices().getSettings(request, RequestOptions.DEFAULT);
+      ImmutableOpenMap<String, Settings> settings = response.getIndexToSettings();
+      ImmutableOpenMap<String, Settings> defaultSettings = response.getIndexToDefaultSettings();
+      Map<String, Integer> result = new HashMap<>();
+
+      defaultSettings.forEach(entry -> {
+        Integer maxResultWindow = entry.value.getAsInt("index.max_result_window", null);
+        if (maxResultWindow != null) {
+          result.put(entry.key, maxResultWindow);
+        }
+      });
+
+      settings.forEach(entry -> {
+        Integer maxResultWindow = entry.value.getAsInt("index.max_result_window", null);
+        if (maxResultWindow != null) {
+          result.put(entry.key, maxResultWindow);
+        }
+      });
+
+      return result;
+    } catch (IOException e) {
+      throw new IllegalStateException("Failed to get max result window for " + indexExpression, e);
+    }
+  }
+
   @Override
   public OpenSearchResponse search(OpenSearchRequest request) {
     return request.search(