From 5f3c79d64b8f4580149b4632032561081a4a4567 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Sat, 17 Oct 2020 01:36:26 +0300 Subject: [PATCH] Remove filter from QL's field_caps requests (#63840) (#63845) (cherry picked from commit f009e6341d0fc0471f212d5a41c91e7aab77e006) --- .../xpack/eql/session/EqlSession.java | 2 +- .../xpack/ql/index/IndexResolver.java | 20 +++-- .../xpack/sql/qa/rest/RestSqlTestCase.java | 81 +------------------ .../sql/plan/logical/command/ShowColumns.java | 2 +- .../plan/logical/command/sys/SysColumns.java | 4 +- .../xpack/sql/session/SqlSession.java | 2 +- .../logical/command/sys/SysColumnsTests.java | 8 +- 7 files changed, 19 insertions(+), 100 deletions(-) diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java index 225ea1c144d..4ba9fdff687 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java @@ -100,7 +100,7 @@ public class EqlSession { listener.onFailure(new TaskCancelledException("cancelled")); return; } - indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.indicesOptions(), configuration.filter(), + indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.indicesOptions(), map(listener, r -> preAnalyzer.preAnalyze(parsed, r)) ); } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java index ac69f929bfb..e86ac79ee3d 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java @@ -25,7 +25,6 @@ import org.elasticsearch.cluster.metadata.AliasMetadata; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.index.IndexNotFoundException; -import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; import org.elasticsearch.xpack.ql.type.ConstantKeywordEsField; import org.elasticsearch.xpack.ql.type.DataType; @@ -280,9 +279,9 @@ public class IndexResolver { /** * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping. */ - public void resolveAsMergedMapping(String indexWildcard, String javaRegex, IndicesOptions indicesOptions, QueryBuilder filter, + public void resolveAsMergedMapping(String indexWildcard, String javaRegex, IndicesOptions indicesOptions, ActionListener listener) { - FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, indicesOptions, filter); + FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, indicesOptions); client.fieldCaps(fieldRequest, ActionListener.wrap( response -> listener.onResponse(mergedMappings(typeRegistry, indexWildcard, response.getIndices(), response.get())), @@ -292,9 +291,9 @@ public class IndexResolver { /** * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping. */ - public void resolveAsMergedMapping(String indexWildcard, String javaRegex, boolean includeFrozen, QueryBuilder filter, + public void resolveAsMergedMapping(String indexWildcard, String javaRegex, boolean includeFrozen, ActionListener listener) { - FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, includeFrozen, filter); + FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, includeFrozen); client.fieldCaps(fieldRequest, ActionListener.wrap( response -> listener.onResponse(mergedMappings(typeRegistry, indexWildcard, response.getIndices(), response.get())), @@ -470,28 +469,27 @@ public class IndexResolver { return new EsField(fieldName, esType, props, isAggregateable, isAlias); } - private static FieldCapabilitiesRequest createFieldCapsRequest(String index, IndicesOptions indicesOptions, QueryBuilder filter) { + private static FieldCapabilitiesRequest createFieldCapsRequest(String index, IndicesOptions indicesOptions) { return new FieldCapabilitiesRequest() .indices(Strings.commaDelimitedListToStringArray(index)) .fields("*") .includeUnmapped(true) - .indexFilter(filter) //lenient because we throw our own errors looking at the response e.g. if something was not resolved //also because this way security doesn't throw authorization exceptions but rather honors ignore_unavailable .indicesOptions(indicesOptions); } - private static FieldCapabilitiesRequest createFieldCapsRequest(String index, boolean includeFrozen, QueryBuilder filter) { + private static FieldCapabilitiesRequest createFieldCapsRequest(String index, boolean includeFrozen) { IndicesOptions indicesOptions = includeFrozen ? FIELD_CAPS_FROZEN_INDICES_OPTIONS : FIELD_CAPS_INDICES_OPTIONS; - return createFieldCapsRequest(index, indicesOptions, filter); + return createFieldCapsRequest(index, indicesOptions); } /** * Resolves a pattern to multiple, separate indices. Doesn't perform validation. */ - public void resolveAsSeparateMappings(String indexWildcard, String javaRegex, boolean includeFrozen, QueryBuilder filter, + public void resolveAsSeparateMappings(String indexWildcard, String javaRegex, boolean includeFrozen, ActionListener> listener) { - FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, includeFrozen, filter); + FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, includeFrozen); client.fieldCaps(fieldRequest, wrap(response -> { client.admin().indices().getAliases(createGetAliasesRequest(response, includeFrozen), wrap(aliases -> listener.onResponse(separateMappings(typeRegistry, javaRegex, response.getIndices(), response.get(), aliases.getAliases())), diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java index 8fef7c14936..4bf509bd42a 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.qa.rest; import com.fasterxml.jackson.core.io.JsonStringEncoder; + import org.apache.http.HttpEntity; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; @@ -697,86 +698,6 @@ public abstract class RestSqlTestCase extends BaseRestSqlTestCase implements Err ); } - /** - * Test for filtering the field_caps response with a filter. - * Because there is no actual SELECT involved (thus, the REST request filter not actually being applied on an actual _search), we can - * test if the filtering is correctly applied at field_caps request level. - */ - public void testSysColumnsCommandWithFilter() throws IOException { - String mode = randomMode(); - // create three indices with same @timestamp date field and with differently named one more field - indexWithIndexName("test2018", "{\"@timestamp\":\"2018-06-01\",\"field2018\":\"foo\"}"); - indexWithIndexName("test2019", "{\"@timestamp\":\"2019-06-01\",\"field2019\":\"foo\"}"); - indexWithIndexName("test2020", "{\"@timestamp\":\"2020-06-01\",\"field2020\":\"foo\"}"); - - // filter the results so that only test2020's columns are displayed - Map actual = runSql( - new StringEntity( - query("SYS COLUMNS").mode(mode).filter("{\"range\": {\"@timestamp\": {\"gte\": \"2020\"}}}").toString(), - ContentType.APPLICATION_JSON - ), - StringUtils.EMPTY, - mode - ); - @SuppressWarnings("unchecked") - List> rows = (List>) actual.get("rows"); - assertEquals(3, rows.size()); - List currentRow = rows.get(0); - assertEquals("test2020", currentRow.get(2)); - assertEquals("@timestamp", currentRow.get(3)); - currentRow = rows.get(1); - assertEquals("test2020", currentRow.get(2)); - assertEquals("field2020", currentRow.get(3)); - currentRow = rows.get(2); - assertEquals("test2020", currentRow.get(2)); - assertEquals("field2020.keyword", currentRow.get(3)); - } - - /** - * Similar test with {@link #testSysColumnsCommandWithFilter()} but using "SHOW COLUMNS" command which, compared to "SYS COLUMNS" - * goes through a different calls path in IndexResolver - */ - @SuppressWarnings("unchecked") - public void testShowColumnsCommandWithFilter() throws IOException { - String mode = randomMode(); - // create three indices with same @timestamp date field and with differently named one more field - indexWithIndexName("test2018", "{\"@timestamp\":\"2018-06-01\",\"field2018\":\"foo\"}"); - indexWithIndexName("test2019", "{\"@timestamp\":\"2019-06-01\",\"field2019\":\"foo\"}"); - indexWithIndexName("test2020", "{\"@timestamp\":\"2020-06-01\",\"field2020\":\"foo\"}"); - - // filter the results so that only test2020's columns are displayed - Map actual = runSql( - new StringEntity( - query("SHOW COLUMNS FROM test2020").mode(mode).filter("{\"range\": {\"@timestamp\": {\"gte\": \"2020\"}}}").toString(), - ContentType.APPLICATION_JSON - ), - StringUtils.EMPTY, - mode - ); - - List> rows = (List>) actual.get("rows"); - assertEquals(3, rows.size()); - List currentRow = rows.get(0); - assertEquals("@timestamp", currentRow.get(0)); - currentRow = rows.get(1); - assertEquals("field2020", currentRow.get(0)); - currentRow = rows.get(2); - assertEquals("field2020.keyword", currentRow.get(0)); - - // the second test is from an index that is filtered out by the range filter, so the result list should be empty - actual = runSql( - new StringEntity( - query("SHOW COLUMNS FROM test2019").mode(mode).filter("{\"range\": {\"@timestamp\": {\"gte\": \"2020\"}}}").toString(), - ContentType.APPLICATION_JSON - ), - StringUtils.EMPTY, - mode - ); - - rows = (List>) actual.get("rows"); - assertTrue(rows.isEmpty()); - } - public void testBasicTranslateQueryWithFilter() throws IOException { index("{\"test\":\"foo\"}", "{\"test\":\"bar\"}"); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumns.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumns.java index ba789edd4c8..d510e2c4926 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumns.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumns.java @@ -66,7 +66,7 @@ public class ShowColumns extends Command { String regex = pattern != null ? pattern.asJavaRegex() : null; boolean withFrozen = includeFrozen || session.configuration().includeFrozen(); - session.indexResolver().resolveAsMergedMapping(idx, regex, withFrozen, session.configuration().filter(), ActionListener.wrap( + session.indexResolver().resolveAsMergedMapping(idx, regex, withFrozen, ActionListener.wrap( indexResult -> { List> rows = emptyList(); if (indexResult.isValid()) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java index 8730915b5ee..53633cd5875 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java @@ -128,7 +128,7 @@ public class SysColumns extends Command { // special case for '%' (translated to *) if ("*".equals(idx)) { - session.indexResolver().resolveAsSeparateMappings(idx, regex, includeFrozen, session.configuration().filter(), + session.indexResolver().resolveAsSeparateMappings(idx, regex, includeFrozen, ActionListener.wrap(esIndices -> { List> rows = new ArrayList<>(); for (EsIndex esIndex : esIndices) { @@ -139,7 +139,7 @@ public class SysColumns extends Command { } // otherwise use a merged mapping else { - session.indexResolver().resolveAsMergedMapping(idx, regex, includeFrozen, session.configuration().filter(), + session.indexResolver().resolveAsMergedMapping(idx, regex, includeFrozen, ActionListener.wrap(r -> { List> rows = new ArrayList<>(); // populate the data only when a target is found diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java index 9a6e2efacc3..c93000308cd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java @@ -140,7 +140,7 @@ public class SqlSession implements Session { } boolean includeFrozen = configuration.includeFrozen() || tableInfo.isFrozen(); - indexResolver.resolveAsMergedMapping(table.index(), null, includeFrozen, configuration.filter(), + indexResolver.resolveAsMergedMapping(table.index(), null, includeFrozen, wrap(indexResult -> listener.onResponse(action.apply(indexResult)), listener::onFailure)); } else { try { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java index 36027999b53..ea3adea32af 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java @@ -559,13 +559,13 @@ public class SysColumnsTests extends ESTestCase { IndexResolver resolver = mock(IndexResolver.class); when(resolver.clusterName()).thenReturn(CLUSTER_NAME); doAnswer(invocation -> { - ((ActionListener) invocation.getArguments()[4]).onResponse(IndexResolution.valid(test)); + ((ActionListener) invocation.getArguments()[3]).onResponse(IndexResolution.valid(test)); return Void.TYPE; - }).when(resolver).resolveAsMergedMapping(any(), any(), anyBoolean(), any(), any()); + }).when(resolver).resolveAsMergedMapping(any(), any(), anyBoolean(), any()); doAnswer(invocation -> { - ((ActionListener>) invocation.getArguments()[4]).onResponse(singletonList(test)); + ((ActionListener>) invocation.getArguments()[3]).onResponse(singletonList(test)); return Void.TYPE; - }).when(resolver).resolveAsSeparateMappings(any(), any(), anyBoolean(), any(), any()); + }).when(resolver).resolveAsSeparateMappings(any(), any(), anyBoolean(), any()); SqlSession session = new SqlSession(config, null, null, resolver, null, null, null, null, null); return new Tuple<>(cmd, session);