Replace simpleMatchToFullName (#72674)

MappingLookup has a method simpleMatchToFieldName that attempts to return all field names that match a given pattern; if no patterns match, then it returns a single-valued collection containing just the pattern that was originally passed in. This is a fairly confusing semantic. This PR replaces simpleMatchToFullName with two new methods: * getMatchingFieldNames(), which returns a set of all mapped field names that match a pattern. Calling getFieldType() with a name returned by this method is guaranteed to return a non-null MappedFieldType * getMatchingFieldTypes, that returns a collection of all MappedFieldTypes in a mapping that match the passed-in pattern. This allows us to clean up several call-sites because we know that MappedFieldTypes returned from these calls will never be null. It also simplifies object field exists query construction.
elastic · May 13, 2021 · 3bd594e · 3bd594e
1 parent a5e39ce
commit 3bd594e
Show file tree

Hide file tree

Showing 26 changed files with 270 additions and 222 deletions.
diff --git a/...a/org/elasticsearch/benchmark/search/aggregations/AggConstructionContentionBenchmark.java b/...a/org/elasticsearch/benchmark/search/aggregations/AggConstructionContentionBenchmark.java
@@ -213,7 +213,7 @@ public MappedFieldType getFieldType(String path) {
         }
 
         @Override
-        public Collection<MappedFieldType> getFieldTypes() {
+        public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
             throw new UnsupportedOperationException();
         }
 

diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/Joiner.java b/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/Joiner.java
@@ -36,14 +36,14 @@ public final class Joiner {
      * Get the Joiner for this context, or {@code null} if none is configured
      */
     public static Joiner getJoiner(SearchExecutionContext context) {
-        return getJoiner(context.getFieldTypes());
+        return getJoiner(context.getAllFieldTypes());
     }
 
     /**
      * Get the Joiner for this context, or {@code null} if none is configured
      */
     public static Joiner getJoiner(AggregationContext context) {
-        return getJoiner(context.getFieldTypes());
+        return getJoiner(context.getMatchingFieldTypes("*"));
     }
 
     /**

diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java b/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java
@@ -292,7 +292,7 @@ public FieldMapper.Builder getMergeBuilder() {
 
     @Override
     protected void doValidate(MappingLookup mappers) {
-        List<String> joinFields = getJoinFieldTypes(mappers.fieldTypes()).stream()
+        List<String> joinFields = getJoinFieldTypes(mappers.getAllFieldTypes()).stream()
             .map(JoinFieldType::name)
             .collect(Collectors.toList());
         if (joinFields.size() > 1) {

diff --git a/...s/parent-join/src/test/java/org/elasticsearch/join/mapper/ParentJoinFieldMapperTests.java b/...s/parent-join/src/test/java/org/elasticsearch/join/mapper/ParentJoinFieldMapperTests.java
@@ -47,7 +47,7 @@ public void testSingleLevel() throws Exception {
         }));
         DocumentMapper docMapper = mapperService.documentMapper();
 
-        Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().fieldTypes());
+        Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
         assertNotNull(joiner);
         assertEquals("join_field", joiner.getJoinField());
 
@@ -233,7 +233,7 @@ public void testUpdateRelations() throws Exception {
             b.endObject();
         }));
 
-        Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().fieldTypes());
+        Joiner joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
         assertNotNull(joiner);
         assertEquals("join_field", joiner.getJoinField());
         assertTrue(joiner.childTypeExists("child2"));
@@ -259,7 +259,7 @@ public void testUpdateRelations() throws Exception {
             }
             b.endObject();
         }));
-        joiner = Joiner.getJoiner(mapperService.mappingLookup().fieldTypes());
+        joiner = Joiner.getJoiner(mapperService.mappingLookup().getAllFieldTypes());
         assertNotNull(joiner);
         assertEquals("join_field", joiner.getJoinField());
         assertTrue(joiner.childTypeExists("child2"));

diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java
@@ -26,7 +26,6 @@
 import org.elasticsearch.index.fielddata.ScriptDocValues;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.script.MockScriptPlugin;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptType;
@@ -63,11 +62,9 @@
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFailures;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
@@ -613,17 +610,6 @@ public void testSearchFieldsMetadata() throws Exception {
         assertThat(searchResponse.getHits().getAt(0).field("_routing").getValue().toString(), equalTo("1"));
     }
 
-    public void testSearchFieldsNonLeafField() throws Exception {
-        client().prepareIndex("my-index").setId("1")
-                .setSource(jsonBuilder().startObject().startObject("field1").field("field2", "value1").endObject().endObject())
-                .setRefreshPolicy(IMMEDIATE)
-                .get();
-
-        assertFailures(client().prepareSearch("my-index").addStoredField("field1"),
-                RestStatus.BAD_REQUEST,
-                containsString("field [field1] isn't a leaf field"));
-    }
-
     public void testGetFieldsComplexField() throws Exception {
         client().admin().indices().prepareCreate("my-index")
                 .setSettings(Settings.builder().put("index.refresh_interval", -1))

diff --git a/...c/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java b/...c/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java
@@ -109,45 +109,43 @@ private FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesInd
 
             Set<String> fieldNames = new HashSet<>();
             for (String pattern : request.fields()) {
-                fieldNames.addAll(searchExecutionContext.simpleMatchToIndexNames(pattern));
+                fieldNames.addAll(searchExecutionContext.getMatchingFieldNames(pattern));
             }
 
             Predicate<String> fieldPredicate = indicesService.getFieldFilter().apply(shardId.getIndexName());
             Map<String, IndexFieldCapabilities> responseMap = new HashMap<>();
             for (String field : fieldNames) {
                 MappedFieldType ft = searchExecutionContext.getFieldType(field);
-                if (ft != null) {
-                    boolean isMetadataField = searchExecutionContext.isMetadataField(field);
-                    if (isMetadataField || fieldPredicate.test(ft.name())) {
-                        IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(field,
-                            ft.familyTypeName(), isMetadataField, ft.isSearchable(), ft.isAggregatable(), ft.meta());
-                        responseMap.put(field, fieldCap);
-                    } else {
-                        continue;
-                    }
+                boolean isMetadataField = searchExecutionContext.isMetadataField(field);
+                if (isMetadataField || fieldPredicate.test(ft.name())) {
+                    IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(field,
+                        ft.familyTypeName(), isMetadataField, ft.isSearchable(), ft.isAggregatable(), ft.meta());
+                    responseMap.put(field, fieldCap);
+                } else {
+                    continue;
+                }
 
-                    // Check the ancestor of the field to find nested and object fields.
-                    // Runtime fields are excluded since they can override any path.
-                    //TODO find a way to do this that does not require an instanceof check
-                    if (ft instanceof RuntimeField == false) {
-                        int dotIndex = ft.name().lastIndexOf('.');
-                        while (dotIndex > -1) {
-                            String parentField = ft.name().substring(0, dotIndex);
-                            if (responseMap.containsKey(parentField)) {
-                                // we added this path on another field already
-                                break;
-                            }
-                            // checks if the parent field contains sub-fields
-                            if (searchExecutionContext.getFieldType(parentField) == null) {
-                                // no field type, it must be an object field
-                                ObjectMapper mapper = searchExecutionContext.getObjectMapper(parentField);
-                                String type = mapper.nested().isNested() ? "nested" : "object";
-                                IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(parentField, type,
-                                    false, false, false, Collections.emptyMap());
-                                responseMap.put(parentField, fieldCap);
-                            }
-                            dotIndex = parentField.lastIndexOf('.');
+                // Check the ancestor of the field to find nested and object fields.
+                // Runtime fields are excluded since they can override any path.
+                //TODO find a way to do this that does not require an instanceof check
+                if (ft instanceof RuntimeField == false) {
+                    int dotIndex = ft.name().lastIndexOf('.');
+                    while (dotIndex > -1) {
+                        String parentField = ft.name().substring(0, dotIndex);
+                        if (responseMap.containsKey(parentField)) {
+                            // we added this path on another field already
+                            break;
+                        }
+                        // checks if the parent field contains sub-fields
+                        if (searchExecutionContext.getFieldType(parentField) == null) {
+                            // no field type, it must be an object field
+                            ObjectMapper mapper = searchExecutionContext.getObjectMapper(parentField);
+                            String type = mapper.nested().isNested() ? "nested" : "object";
+                            IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(parentField, type,
+                                false, false, false, Collections.emptyMap());
+                            responseMap.put(parentField, fieldCap);
                         }
+                        dotIndex = parentField.lastIndexOf('.');
                     }
                 }
             }

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
@@ -10,10 +10,12 @@
 
 import org.elasticsearch.common.regex.Regex;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -140,27 +142,50 @@ private MappedFieldType getDynamicField(String field) {
     }
 
     /**
-     * Returns all the mapped field types.
+     * Returns all the mapped field types that match a pattern
+     *
+     * Note that if a field is aliased and both its actual name and its alias
+     * match the pattern, the returned collection will contain the field type
+     * twice.
      */
-    Collection<MappedFieldType> get() {
-        return fullNameToFieldType.values();
+    Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
+        if ("*".equals(pattern)) {
+            return fullNameToFieldType.values();
+        }
+        if (Regex.isSimpleMatchPattern(pattern) == false) {
+            // no wildcards
+            MappedFieldType ft = get(pattern);
+            return ft == null ? Collections.emptySet() : Collections.singleton(ft);
+        }
+        List<MappedFieldType> matchingFields = new ArrayList<>();
+        for (String field : fullNameToFieldType.keySet()) {
+            if (Regex.simpleMatch(pattern, field)) {
+                matchingFields.add(fullNameToFieldType.get(field));
+            }
+        }
+        return matchingFields;
     }
 
     /**
-     * Returns a list of the full names of a simple match regex like pattern against full name and index name.
+     * Returns a set of field names that match a regex-like pattern
+     *
+     * All field names in the returned set are guaranteed to resolve to a field
      */
-    Set<String> simpleMatchToFullName(String pattern) {
+    Set<String> getMatchingFieldNames(String pattern) {
+        if ("*".equals(pattern)) {
+            return fullNameToFieldType.keySet();
+        }
         if (Regex.isSimpleMatchPattern(pattern) == false) {
             // no wildcards
-            return Collections.singleton(pattern);
+            return get(pattern) == null ? Collections.emptySet() : Collections.singleton(pattern);
         }
-        Set<String> fields = new HashSet<>();
+        Set<String> matchingFields = new HashSet<>();
         for (String field : fullNameToFieldType.keySet()) {
             if (Regex.simpleMatch(pattern, field)) {
-                fields.add(field);
+                matchingFields.add(field);
             }
         }
-        return fields;
+        return matchingFields;
     }
 
     /**

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -373,7 +373,9 @@ public Iterable<MappedFieldType> getEagerGlobalOrdinalsFields() {
         if (this.mapper == null) {
             return Collections.emptySet();
         }
-        return this.mapper.mappers().fieldTypes().stream().filter(MappedFieldType::eagerGlobalOrdinals).collect(Collectors.toList());
+        return this.mapper.mappers().getAllFieldTypes().stream()
+            .filter(MappedFieldType::eagerGlobalOrdinals)
+            .collect(Collectors.toList());
     }
 
     /**

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java
@@ -201,13 +201,6 @@ public Iterable<Mapper> fieldMappers() {
         return fieldMappers.values();
     }
 
-    /**
-     * Returns the registered mapped field types.
-     */
-    public Collection<MappedFieldType> fieldTypes() {
-        return fieldTypeLookup.get();
-    }
-
     void checkLimits(IndexSettings settings) {
         checkFieldLimit(settings.getMappingTotalFieldsLimit());
         checkObjectDepthLimit(settings.getMappingDepthLimit());
@@ -296,8 +289,35 @@ private static String parentObject(String field) {
         return field.substring(0, lastDot);
     }
 
-    public Set<String> simpleMatchToFullName(String pattern) {
-        return fieldTypesLookup().simpleMatchToFullName(pattern);
+    /**
+     * Returns a set of field names that match a regex-like pattern
+     *
+     * All field names in the returned set are guaranteed to resolve to a field
+     *
+     * @param pattern the pattern to match field names against
+     */
+    public Set<String> getMatchingFieldNames(String pattern) {
+        return fieldTypeLookup.getMatchingFieldNames(pattern);
+    }
+
+    /**
+     * Returns all the mapped field types that match a pattern
+     *
+     * Note that if a field is aliased and both its actual name and its alias
+     * match the pattern, the returned collection will contain the field type
+     * twice.
+     *
+     * @param pattern the pattern to match field names against
+     */
+    public Collection<MappedFieldType> getMatchingFieldTypes(String pattern) {
+        return fieldTypeLookup.getMatchingFieldTypes(pattern);
+    }
+
+    /**
+     * @return all mapped field types
+     */
+    public Collection<MappedFieldType> getAllFieldTypes() {
+        return fieldTypeLookup.getMatchingFieldTypes("*");
     }
 
     /**