Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In field retrieval API, handle non-standard source paths. #55889

Merged
merged 1 commit into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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) {
Expand All @@ -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));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can set up copy_to the same field multiple times. I imagine this won't perfectly emulate that. But that is almost certainly ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I also think it's okay not to emulate that.

}
}

for (FieldAliasMapper fieldAliasMapper : fieldAliasMappers) {
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should return something that has an extractValue(SourceLookup) method on it. I'm just thinking of types constant_keyword and the formatting stuff we talked about over video. Not that now is the time to build that, but maybe this is the place to handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will keep this in mind in the next PR where we'll handle field-specific behavior like formatting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return fieldTypes.sourcePaths(fullName);
}

/**
* Returns all mapped field types.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
Loading