Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/2.x' into backport-2981-to-2x
Browse files Browse the repository at this point in the history
  • Loading branch information
Swiddis committed Oct 23, 2024
2 parents 4d4e5d0 + 2921f2e commit eb54414
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class GrokCompiler implements Serializable {

// We don't want \n and commented line
private static final Pattern patternLinePattern = Pattern.compile("^([A-z0-9_]+)\\s+(.*)$");
private static final Pattern patternLinePattern = Pattern.compile("^([a-zA-Z0-9_]+)\\s+(.*)$");

/** {@code Grok} patterns definitions. */
private final Map<String, String> grokPatternDefinitions = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public class GrokUtils {
Pattern.compile(
"%\\{"
+ "(?<name>"
+ "(?<pattern>[A-z0-9]+)"
+ "(?::(?<subname>[A-z0-9_:;,\\-\\/\\s\\.']+))?"
+ "(?<pattern>[a-zA-Z0-9_]+)"
+ "(?::(?<subname>[a-zA-Z0-9_:;,\\-\\/\\s\\.']+))?"
+ ")"
+ "(?:=(?<definition>"
+ "(?:"
Expand Down
20 changes: 20 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/jdbc/CursorIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import lombok.SneakyThrows;
import org.json.JSONObject;
Expand Down Expand Up @@ -116,6 +117,8 @@ public void select_all_no_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -134,6 +137,8 @@ public void select_count_all_no_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -152,6 +157,8 @@ public void select_all_small_table_big_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -170,6 +177,8 @@ public void select_all_small_table_small_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -188,6 +197,8 @@ public void select_all_big_table_small_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -206,6 +217,8 @@ public void select_all_big_table_big_cursor() {

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
var restPrettyResponse = executeRestQuery(query, null, Map.of("pretty", "true"));
assertEquals(rows, restPrettyResponse.getInt("total"));
}
}

Expand All @@ -218,13 +231,20 @@ private static String getConnectionString() {

@SneakyThrows
protected JSONObject executeRestQuery(String query, @Nullable Integer fetch_size) {
return executeRestQuery(query, fetch_size, Map.of());
}

@SneakyThrows
protected JSONObject executeRestQuery(
String query, @Nullable Integer fetch_size, Map<String, String> params) {
Request request = new Request("POST", QUERY_API_ENDPOINT);
if (fetch_size != null) {
request.setJsonEntity(
String.format("{ \"query\": \"%s\", \"fetch_size\": %d }", query, fetch_size));
} else {
request.setJsonEntity(String.format("{ \"query\": \"%s\" }", query));
}
request.addParameters(params);

RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
restOptionsBuilder.addHeader("Content-Type", "application/json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,12 @@ protected String makeRequest(String query, int fetch_size) {
"{\n" + " \"fetch_size\": \"%s\",\n" + " \"query\": \"%s\"\n" + "}", fetch_size, query);
}

protected String makeRequest(String query, int fetch_size, String filterQuery) {
return String.format(
"{ \"fetch_size\": \"%s\", \"query\": \"%s\", \"filter\" : %s }",
fetch_size, query, filterQuery);
}

protected String makeFetchLessRequest(String query) {
return String.format("{\n" + " \"query\": \"%s\"\n" + "}", query);
}
Expand Down
50 changes: 50 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/PaginationIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.opensearch.sql.legacy.TestUtils.getResponseBody;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_CALCS;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ONLINE;

Expand All @@ -18,6 +19,7 @@
import org.junit.Test;
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.Response;
import org.opensearch.client.ResponseException;
import org.opensearch.sql.common.setting.Settings;
import org.opensearch.sql.legacy.SQLIntegTestCase;
Expand Down Expand Up @@ -215,4 +217,52 @@ public void testQueryWithoutFrom() {
assertEquals(1, response.getInt("total"));
assertEquals(1, response.getJSONArray("datarows").getJSONArray(0).getInt(0));
}

@Test
public void testAlias() throws Exception {
String indexName = Index.ONLINE.getName();
String aliasName = "alias_ONLINE";
String filterQuery = "{\n" + " \"term\": {\n" + " \"107\": 72 \n" + " }\n" + "}";

// Execute the SQL query with filter
String selectQuery = "SELECT * FROM " + TEST_INDEX_ONLINE;
JSONObject initialResponse =
new JSONObject(executeFetchQuery(selectQuery, 10, "jdbc", filterQuery));
assertEquals(initialResponse.getInt("size"), 10);

// Create an alias
String createAliasQuery =
String.format(
"{ \"actions\": [ { \"add\": { \"index\": \"%s\", \"alias\": \"%s\" } } ] }",
indexName, aliasName);
Request createAliasRequest = new Request("POST", "/_aliases");
createAliasRequest.setJsonEntity(createAliasQuery);
JSONObject aliasResponse = new JSONObject(executeRequest(createAliasRequest));

// Assert that alias creation was acknowledged
assertTrue(aliasResponse.getBoolean("acknowledged"));

// Query using the alias
String aliasSelectQuery = String.format("SELECT * FROM %s", aliasName);
JSONObject aliasQueryResponse = new JSONObject(executeFetchQuery(aliasSelectQuery, 4, "jdbc"));
assertEquals(4, aliasQueryResponse.getInt("size"));

// Query using the alias with filter
JSONObject aliasFilteredResponse =
new JSONObject(executeFetchQuery(aliasSelectQuery, 4, "jdbc", filterQuery));
assertEquals(aliasFilteredResponse.getInt("size"), 4);
}

private String executeFetchQuery(String query, int fetchSize, String requestType, String filter)
throws IOException {
String endpoint = "/_plugins/_sql?format=" + requestType;
String requestBody = makeRequest(query, fetchSize, filter);

Request sqlRequest = new Request("POST", endpoint);
sqlRequest.setJsonEntity(requestBody);

Response response = client().performRequest(sqlRequest);
String responseString = getResponseBody(response, true);
return responseString;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.opensearch.sql.common.setting.Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER;

import java.util.Map;
import java.util.Objects;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
Expand Down Expand Up @@ -132,13 +133,11 @@ private Protocol buildProtocolForDefaultQuery(Client client, DefaultQueryAction
return protocol;
}

private boolean isDefaultCursor(SearchResponse searchResponse, DefaultQueryAction queryAction) {
protected boolean isDefaultCursor(SearchResponse searchResponse, DefaultQueryAction queryAction) {
if (LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER)) {
if (searchResponse.getHits().getTotalHits().value < queryAction.getSqlRequest().fetchSize()) {
return false;
} else {
return true;
}
return queryAction.getSqlRequest().fetchSize() != 0
&& Objects.requireNonNull(searchResponse.getHits().getTotalHits()).value
>= queryAction.getSqlRequest().fetchSize();
} else {
return !Strings.isNullOrEmpty(searchResponse.getScrollId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.stream.StreamSupport;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.opensearch.action.admin.indices.alias.get.GetAliasesResponse;
import org.opensearch.action.admin.indices.mapping.get.GetFieldMappingsRequest;
import org.opensearch.action.admin.indices.mapping.get.GetFieldMappingsResponse;
import org.opensearch.action.search.ClearScrollResponse;
Expand Down Expand Up @@ -164,7 +166,11 @@ private void populateResultSetFromDefaultCursor(DefaultCursor cursor) {
private void loadFromEsState(Query query) {
String indexName = fetchIndexName(query);
String[] fieldNames = fetchFieldsAsArray(query);

GetAliasesResponse getAliasesResponse =
client.admin().indices().getAliases(new GetAliasesRequest(indexName)).actionGet();
if (getAliasesResponse != null && !getAliasesResponse.getAliases().isEmpty()) {
indexName = getAliasesResponse.getAliases().keySet().iterator().next();
}
// Reset boolean in the case of JOIN query where multiple calls to loadFromEsState() are made
selectAll = isSimpleQuerySelectAll(query) || isJoinQuerySelectAll(query, fieldNames);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public void run() throws IOException, SqlParseException {
this.metaResults.setTookImMilli(joinTimeInMilli);
} catch (Exception e) {
LOG.error("Failed during join query run.", e);
throw new IllegalStateException("Error occurred during join query run", e);
} finally {
if (LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER)) {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package org.opensearch.sql.legacy.executor.format;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.opensearch.sql.common.setting.Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER;

import org.apache.lucene.search.TotalHits;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.search.SearchHit;
import org.opensearch.search.SearchHits;
import org.opensearch.sql.legacy.esdomain.LocalClusterState;
import org.opensearch.sql.legacy.query.DefaultQueryAction;
import org.opensearch.sql.legacy.request.SqlRequest;
import org.opensearch.sql.opensearch.setting.OpenSearchSettings;

@RunWith(MockitoJUnitRunner.class)
public class PrettyFormatRestExecutorTest {

@Mock private SearchResponse searchResponse;
@Mock private SearchHits searchHits;
@Mock private SearchHit searchHit;
@Mock private DefaultQueryAction queryAction;
@Mock private SqlRequest sqlRequest;
private PrettyFormatRestExecutor executor;

@Before
public void setUp() {
OpenSearchSettings settings = mock(OpenSearchSettings.class);
LocalClusterState.state().setPluginSettings(settings);
when(LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER))
.thenReturn(true);
when(queryAction.getSqlRequest()).thenReturn(sqlRequest);
executor = new PrettyFormatRestExecutor("jdbc");
}

@Test
public void testIsDefaultCursor_fetchSizeZero() {
when(sqlRequest.fetchSize()).thenReturn(0);

assertFalse(executor.isDefaultCursor(searchResponse, queryAction));
}

@Test
public void testIsDefaultCursor_totalHitsLessThanFetchSize() {
when(sqlRequest.fetchSize()).thenReturn(10);
when(searchResponse.getHits())
.thenReturn(
new SearchHits(
new SearchHit[] {searchHit}, new TotalHits(5, TotalHits.Relation.EQUAL_TO), 1.0F));

assertFalse(executor.isDefaultCursor(searchResponse, queryAction));
}

@Test
public void testIsDefaultCursor_totalHitsGreaterThanOrEqualToFetchSize() {
when(sqlRequest.fetchSize()).thenReturn(5);
when(searchResponse.getHits())
.thenReturn(
new SearchHits(
new SearchHit[] {searchHit}, new TotalHits(5, TotalHits.Relation.EQUAL_TO), 1.0F));

assertTrue(executor.isDefaultCursor(searchResponse, queryAction));
}

@Test
public void testIsDefaultCursor_PaginationApiDisabled() {
when(LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER))
.thenReturn(false);
when(searchResponse.getScrollId()).thenReturn("someScrollId");

assertTrue(executor.isDefaultCursor(searchResponse, queryAction));
}

@Test
public void testIsDefaultCursor_PaginationApiDisabled_NoScrollId() {
when(LocalClusterState.state().getSettingValue(SQL_PAGINATION_API_SEARCH_AFTER))
.thenReturn(false);
when(searchResponse.getScrollId()).thenReturn(null);

assertFalse(executor.isDefaultCursor(searchResponse, queryAction));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Stream;
import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand Down Expand Up @@ -81,19 +82,21 @@ public SQLQueryRequest(
* @return true if supported.
*/
public boolean isSupported() {
var noCursor = !isCursor();
var noQuery = query == null;
var noUnsupportedParams =
params.isEmpty() || (params.size() == 1 && params.containsKey(QUERY_PARAMS_FORMAT));
var noContent = jsonContent == null || jsonContent.isEmpty();

return ((!noCursor
&& noQuery
&& noUnsupportedParams
&& noContent) // if cursor is given, but other things
|| (noCursor && !noQuery)) // or if cursor is not given, but query
&& isOnlySupportedFieldInPayload() // and request has supported fields only
&& isSupportedFormat(); // and request is in supported format
boolean hasCursor = isCursor();
boolean hasQuery = query != null;
boolean hasContent = jsonContent != null && !jsonContent.isEmpty();

Predicate<String> supportedParams = Set.of(QUERY_PARAMS_FORMAT, QUERY_PARAMS_PRETTY)::contains;
boolean hasUnsupportedParams =
(!params.isEmpty())
&& params.keySet().stream().dropWhile(supportedParams).findAny().isPresent();

boolean validCursor = hasCursor && !hasQuery && !hasUnsupportedParams && !hasContent;
boolean validQuery = !hasCursor && hasQuery;

return (validCursor || validQuery) // It's a valid cursor or a valid query
&& isOnlySupportedFieldInPayload() // and request must contain supported fields only
&& isSupportedFormat(); // and request must be a supported format
}

private boolean isCursor() {
Expand Down
Loading

0 comments on commit eb54414

Please sign in to comment.