Resolve field aliases and multi-fields. (#55889)

jtibshirani · jtibshirani · commit 8d64f607f74e · 2020-05-27T21:03:45.000-07:00
This commit adds the capability to `FieldTypeLookup` to retrieve a field's paths in the _source. When retrieving a field's values, we consult these source paths to make sure we load the relevant values. This allows us to handle requests for field aliases and multi-fields. We also retrieve values that were copied into the field through copy_to. To me this is what users would expect out of the API, and it's consistent with what comes back from `docvalues_fields` and `stored_fields`. However it does add some complexity, and was not something flagged as important from any of the clients I spoke to about this API. I'm looking for feedback on this point. Relates to #55363.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
@@ -24,6 +24,7 @@
 import org.elasticsearch.common.regex.Regex;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -38,20 +39,32 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
 
     final CopyOnWriteHashMap<String, MappedFieldType> fullNameToFieldType;
     private final CopyOnWriteHashMap<String, String> aliasToConcreteName;
+
+    /**
+     * A map from field name to all fields whose content has been copied into it
+     * through copy_to. A field only be present in the map if some other field
+     * has listed it as a target of copy_to.
+     *
+     * For convenience, the set of copied fields includes the field itself.
+     */
+    private final CopyOnWriteHashMap<String, Set<String>> fieldToCopiedFields;
     private final DynamicKeyFieldTypeLookup dynamicKeyLookup;
 
 
     FieldTypeLookup() {
         fullNameToFieldType = new CopyOnWriteHashMap<>();
         aliasToConcreteName = new CopyOnWriteHashMap<>();
+        fieldToCopiedFields = new CopyOnWriteHashMap<>();
         dynamicKeyLookup = new DynamicKeyFieldTypeLookup();
     }
 
     private FieldTypeLookup(CopyOnWriteHashMap<String, MappedFieldType> fullNameToFieldType,
                             CopyOnWriteHashMap<String, String> aliasToConcreteName,
+                            CopyOnWriteHashMap<String, Set<String>> fieldToCopiedFields,
                             DynamicKeyFieldTypeLookup dynamicKeyLookup) {
         this.fullNameToFieldType = fullNameToFieldType;
         this.aliasToConcreteName = aliasToConcreteName;
+        this.fieldToCopiedFields = fieldToCopiedFields;
         this.dynamicKeyLookup = dynamicKeyLookup;
     }
 
@@ -66,6 +79,7 @@ public FieldTypeLookup copyAndAddAll(Collection<FieldMapper> fieldMappers,
 
         CopyOnWriteHashMap<String, MappedFieldType> fullName = this.fullNameToFieldType;
         CopyOnWriteHashMap<String, String> aliases = this.aliasToConcreteName;
+        CopyOnWriteHashMap<String, Set<String>> sourcePaths = this.fieldToCopiedFields;
         Map<String, DynamicKeyFieldMapper> dynamicKeyMappers = new HashMap<>();
 
         for (FieldMapper fieldMapper : fieldMappers) {
@@ -80,6 +94,17 @@ public FieldTypeLookup copyAndAddAll(Collection<FieldMapper> fieldMappers,
             if (fieldMapper instanceof DynamicKeyFieldMapper) {
                 dynamicKeyMappers.put(fieldName, (DynamicKeyFieldMapper) fieldMapper);
             }
+
+            for (String targetField : fieldMapper.copyTo().copyToFields()) {
+                Set<String> sourcePath = sourcePaths.get(targetField);
+                if (sourcePath == null) {
+                    sourcePaths = sourcePaths.copyAndPut(targetField, Set.of(targetField, fieldName));
+                } else if (sourcePath.contains(fieldName) == false) {
+                    Set<String> newSourcePath = new HashSet<>(sourcePath);
+                    newSourcePath.add(fieldName);
+                    sourcePaths = sourcePaths.copyAndPut(targetField, Collections.unmodifiableSet(newSourcePath));
+                }
+            }
         }
 
         for (FieldAliasMapper fieldAliasMapper : fieldAliasMappers) {
@@ -93,7 +118,7 @@ public FieldTypeLookup copyAndAddAll(Collection<FieldMapper> fieldMappers,
         }
 
         DynamicKeyFieldTypeLookup newDynamicKeyLookup = this.dynamicKeyLookup.copyAndAddAll(dynamicKeyMappers, aliases);
-        return new FieldTypeLookup(fullName, aliases, newDynamicKeyLookup);
+        return new FieldTypeLookup(fullName, aliases, sourcePaths, newDynamicKeyLookup);
     }
 
     /**
@@ -129,6 +154,31 @@ public Set<String> simpleMatchToFullName(String pattern) {
         return fields;
     }
 
+    /**
+     * Given a field, returns its possible paths in the _source.
+     *
+     * For most fields, the source path is the same as the field itself. However
+     * there are some exceptions:
+     *   - The 'source path' for a field alias is its target field.
+     *   - For a multi-field, the source path is the parent field.
+     *   - One field's content could have been copied to another through copy_to.
+     */
+    public Set<String> sourcePaths(String field) {
+        String resolvedField = aliasToConcreteName.getOrDefault(field, field);
+
+        int lastDotIndex = resolvedField.lastIndexOf('.');
+        if (lastDotIndex > 0) {
+            String parentField = resolvedField.substring(0, lastDotIndex);
+            if (fullNameToFieldType.containsKey(parentField)) {
+                resolvedField = parentField;
+            }
+        }
+
+        return fieldToCopiedFields.containsKey(resolvedField)
+            ? fieldToCopiedFields.get(resolvedField)
+            : Set.of(resolvedField);
+    }
+
     @Override
     public Iterator<MappedFieldType> iterator() {
         Iterator<MappedFieldType> concreteFieldTypes = fullNameToFieldType.values().iterator();
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -580,6 +580,14 @@ public Set<String> simpleMatchToFullName(String pattern) {
         return fieldTypes.simpleMatchToFullName(pattern);
     }
 
+    /**
+     * Given a field name, returns its possible paths in the _source. For example,
+     * the 'source path' for a multi-field is the path to its parent field.
+     */
+    public Set<String> sourcePath(String fullName) {
+        return fieldTypes.sourcePaths(fullName);
+    }
+
     /**
      * Returns all mapped field types.
      */
diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java
@@ -21,50 +21,72 @@
 
 import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
-import org.elasticsearch.index.mapper.DocumentMapper;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.search.lookup.SourceLookup;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+/**
+ * A helper class to {@link FetchFieldsPhase} that's initialized with a list of field patterns to fetch.
+ * Then given a specific document, it can retrieve the corresponding fields from the document's source.
+ */
 public class FieldValueRetriever {
-    private final Set<String> fields;
+    private final List<FieldContext> fieldContexts;
+    private final Set<String> sourcePaths;
 
     public static FieldValueRetriever create(MapperService mapperService,
                                              Collection<String> fieldPatterns) {
-        Set<String> fields = new HashSet<>();
-        DocumentMapper documentMapper = mapperService.documentMapper();
+        List<FieldContext> fields = new ArrayList<>();
+        Set<String> sourcePaths = new HashSet<>();
 
         for (String fieldPattern : fieldPatterns) {
-            if (documentMapper.objectMappers().containsKey(fieldPattern)) {
-                continue;
-            }
             Collection<String> concreteFields = mapperService.simpleMatchToFullName(fieldPattern);
-            fields.addAll(concreteFields);
+            for (String field : concreteFields) {
+                MappedFieldType fieldType = mapperService.fieldType(field);
+
+                if (fieldType != null) {
+                    Set<String> sourcePath = mapperService.sourcePath(field);
+                    fields.add(new FieldContext(field, sourcePath));
+                    sourcePaths.addAll(sourcePath);
+                }
+            }
         }
-        return new FieldValueRetriever(fields);
+
+        return new FieldValueRetriever(fields, sourcePaths);
     }
 
-    private FieldValueRetriever(Set<String> fields) {
-        this.fields = fields;
+    private FieldValueRetriever(List<FieldContext> fieldContexts, Set<String> sourcePaths) {
+        this.fieldContexts = fieldContexts;
+        this.sourcePaths = sourcePaths;
     }
 
     @SuppressWarnings("unchecked")
     public Map<String, DocumentField> retrieve(SourceLookup sourceLookup) {
         Map<String, DocumentField> result = new HashMap<>();
-        Map<String, Object> sourceValues = extractValues(sourceLookup, this.fields);
+        Map<String, Object> sourceValues = extractValues(sourceLookup, sourcePaths);
+
+        for (FieldContext fieldContext : fieldContexts) {
+            String field = fieldContext.fieldName;
+            Set<String> sourcePath = fieldContext.sourcePath;
 
-        for (Map.Entry<String, Object> entry : sourceValues.entrySet()) {
-            String field = entry.getKey();
-            Object value = entry.getValue();
-            List<Object> values = value instanceof List
-                ? (List<Object>) value
-                : List.of(value);
+            List<Object> values = new ArrayList<>();
+            for (String path : sourcePath) {
+                Object value = sourceValues.get(path);
+                if (value != null) {
+                    if (value instanceof List) {
+                        values.addAll((List<Object>) value);
+                    } else {
+                        values.add(value);
+                    }
+                }
+            }
             result.put(field, new DocumentField(field, values));
         }
         return result;
@@ -74,7 +96,7 @@ public Map<String, DocumentField> retrieve(SourceLookup sourceLookup) {
      * For each of the provided paths, return its value in the source. Note that in contrast with
      * {@link SourceLookup#extractRawValues}, array and object values can be returned.
      */
-    private static Map<String, Object> extractValues(SourceLookup sourceLookup, Collection<String> paths) {
+    private static Map<String, Object> extractValues(SourceLookup sourceLookup, Set<String> paths) {
         Map<String, Object> result = new HashMap<>(paths.size());
         for (String path : paths) {
             Object value = XContentMapValues.extractValue(path, sourceLookup);
@@ -84,4 +106,14 @@ private static Map<String, Object> extractValues(SourceLookup sourceLookup, Coll
         }
         return result;
     }
+
+    private static class FieldContext {
+        final String fieldName;
+        final Set<String> sourcePath;
+
+        FieldContext(String fieldName, Set<String> sourcePath) {
+            this.fieldName = fieldName;
+            this.sourcePath = sourcePath;
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java
@@ -30,6 +30,7 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 import static java.util.Collections.emptyList;
 
@@ -150,6 +151,65 @@ public void testSimpleMatchToFullName() {
         assertTrue(names.contains("barometer"));
     }
 
+    public void testSourcePathWithMultiFields() {
+        MappedFieldType ft = new MockFieldMapper.FakeFieldType();
+        Mapper.BuilderContext context = new Mapper.BuilderContext(
+            MockFieldMapper.dummySettings, new ContentPath());
+
+        MockFieldMapper field = new MockFieldMapper.Builder("field", ft, ft)
+            .addMultiField(new MockFieldMapper.Builder("field.subfield1", ft, ft))
+            .addMultiField(new MockFieldMapper.Builder("field.subfield2", ft, ft))
+            .build(context);
+
+        FieldTypeLookup lookup = new FieldTypeLookup();
+        lookup = lookup.copyAndAddAll(newList(field), emptyList());
+
+        assertEquals(Set.of("field"), lookup.sourcePaths("field"));
+        assertEquals(Set.of("field"), lookup.sourcePaths("field.subfield1"));
+        assertEquals(Set.of("field"), lookup.sourcePaths("field.subfield2"));
+    }
+
+    public void testSourcePathWithAliases() {
+        MappedFieldType ft = new MockFieldMapper.FakeFieldType();
+        Mapper.BuilderContext context = new Mapper.BuilderContext(
+            MockFieldMapper.dummySettings, new ContentPath());
+
+        MockFieldMapper field = new MockFieldMapper.Builder("field", ft, ft)
+            .addMultiField(new MockFieldMapper.Builder("field.subfield", ft, ft))
+            .build(context);
+
+        FieldAliasMapper alias1 = new FieldAliasMapper("alias1", "alias1", "field");
+        FieldAliasMapper alias2 = new FieldAliasMapper("alias2", "alias2", "field.subfield");
+
+        FieldTypeLookup lookup = new FieldTypeLookup();
+        lookup = lookup.copyAndAddAll(newList(field), newList(alias1, alias2));
+
+        assertEquals(Set.of("field"), lookup.sourcePaths("alias1"));
+        assertEquals(Set.of("field"), lookup.sourcePaths("alias2"));
+    }
+
+    public void testSourcePathsWithCopyTo() {
+        MappedFieldType ft = new MockFieldMapper.FakeFieldType();
+        Mapper.BuilderContext context = new Mapper.BuilderContext(
+            MockFieldMapper.dummySettings, new ContentPath());
+
+        MockFieldMapper field = new MockFieldMapper.Builder("field", ft, ft)
+            .addMultiField(new MockFieldMapper.Builder("field.subfield1", ft, ft))
+            .build(context);
+
+        MockFieldMapper otherField = new MockFieldMapper.Builder("other_field", ft, ft)
+            .copyTo(new FieldMapper.CopyTo.Builder()
+                .add("field")
+                .build())
+            .build(context);
+
+        FieldTypeLookup lookup = new FieldTypeLookup();
+        lookup = lookup.copyAndAddAll(newList(field, otherField), emptyList());
+
+        assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field"));
+        assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field.subfield1"));
+    }
+
     public void testIteratorImmutable() {
         MockFieldMapper f1 = new MockFieldMapper("foo");
         FieldTypeLookup lookup = new FieldTypeLookup();
diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java
diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MockFieldMapper.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MockFieldMapper.java
