From 7ca83efd3fce447fb406123410ca1c8120574170 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 8 Mar 2022 10:43:27 +0000 Subject: [PATCH 01/14] WIP --- .../fieldcaps/FieldCapabilitiesFetcher.java | 33 +++++++++---------- .../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 | 2 +- .../index/mapper/MappingLookup.java | 2 +- .../index/query/SearchExecutionContext.java | 3 ++ .../action/RestFieldCapabilitiesAction.java | 2 +- 11 files changed, 40 insertions(+), 42 deletions(-) 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..19692b4a253c8 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(), @@ -190,30 +190,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..b94016d729fb5 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 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 4528de36db026..d41e4d67ecfd1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -507,7 +507,7 @@ public boolean isMetadataField(String field) { } public boolean isMultiField(String field) { - return mappingLookup().isMultiField(field); + return mappingLookup().isShadowed(field) == false && mappingLookup().isMultiField(field); } public synchronized List reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException { 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..06cc37e17e9d2 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,7 @@ public NestedLookup nestedLookup() { } public boolean isMultiField(String field) { - if (fieldTypeLookup.isRuntimeField(field)) { + if (fieldMappers.containsKey(field) == false) { 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); From 7f5857df0aa8358df4f4e7e5887290c9aebf00ea Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 17 Mar 2022 15:46:02 +0000 Subject: [PATCH 02/14] add mixed-cluster test --- .../elasticsearch/upgrades/FieldCapsIT.java | 67 ++++++++++++++++--- .../test/rest/ESRestTestCase.java | 14 +++- 2 files changed, 70 insertions(+), 11 deletions(-) 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..76638ed1ec2fd 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 @@ -27,6 +27,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 +45,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 +54,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 +93,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 +103,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 +121,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 +131,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 +146,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 +156,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 +175,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 +185,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 +199,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 +232,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 +257,46 @@ public void testAllIndicesWithIndexFilter() throws Exception { assertThat(resp.getField("blue_field").keySet(), contains("keyword")); assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } + + // Test field type filtering on old cluster + public void testFieldTypeFilteringOnOldOnly() throws Exception { + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, "keyword", null); + assertThat(resp.getField("red_field").keySet(), contains("keyword")); + assertNull(resp.getField("yellow_field")); + } + + // Test field type filtering on mixed cluster + public void testAllIndicesWithFieldTypeFilter() throws Exception { + assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, "keyword", null); + assertThat(resp.getField("red_field").keySet(), contains("keyword")); + assertNull(resp.getField("yellow_field")); + } + + // Test multifield exclusion on old cluster + public void testExclusionFilterOnOldOnly() throws Exception { + { + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, null, null); + assertThat(resp.getField("multi_field.keyword").keySet(), contains("keyword")); + } + { + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, null, "-multifield"); + assertThat(resp.getField("multi_field").keySet(), contains("ip")); + assertNull(resp.getField("multi_field.keyword")); + } + } + + // Test multifield exclusion on mixed cluster + public void testAllIndicesWithExclusionFilter() throws Exception { + assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); + { + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, null, null); + assertThat(resp.getField("multi_field.keyword").keySet(), contains("keyword")); + } + { + FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, null, "-multifield"); + assertThat(resp.getField("multi_field").keySet(), contains("ip")); + assertNull(resp.getField("multi_field.keyword")); + } + } } 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 25dac3d3565dd..18aa50affc00c 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 @@ -1962,10 +1962,22 @@ 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 { 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(); From 38087193fa91ba3314502904bb8d84086d455173 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 17 Mar 2022 15:50:44 +0000 Subject: [PATCH 03/14] Update docs/changelog/85068.yaml --- docs/changelog/85068.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/85068.yaml diff --git a/docs/changelog/85068.yaml b/docs/changelog/85068.yaml new file mode 100644 index 0000000000000..76f632d3a1bae --- /dev/null +++ b/docs/changelog/85068.yaml @@ -0,0 +1,5 @@ +pr: 85068 +summary: Extra testing and some cleanups for filtering on field caps +area: Mapping +type: tech debt +issues: [] From 492e16445ed2cbad54d539dc14dbe5bccce066ff Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 18 Mar 2022 12:12:53 +0000 Subject: [PATCH 04/14] Delete docs/changelog/85068.yaml --- docs/changelog/85068.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/85068.yaml diff --git a/docs/changelog/85068.yaml b/docs/changelog/85068.yaml deleted file mode 100644 index 76f632d3a1bae..0000000000000 --- a/docs/changelog/85068.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 85068 -summary: Extra testing and some cleanups for filtering on field caps -area: Mapping -type: tech debt -issues: [] From 179ac072be8a8e116cb974629bfebfae940bd3e0 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 18 Mar 2022 12:13:01 +0000 Subject: [PATCH 05/14] non-issue so we don't need a changelog entry --- docs/changelog/85068.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/85068.yaml diff --git a/docs/changelog/85068.yaml b/docs/changelog/85068.yaml deleted file mode 100644 index 76f632d3a1bae..0000000000000 --- a/docs/changelog/85068.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 85068 -summary: Extra testing and some cleanups for filtering on field caps -area: Mapping -type: tech debt -issues: [] From 3fd44670fa226782e1073ae766892981eb1b6cf6 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 18 Mar 2022 12:17:34 +0000 Subject: [PATCH 06/14] Don't test new params against the unupgraded cluster, duh --- .../elasticsearch/upgrades/FieldCapsIT.java | 20 ------------------- 1 file changed, 20 deletions(-) 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 76638ed1ec2fd..3df6a70d28e31 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 @@ -258,13 +258,6 @@ public void testAllIndicesWithIndexFilter() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } - // Test field type filtering on old cluster - public void testFieldTypeFilteringOnOldOnly() throws Exception { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, "keyword", null); - assertThat(resp.getField("red_field").keySet(), contains("keyword")); - assertNull(resp.getField("yellow_field")); - } - // Test field type filtering on mixed cluster public void testAllIndicesWithFieldTypeFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); @@ -273,19 +266,6 @@ public void testAllIndicesWithFieldTypeFilter() throws Exception { assertNull(resp.getField("yellow_field")); } - // Test multifield exclusion on old cluster - public void testExclusionFilterOnOldOnly() throws Exception { - { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, null, null); - assertThat(resp.getField("multi_field.keyword").keySet(), contains("keyword")); - } - { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*"), List.of("*"), null, null, "-multifield"); - assertThat(resp.getField("multi_field").keySet(), contains("ip")); - assertNull(resp.getField("multi_field.keyword")); - } - } - // Test multifield exclusion on mixed cluster public void testAllIndicesWithExclusionFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); From e827a703609c834bb97adaa6adea9b58a64d80b9 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 21 Mar 2022 11:05:04 +0000 Subject: [PATCH 07/14] Use upgraded node to test against --- .../elasticsearch/upgrades/FieldCapsIT.java | 28 +++++++++++++++++-- .../test/rest/ESRestTestCase.java | 13 ++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) 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 3df6a70d28e31..be66ae7ed7c34 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; @@ -258,10 +263,26 @@ public void testAllIndicesWithIndexFilter() throws Exception { assertTrue(resp.getField("blue_field").get("keyword").isSearchable()); } + @SuppressWarnings("unchecked") + 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; + } + } + throw new IllegalStateException("Couldn't find node on version " + Version.CURRENT); + } + // Test field type filtering on mixed cluster public void testAllIndicesWithFieldTypeFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, "keyword", null); + 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")); } @@ -269,12 +290,13 @@ public void testAllIndicesWithFieldTypeFilter() throws Exception { // Test multifield exclusion on mixed cluster public void testAllIndicesWithExclusionFilter() throws Exception { assumeFalse("required mixed or upgraded cluster", CLUSTER_TYPE == ClusterType.OLD); + RestClient client = getUpgradedNodeClient(); { - FieldCapabilitiesResponse resp = fieldCaps(List.of("old_*", "new_*"), List.of("*"), null, null, null); + 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(List.of("old_*", "new_*"), List.of("*"), null, null, "-multifield"); + 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")); } 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 18aa50affc00c..27e70b9da2ae2 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 @@ -1968,6 +1968,17 @@ protected FieldCapabilitiesResponse fieldCaps( 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)); @@ -1985,7 +1996,7 @@ protected FieldCapabilitiesResponse fieldCaps( 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); From ac78390b2d44223b9d5d162e79bf4c13ed2fac77 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 21 Mar 2022 11:11:49 +0000 Subject: [PATCH 08/14] close the client --- .../src/test/java/org/elasticsearch/upgrades/FieldCapsIT.java | 3 +++ 1 file changed, 3 insertions(+) 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 be66ae7ed7c34..0d7a2d530b5f4 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 @@ -274,6 +274,7 @@ private RestClient getUpgradedNodeClient() throws IOException { if (version.equals(Version.CURRENT.toString())) { return client; } + client.close(); } throw new IllegalStateException("Couldn't find node on version " + Version.CURRENT); } @@ -285,6 +286,7 @@ public void testAllIndicesWithFieldTypeFilter() throws Exception { 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 @@ -300,5 +302,6 @@ public void testAllIndicesWithExclusionFilter() throws Exception { assertThat(resp.getField("multi_field").keySet(), contains("ip")); assertNull(resp.getField("multi_field.keyword")); } + client.close(); } } From e8ef8ec3b48323fb373c9596f67aa94b31df8cef Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 28 Mar 2022 12:32:14 +0100 Subject: [PATCH 09/14] deef --- docs/reference/search/field-caps.asciidoc | 23 ++++++++++--- .../elasticsearch/upgrades/FieldCapsIT.java | 7 ++++ .../test/field_caps/50_fieldtype_filter.yml | 15 +++++++++ .../action/fieldcaps/ResponseRewriter.java | 2 +- .../index/mapper/MapperService.java | 5 ++- .../index/mapper/MappingLookup.java | 4 +++ .../index/mapper/MapperServiceTests.java | 33 +++++++++++++++++++ 7 files changed, 83 insertions(+), 6 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 0d7a2d530b5f4..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 @@ -264,6 +264,7 @@ public void testAllIndicesWithIndexFilter() throws Exception { } @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(); @@ -280,6 +281,9 @@ private RestClient getUpgradedNodeClient() throws IOException { } // 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(); @@ -290,6 +294,9 @@ public void testAllIndicesWithFieldTypeFilter() throws Exception { } // 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(); 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..8f87e20f28fc5 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/ResponseRewriter.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java index b94016d729fb5..5ea9d1f460836 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/ResponseRewriter.java @@ -18,7 +18,7 @@ import java.util.stream.Collectors; /** - * Applies field type filters to responses that come from earlier versions of ES + * Applies field type filters to field caps responses that come from earlier versions of ES * that do not support filtering directly. */ final class ResponseRewriter { 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 b92d9bd5964ad..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,8 +505,11 @@ 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().isShadowed(field) == false && mappingLookup().isMultiField(field); + return mappingLookup().isMultiField(field); } public synchronized List reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException { 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 06cc37e17e9d2..c37ced7ea92e9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -339,6 +339,10 @@ public boolean isMultiField(String 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); return sourceParent != null && fieldMappers.containsKey(sourceParent); } 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")); + } + } From 7b38cb21127aa3093ba7d5d6a59415e2efc7b7a6 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 28 Mar 2022 12:52:11 +0100 Subject: [PATCH 10/14] sake --- .../rest-api-spec/test/field_caps/50_fieldtype_filter.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8f87e20f28fc5..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 @@ -198,8 +198,8 @@ setup: text.keyword: type: keyword - - is_true: fields.text\\.keyword - - is_true: fields.misc\\.keyword + - is_true: fields.text\\.keyword + - is_true: fields.misc\\.keyword --- "Field type filters": From 4e6ec4703a001f385ee7fccadf4c25748992f600 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 28 Mar 2022 15:23:55 +0100 Subject: [PATCH 11/14] unit test for SEC multifield check --- .../query/SearchExecutionContextTests.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) 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..6b4045a3cc365 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,31 @@ 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 +422,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 +447,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 +472,8 @@ private static MapperService createMapperService(IndexSettings indexSettings) { indexSettings.getMode().buildIdFieldMapper(() -> true) ) ); + when(mapperService.isMultiField(anyString())) + .then((Answer) invocation -> mappingLookup.isMultiField(invocation.getArgument(0))); return mapperService; } From e1d59216fd6888f1b429d8ab8fdc342c79541524 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 28 Mar 2022 15:36:18 +0100 Subject: [PATCH 12/14] precommit --- .../index/query/SearchExecutionContextTests.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) 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 6b4045a3cc365..1d19447977845 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java @@ -379,15 +379,11 @@ public void testSearchRequestRuntimeFieldsAndMultifieldDetection() { new MockFieldMapper.FakeFieldType("pig"), new MockFieldMapper.FakeFieldType("pig.subfield"), new MockFieldMapper.FakeFieldType("cat"), - new MockFieldMapper.FakeFieldType("cat.subfield")), + new MockFieldMapper.FakeFieldType("cat.subfield") + ), List.of(new TestRuntimeField("runtime", "long")) ); - SearchExecutionContext context = createSearchExecutionContext( - "uuid", - null, - mappingLookup, - runtimeMappings - ); + SearchExecutionContext context = createSearchExecutionContext("uuid", null, mappingLookup, runtimeMappings); assertTrue(context.isMultiField("pig.subfield")); assertFalse(context.isMultiField("cat.subfield")); assertTrue(mappingLookup.isMultiField("cat.subfield")); @@ -472,8 +468,9 @@ private static MapperService createMapperService(IndexSettings indexSettings, Ma indexSettings.getMode().buildIdFieldMapper(() -> true) ) ); - when(mapperService.isMultiField(anyString())) - .then((Answer) invocation -> mappingLookup.isMultiField(invocation.getArgument(0))); + when(mapperService.isMultiField(anyString())).then( + (Answer) invocation -> mappingLookup.isMultiField(invocation.getArgument(0)) + ); return mapperService; } From 1cae625bbdedd64c3affdd7ddfc01b99aaad1c28 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 28 Mar 2022 16:50:27 +0100 Subject: [PATCH 13/14] remove unused filter --- .../action/fieldcaps/FieldCapabilitiesFetcher.java | 3 --- 1 file changed, 3 deletions(-) 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 19692b4a253c8..983da5855b711 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java @@ -168,9 +168,6 @@ private static boolean checkIncludeParents(String[] filters) { if ("-parent".equals(filter)) { return false; } - if ("parent".equals(filter)) { - return true; - } } return true; } From 72594aafe2c3913ad0c0cae9143c29482a22d123 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 29 Mar 2022 10:31:05 +0100 Subject: [PATCH 14/14] request tests --- .../action/fieldcaps/FieldCapabilitiesRequestTests.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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..be4533ce5c73f 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);