From a5452603ccc91fe82d7f491ced34414014e08a1f Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 29 Mar 2022 11:38:52 +0100 Subject: [PATCH] Extra testing and some cleanups for filtering on field caps (#85068) * adds a test for mixed cluster requests * fixes a bad stream version check (above test will fail if this isn't included) * replaces private FieldCapsFilter interface with Predicate * renames 'allowedTypes' to 'types' to maintain consistency with external API * adds javadoc to ResponseRewriter * removes isRuntimeField from FieldTypeLookup Relates to #83636 --- docs/reference/search/field-caps.asciidoc | 23 +++++- .../elasticsearch/upgrades/FieldCapsIT.java | 79 ++++++++++++++++--- .../test/field_caps/50_fieldtype_filter.yml | 15 ++++ .../fieldcaps/FieldCapabilitiesFetcher.java | 36 ++++----- .../fieldcaps/FieldCapabilitiesRequest.java | 20 ++--- .../fieldcaps/IndexFieldCapabilities.java | 2 +- .../action/fieldcaps/RequestDispatcher.java | 2 +- .../action/fieldcaps/ResponseRewriter.java | 6 +- .../TransportFieldCapabilitiesAction.java | 4 +- .../index/mapper/FieldTypeLookup.java | 6 -- .../index/mapper/MapperService.java | 3 + .../index/mapper/MappingLookup.java | 6 +- .../index/query/SearchExecutionContext.java | 3 + .../action/RestFieldCapabilitiesAction.java | 2 +- .../FieldCapabilitiesRequestTests.java | 8 ++ .../index/mapper/MapperServiceTests.java | 33 ++++++++ .../query/SearchExecutionContextTests.java | 30 ++++++- .../test/rest/ESRestTestCase.java | 27 ++++++- 18 files changed, 243 insertions(+), 62 deletions(-) diff --git a/docs/reference/search/field-caps.asciidoc b/docs/reference/search/field-caps.asciidoc index 580553b027fa2..c6c7e2cbe75c2 100644 --- a/docs/reference/search/field-caps.asciidoc +++ b/docs/reference/search/field-caps.asciidoc @@ -78,13 +78,28 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=index-ignore-unavailab Defaults to `false`. `filters`:: -(Optional, string) Comma-separated list of filters to apply to the response. The -following filters are supported: +metadata,-metadata,-parent,-nested,-multifield +(Optional, string) Comma-separated list of filters to apply to the response. ++ +.Valid values for `filters` +[%collapsible%open] +==== +`+metadata`:: +Only include metadata fields +`-metadata`:: +Exclude metadata fields +`-parent`:: +Exclude parent fields +`-nested`:: +Exclude nested fields +`-multifield`:: +Exclude multifields +==== `types`:: (Optional, string) Comma-separated list of field types to include. Any fields that do not match one of these types will be excluded from the results. Defaults to empty, -meaning that all field types are returned. +meaning that all field types are returned. See <> for +more information about field types in field capabilities requests and responses. [[search-field-caps-api-request-body]] ==== {api-request-body-title} @@ -103,7 +118,7 @@ same name in the index mappings. [[search-field-caps-api-response-body]] ==== {api-response-body-title} - +[[field-caps-field-types]] The types used in the response describe _families_ of field types. Normally a type family is the same as the field type declared in the mapping, but to simplify matters certain field types that behave identically are diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FieldCapsIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FieldCapsIT.java index ddf61aa4e25db..83865222a8867 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FieldCapsIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FieldCapsIT.java @@ -8,8 +8,11 @@ package org.elasticsearch.upgrades; +import org.apache.http.HttpHost; +import org.elasticsearch.Version; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; import org.elasticsearch.client.Request; +import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.QueryBuilder; @@ -17,7 +20,9 @@ import org.elasticsearch.xcontent.json.JsonXContent; import org.junit.Before; +import java.io.IOException; import java.util.List; +import java.util.Map; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; @@ -27,6 +32,9 @@ * reduce the transport message size between nodes and clusters, and the memory usage to hold these internal responses. * As the optimization is applied for only field-caps requests without index-filter and nodes on 8.2 or later, * these BWC tests verify these combinations of field-caps requests: (old|new|mixed indices) and (with|without index filter) + * + * In 8.2 we also added the ability to filter fields by type and metadata, with some post-hoc filtering applied on + * the co-ordinating node if older nodes were included in the system */ public class FieldCapsIT extends AbstractRollingTestCase { private static boolean indicesCreated = false; @@ -42,6 +50,7 @@ public void setupIndices() throws Exception { "red_field": { "type": "keyword" }, "yellow_field": { "type": "integer" }, "blue_field": { "type": "keyword" }, + "multi_field" : { "type" : "ip", "fields" : { "keyword" : { "type" : "keyword" } } }, "timestamp": {"type": "date"} } """; @@ -50,6 +59,7 @@ public void setupIndices() throws Exception { "green_field": { "type": "keyword" }, "yellow_field": { "type": "long" }, "blue_field": { "type": "keyword" }, + "multi_field" : { "type" : "ip", "fields" : { "keyword" : { "type" : "keyword" } } }, "timestamp": {"type": "date"} } """; @@ -88,7 +98,7 @@ public void setupIndices() throws Exception { public void testOldIndicesOnly() throws Exception { { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_red_*"), List.of("*"), null); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_red_*"), List.of("*"), null, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "old_red_1", "old_red_2", "old_red_empty" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -98,7 +108,7 @@ public void testOldIndicesOnly() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, null, null); assertThat( resp.getIndices(), equalTo(new String[] { "old_green_1", "old_green_2", "old_green_empty", "old_red_1", "old_red_2", "old_red_empty" }) @@ -116,7 +126,7 @@ public void testOldIndicesOnly() throws Exception { public void testOldIndicesWithIndexFilter() throws Exception { final QueryBuilder indexFilter = QueryBuilders.rangeQuery("timestamp").gte("2020-01-01").lte("2020-12-12"); { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_red_*"), List.of("*"), indexFilter); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_red_*"), List.of("*"), indexFilter, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "old_red_1", "old_red_2" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -126,7 +136,7 @@ public void testOldIndicesWithIndexFilter() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), indexFilter); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), indexFilter, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "old_green_1", "old_green_2", "old_red_1", "old_red_2" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -141,7 +151,7 @@ public void testOldIndicesWithIndexFilter() throws Exception { public void testNewIndicesOnly() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); { - FieldCapabilitiesResponse resp = fieldCaps(List.of("new_red_*"), List.of("*"), null); + FieldCapabilitiesResponse resp = fieldCaps(List.of("new_red_*"), List.of("*"), null, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "new_red_1", "new_red_2", "new_red_empty" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -151,7 +161,7 @@ public void testNewIndicesOnly() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } { - FieldCapabilitiesResponse resp = fieldCaps(List.of("new_*"), List.of("*"), null); + FieldCapabilitiesResponse resp = fieldCaps(List.of("new_*"), List.of("*"), null, null, null); assertThat( resp.getIndices(), equalTo(new String[] { "new_green_1", "new_green_2", "new_green_empty", "new_red_1", "new_red_2", "new_red_empty" }) @@ -170,7 +180,7 @@ public void testNewIndicesOnlyWithIndexFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); final QueryBuilder indexFilter = QueryBuilders.rangeQuery("timestamp").gte("2020-01-01").lte("2020-12-12"); { - FieldCapabilitiesResponse resp = fieldCaps(List.of("new_red_*"), List.of("*"), indexFilter); + FieldCapabilitiesResponse resp = fieldCaps(List.of("new_red_*"), List.of("*"), indexFilter, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "new_red_1", "new_red_2" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -180,7 +190,7 @@ public void testNewIndicesOnlyWithIndexFilter() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } { - FieldCapabilitiesResponse resp = fieldCaps(List.of("new_*"), List.of("*"), indexFilter); + FieldCapabilitiesResponse resp = fieldCaps(List.of("new_*"), List.of("*"), indexFilter, null, null); assertThat(resp.getIndices(), equalTo(new String[] { "new_green_1", "new_green_2", "new_red_1", "new_red_2" })); assertThat(resp.getField("red_field").keySet(), contains("keyword")); assertTrue(resp.getField("red_field").get("keyword").isSearchable()); @@ -194,7 +204,7 @@ public void testNewIndicesOnlyWithIndexFilter() throws Exception { public void testAllIndices() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, null, null); assertThat( resp.getIndices(), equalTo( @@ -227,7 +237,7 @@ public void testAllIndices() throws Exception { public void testAllIndicesWithIndexFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); final QueryBuilder indexFilter = QueryBuilders.rangeQuery("timestamp").gte("2020-01-01").lte("2020-12-12"); - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), indexFilter); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), indexFilter, null, null); assertThat( resp.getIndices(), equalTo( @@ -252,4 +262,53 @@ public void testAllIndicesWithIndexFilter() throws Exception { assertThat(resp.getField("blue_field").keySet(), contains("keyword")); assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } + + @SuppressWarnings("unchecked") + // Returns a client connected to one of the upgraded nodes. + private RestClient getUpgradedNodeClient() throws IOException { + for (HttpHost host : getClusterHosts()) { + RestClient client = RestClient.builder(host).build(); + Request nodesRequest = new Request("GET", "_nodes/_local/_none"); + Map nodeMap = (Map) entityAsMap(client.performRequest(nodesRequest)).get("nodes"); + Map nameMap = (Map) nodeMap.values().iterator().next(); + String version = (String) nameMap.get("version"); + if (version.equals(Version.CURRENT.toString())) { + return client; + } + client.close(); + } + throw new IllegalStateException("Couldn't find node on version " + Version.CURRENT); + } + + // Test field type filtering on mixed cluster + // We need to use a client that is connected to one of the upgraded nodes, + // because we are testing that the upgraded node will correctly apply filtering + // to responses from older nodes that don't understand the filter parameters + public void testAllIndicesWithFieldTypeFilter() throws Exception { + assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); + RestClient restClient = getUpgradedNodeClient(); + FieldCapabilitiesResponse resp = fieldCaps(restClient, List.of("old_*", "new_*"), List.of("*"), null, "keyword", null); + assertThat(resp.getField("red_field").keySet(), contains("keyword")); + assertNull(resp.getField("yellow_field")); + restClient.close(); + } + + // Test multifield exclusion on mixed cluster + // We need to use a client that is connected to one of the upgraded nodes, + // because we are testing that the upgraded node will correctly apply filtering + // to responses from older nodes that don't understand the filter parameters + public void testAllIndicesWithExclusionFilter() throws Exception { + assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); + RestClient client = getUpgradedNodeClient(); + { + FieldCapabilitiesResponse resp = fieldCaps(client, List.of("old_*", "new_*"), List.of("*"), null, null, null); + assertThat(resp.getField("multi_field.keyword").keySet(), contains("keyword")); + } + { + FieldCapabilitiesResponse resp = fieldCaps(client, List.of("old_*", "new_*"), List.of("*"), null, null, "-multifield"); + assertThat(resp.getField("multi_field").keySet(), contains("ip")); + assertNull(resp.getField("multi_field.keyword")); + } + client.close(); + } } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/50_fieldtype_filter.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/50_fieldtype_filter.yml index cfed4f68ea5e7..667caf1ba92a7 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/50_fieldtype_filter.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/50_fieldtype_filter.yml @@ -186,6 +186,21 @@ setup: - is_false: fields.text\\.keyword - is_true: fields.misc\\.keyword +--- +"Exclude multifields but include runtime fields": + - do: + field_caps: + index: 'test1,test2,test3' + fields: '*' + filters: '-multifield' + body: + runtime_mappings: + text.keyword: + type: keyword + + - is_true: fields.text\\.keyword + - is_true: fields.misc\\.keyword + --- "Field type filters": - do: diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java index 2926b277ba52e..983da5855b711 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java @@ -109,11 +109,11 @@ static Map retrieveFieldCaps( boolean includeParentObjects = checkIncludeParents(filters); - FieldCapsFilter filter = buildFilter(indexFieldfilter, filters, types); + Predicate filter = buildFilter(indexFieldfilter, filters, types, context); Map responseMap = new HashMap<>(); for (String field : fieldNames) { MappedFieldType ft = context.getFieldType(field); - if (filter.matches(ft, context)) { + if (filter.test(ft)) { IndexFieldCapabilities fieldCap = new IndexFieldCapabilities( field, ft.familyTypeName(), @@ -168,9 +168,6 @@ private static boolean checkIncludeParents(String[] filters) { if ("-parent".equals(filter)) { return false; } - if ("parent".equals(filter)) { - return true; - } } return true; } @@ -190,30 +187,27 @@ private boolean canMatchShard( return SearchService.queryStillMatchesAfterRewrite(searchRequest, searchExecutionContext); } - private interface FieldCapsFilter { - boolean matches(MappedFieldType fieldType, SearchExecutionContext context); - - default FieldCapsFilter and(FieldCapsFilter other) { - return (ft, context) -> matches(ft, context) && other.matches(ft, context); - } - } - - private static FieldCapsFilter buildFilter(Predicate fieldFilter, String[] filters, String[] fieldTypes) { + private static Predicate buildFilter( + Predicate fieldFilter, + String[] filters, + String[] fieldTypes, + SearchExecutionContext context + ) { // security filters don't exclude metadata fields - FieldCapsFilter fcf = (ft, c) -> fieldFilter.test(ft.name()) || c.isMetadataField(ft.name()); + Predicate fcf = ft -> fieldFilter.test(ft.name()) || context.isMetadataField(ft.name()); if (fieldTypes.length > 0) { Set acceptedTypes = Set.of(fieldTypes); - fcf = fcf.and((ft, c) -> acceptedTypes.contains(ft.familyTypeName())); + fcf = fcf.and(ft -> acceptedTypes.contains(ft.familyTypeName())); } for (String filter : filters) { if ("parent".equals(filter) || "-parent".equals(filter)) { continue; } - FieldCapsFilter next = switch (filter) { - case "+metadata" -> (ft, c) -> c.isMetadataField(ft.name()); - case "-metadata" -> (ft, c) -> c.isMetadataField(ft.name()) == false; - case "-nested" -> (ft, c) -> c.nestedLookup().getNestedParent(ft.name()) == null; - case "-multifield" -> (ft, c) -> c.isMultiField(ft.name()) == false; + Predicate next = switch (filter) { + case "+metadata" -> ft -> context.isMetadataField(ft.name()); + case "-metadata" -> ft -> context.isMetadataField(ft.name()) == false; + case "-nested" -> ft -> context.nestedLookup().getNestedParent(ft.name()) == null; + case "-multifield" -> ft -> context.isMultiField(ft.name()) == false; default -> throw new IllegalArgumentException("Unknown field caps filter [" + filter + "]"); }; fcf = fcf.and(next); diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java index f6e390244265e..b1b51de5c0693 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequest.java @@ -37,7 +37,7 @@ public final class FieldCapabilitiesRequest extends ActionRequest implements Ind private IndicesOptions indicesOptions = DEFAULT_INDICES_OPTIONS; private String[] fields = Strings.EMPTY_ARRAY; private String[] filters = Strings.EMPTY_ARRAY; - private String[] allowedTypes = Strings.EMPTY_ARRAY; + private String[] types = Strings.EMPTY_ARRAY; private boolean includeUnmapped = false; // pkg private API mainly for cross cluster search to signal that we do multiple reductions ie. the results should not be merged private boolean mergeResults = true; @@ -57,7 +57,7 @@ public FieldCapabilitiesRequest(StreamInput in) throws IOException { runtimeFields = in.readMap(); if (in.getVersion().onOrAfter(Version.V_8_2_0)) { filters = in.readStringArray(); - allowedTypes = in.readStringArray(); + types = in.readStringArray(); } } @@ -95,7 +95,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeGenericMap(runtimeFields); if (out.getVersion().onOrAfter(Version.V_8_2_0)) { out.writeStringArray(filters); - out.writeStringArray(allowedTypes); + out.writeStringArray(types); } } @@ -137,13 +137,13 @@ public String[] filters() { return filters; } - public FieldCapabilitiesRequest allowedTypes(String... types) { - this.allowedTypes = types; + public FieldCapabilitiesRequest types(String... types) { + this.types = types; return this; } - public String[] allowedTypes() { - return allowedTypes; + public String[] types() { + return types; } /** @@ -243,7 +243,7 @@ public boolean equals(Object o) { && Objects.equals(indexFilter, that.indexFilter) && Objects.equals(nowInMillis, that.nowInMillis) && Arrays.equals(filters, that.filters) - && Arrays.equals(allowedTypes, that.allowedTypes) + && Arrays.equals(types, that.types) && Objects.equals(runtimeFields, that.runtimeFields); } @@ -253,7 +253,7 @@ public int hashCode() { result = 31 * result + Arrays.hashCode(indices); result = 31 * result + Arrays.hashCode(fields); result = 31 * result + Arrays.hashCode(filters); - result = 31 * result + Arrays.hashCode(allowedTypes); + result = 31 * result + Arrays.hashCode(types); return result; } @@ -266,7 +266,7 @@ public String getDescription() { stringBuilder.append("], filters["); stringBuilder.append(Strings.collectionToDelimitedString(Arrays.asList(filters), ",")); stringBuilder.append("], types["); - stringBuilder.append(Strings.collectionToDelimitedString(Arrays.asList(allowedTypes), ",")); + stringBuilder.append(Strings.collectionToDelimitedString(Arrays.asList(types), ",")); stringBuilder.append("]"); return stringBuilder.toString(); } diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java index 6cefa39f7f6ab..c08540e08ae79 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/IndexFieldCapabilities.java @@ -35,7 +35,7 @@ public class IndexFieldCapabilities implements Writeable { private final TimeSeriesParams.MetricType metricType; private final Map meta; - public static IndexFieldCapabilities withMetadata(IndexFieldCapabilities input, boolean isMetadata) { + static IndexFieldCapabilities withMetadata(IndexFieldCapabilities input, boolean isMetadata) { return new IndexFieldCapabilities( input.getName(), input.getType(), diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/RequestDispatcher.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/RequestDispatcher.java index f9d5cff2471b4..248caac8a33d6 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/RequestDispatcher.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/RequestDispatcher.java @@ -175,7 +175,7 @@ private void sendRequestToNode(String nodeId, List shardIds) { shardIds, fieldCapsRequest.fields(), fieldCapsRequest.filters(), - fieldCapsRequest.allowedTypes(), + fieldCapsRequest.types(), originalIndices, fieldCapsRequest.indexFilter(), nowInMillis, diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java index 59c1caa80c2e6..5ea9d1f460836 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java @@ -17,6 +17,10 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +/** + * Applies field type filters to field caps responses that come from earlier versions of ES + * that do not support filtering directly. + */ final class ResponseRewriter { public static Map rewriteOldResponses( @@ -26,7 +30,7 @@ public static Map rewriteOldResponses( String[] allowedTypes, Predicate isMetadata ) { - if (version.onOrAfter(Version.V_8_1_0)) { + if (version.onOrAfter(Version.V_8_2_0)) { return input; // nothing needs to be done } Function transformer = buildTransformer( diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java index a12bef8aa74d4..6508eb0b6686b 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java @@ -221,7 +221,7 @@ private static FieldCapabilitiesRequest prepareRemoteRequest( remoteRequest.indices(originalIndices.indices()); remoteRequest.fields(request.fields()); remoteRequest.filters(request.filters()); - remoteRequest.allowedTypes(request.allowedTypes()); + remoteRequest.types(request.types()); remoteRequest.runtimeFields(request.runtimeFields()); remoteRequest.indexFilter(request.indexFilter()); remoteRequest.nowInMillis(nowInMillis); @@ -281,7 +281,7 @@ private void innerMerge( response.getOriginVersion(), response.get(), request.filters(), - request.allowedTypes(), + request.types(), metadataFieldPred ); for (Map.Entry entry : fields.entrySet()) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index c67d64bd2048c..af51b42deda80 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -25,7 +25,6 @@ final class FieldTypeLookup { private final Map fullNameToFieldType = new HashMap<>(); private final Map dynamicFieldTypes = new HashMap<>(); - private final Set runtimeFieldNames = new HashSet<>(); /** * A map from field name to all fields whose content has been copied into it @@ -81,7 +80,6 @@ final class FieldTypeLookup { for (MappedFieldType fieldType : RuntimeField.collectFieldTypes(runtimeFields).values()) { // this will override concrete fields with runtime fields that have the same name fullNameToFieldType.put(fieldType.name(), fieldType); - runtimeFieldNames.add(fieldType.name()); } } @@ -106,10 +104,6 @@ MappedFieldType get(String field) { return getDynamicField(field); } - boolean isRuntimeField(String field) { - return runtimeFieldNames.contains(field); - } - // for testing int getMaxParentPathDots() { return maxParentPathDots; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 812e7e80d969c..8957fc0f14ac2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -505,6 +505,9 @@ public boolean isMetadataField(String field) { return mapperRegistry.getMetadataMapperParsers(indexVersionCreated).containsKey(field); } + /** + * @return If this field is defined as a multifield of another field + */ public boolean isMultiField(String field) { return mappingLookup().isMultiField(field); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index e35b283af0197..c37ced7ea92e9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -336,7 +336,11 @@ public NestedLookup nestedLookup() { } public boolean isMultiField(String field) { - if (fieldTypeLookup.isRuntimeField(field)) { + if (fieldMappers.containsKey(field) == false) { + return false; + } + // Is it a runtime field? + if (indexTimeLookup.get(field) != fieldTypeLookup.get(field)) { return false; } String sourceParent = parentObject(field); diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java index ee584dd9acb38..4252cb3b012e8 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java @@ -375,6 +375,9 @@ public boolean isMetadataField(String field) { } public boolean isMultiField(String field) { + if (runtimeMappings.containsKey(field)) { + return false; + } return mapperService.isMultiField(field); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java b/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java index f39b3d3a3479c..879faff87e0b6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/RestFieldCapabilitiesAction.java @@ -51,7 +51,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC fieldRequest.indicesOptions(IndicesOptions.fromRequest(request, fieldRequest.indicesOptions())); fieldRequest.includeUnmapped(request.paramAsBoolean("include_unmapped", false)); fieldRequest.filters(request.paramAsStringArray("filters", Strings.EMPTY_ARRAY)); - fieldRequest.allowedTypes(request.paramAsStringArray("types", Strings.EMPTY_ARRAY)); + fieldRequest.types(request.paramAsStringArray("types", Strings.EMPTY_ARRAY)); request.withContentOrSourceParamParserOrNull(parser -> { if (parser != null) { PARSER.parse(parser, fieldRequest, null); diff --git a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java index dc72d180ef0c2..eab18d96c2bf7 100644 --- a/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesRequestTests.java @@ -70,6 +70,12 @@ protected FieldCapabilitiesRequest createTestInstance() { if (randomBoolean()) { request.runtimeFields(Collections.singletonMap(randomAlphaOfLength(5), randomAlphaOfLength(5))); } + if (randomBoolean()) { + request.filters("-nested"); + } + if (randomBoolean()) { + request.types(randomAlphaOfLength(5)); + } return request; } @@ -109,6 +115,8 @@ protected FieldCapabilitiesRequest mutateInstance(FieldCapabilitiesRequest insta request -> request.indexFilter(request.indexFilter() != null ? request.indexFilter().boost(2) : QueryBuilders.matchAllQuery()) ); mutators.add(request -> request.runtimeFields(Collections.singletonMap("other_key", "other_value"))); + mutators.add(request -> request.filters(request.filters().length == 0 ? new String[] { "-metadata" } : Strings.EMPTY_ARRAY)); + mutators.add(request -> request.types(request.types().length == 0 ? new String[] { "keyword" } : Strings.EMPTY_ARRAY)); FieldCapabilitiesRequest mutatedInstance = copyInstance(instance); Consumer mutator = randomFrom(mutators); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index 134c869d24927..f3771510d8da9 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -320,4 +320,37 @@ public void testEagerGlobalOrdinals() throws IOException { assertThat(eagerFieldNames, containsInAnyOrder("eager1", "eager2")); } + public void testMultiFieldChecks() throws IOException { + MapperService mapperService = createMapperService(""" + { "_doc" : { + "properties" : { + "field1" : { + "type" : "keyword", + "fields" : { + "subfield1" : { + "type" : "long" + }, + "subfield2" : { + "type" : "text" + } + } + }, + "object.field2" : { "type" : "keyword" } + }, + "runtime" : { + "object.subfield1" : { "type" : "keyword" }, + "field1.subfield2" : { "type" : "keyword" } + } + } } + """); + + assertFalse(mapperService.isMultiField("non_existent_field")); + assertFalse(mapperService.isMultiField("field1")); + assertTrue(mapperService.isMultiField("field1.subfield1")); + // not a multifield, because it's shadowed by a runtime field + assertFalse(mapperService.isMultiField("field1.subfield2")); + assertFalse(mapperService.isMultiField("object.field2")); + assertFalse(mapperService.isMultiField("object.subfield1")); + } + } diff --git a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java index 625fcbb97ab29..1d19447977845 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java @@ -72,6 +72,7 @@ import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.mockito.stubbing.Answer; import java.io.IOException; import java.util.ArrayList; @@ -93,6 +94,7 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -366,6 +368,27 @@ public void testSearchRequestRuntimeFieldsRemoval() { assertEquals("Runtime field [field] was set to null but its removal is not supported in this context", exception.getMessage()); } + public void testSearchRequestRuntimeFieldsAndMultifieldDetection() { + Map runtimeMappings = Map.ofEntries( + Map.entry("cat", Map.of("type", "keyword")), + Map.entry("cat.subfield", Map.of("type", "keyword")), + Map.entry("dog", Map.of("type", "long")) + ); + MappingLookup mappingLookup = createMappingLookup( + List.of( + new MockFieldMapper.FakeFieldType("pig"), + new MockFieldMapper.FakeFieldType("pig.subfield"), + new MockFieldMapper.FakeFieldType("cat"), + new MockFieldMapper.FakeFieldType("cat.subfield") + ), + List.of(new TestRuntimeField("runtime", "long")) + ); + SearchExecutionContext context = createSearchExecutionContext("uuid", null, mappingLookup, runtimeMappings); + assertTrue(context.isMultiField("pig.subfield")); + assertFalse(context.isMultiField("cat.subfield")); + assertTrue(mappingLookup.isMultiField("cat.subfield")); + } + public static SearchExecutionContext createSearchExecutionContext(String indexUuid, String clusterAlias) { return createSearchExecutionContext(indexUuid, clusterAlias, MappingLookup.EMPTY, Map.of()); } @@ -395,7 +418,7 @@ private static SearchExecutionContext createSearchExecutionContext( ); IndexMetadata indexMetadata = indexMetadataBuilder.build(); IndexSettings indexSettings = new IndexSettings(indexMetadata, Settings.EMPTY); - MapperService mapperService = createMapperService(indexSettings); + MapperService mapperService = createMapperService(indexSettings, mappingLookup); final long nowInMillis = randomNonNegativeLong(); return new SearchExecutionContext( 0, @@ -420,7 +443,7 @@ private static SearchExecutionContext createSearchExecutionContext( ); } - private static MapperService createMapperService(IndexSettings indexSettings) { + private static MapperService createMapperService(IndexSettings indexSettings, MappingLookup mappingLookup) { IndexAnalyzers indexAnalyzers = new IndexAnalyzers( singletonMap("default", new NamedAnalyzer("default", AnalyzerScope.INDEX, null)), emptyMap(), @@ -445,6 +468,9 @@ private static MapperService createMapperService(IndexSettings indexSettings) { indexSettings.getMode().buildIdFieldMapper(() -> true) ) ); + when(mapperService.isMultiField(anyString())).then( + (Answer) invocation -> mappingLookup.isMultiField(invocation.getArgument(0)) + ); return mapperService; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index e0cf58cdbc607..4b99cfdf84550 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -1975,10 +1975,33 @@ protected static boolean isNotFoundResponseException(IOException ioe) { return false; } - protected FieldCapabilitiesResponse fieldCaps(List indices, List fields, QueryBuilder indexFilter) throws IOException { + protected FieldCapabilitiesResponse fieldCaps( + List indices, + List fields, + QueryBuilder indexFilter, + String fieldTypes, + String fieldFilters + ) throws IOException { + return fieldCaps(client(), indices, fields, indexFilter, fieldTypes, fieldFilters); + } + + protected FieldCapabilitiesResponse fieldCaps( + RestClient restClient, + List indices, + List fields, + QueryBuilder indexFilter, + String fieldTypes, + String fieldFilters + ) throws IOException { Request request = new Request("POST", "/_field_caps"); request.addParameter("index", String.join(",", indices)); request.addParameter("fields", String.join(",", fields)); + if (fieldTypes != null) { + request.addParameter("types", fieldTypes); + } + if (fieldFilters != null) { + request.addParameter("filters", fieldFilters); + } if (indexFilter != null) { XContentBuilder body = JsonXContent.contentBuilder(); body.startObject(); @@ -1986,7 +2009,7 @@ protected FieldCapabilitiesResponse fieldCaps(List indices, List body.endObject(); request.setJsonEntity(Strings.toString(body)); } - Response response = client().performRequest(request); + Response response = restClient.performRequest(request); assertOK(response); try (XContentParser parser = createParser(JsonXContent.jsonXContent, response.getEntity().getContent())) { return FieldCapabilitiesResponse.fromXContent(parser);