WIP: Support unmapped fields in search 'fields' option #64651
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
|
|
||
| import org.apache.lucene.index.LeafReaderContext; | ||
| import org.elasticsearch.common.document.DocumentField; | ||
| import org.elasticsearch.common.xcontent.support.XContentMapValues; | ||
| import org.elasticsearch.index.mapper.MappedFieldType; | ||
| import org.elasticsearch.index.mapper.ValueFetcher; | ||
| import org.elasticsearch.index.query.QueryShardContext; | ||
|
|
@@ -30,10 +31,12 @@ | |
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.function.Function; | ||
|
|
||
| /** | ||
| * A helper class to {@link FetchFieldsPhase} that's initialized with a list of field patterns to fetch. | ||
|
|
@@ -42,32 +45,53 @@ | |
| public class FieldFetcher { | ||
| public static FieldFetcher create(QueryShardContext context, | ||
| SearchLookup searchLookup, | ||
| Collection<FieldAndFormat> fieldAndFormats, | ||
| boolean includeUnmapped) { | ||
|
|
||
| List<FieldContext> fieldContexts = new ArrayList<>(); | ||
| List<String> originalPattern = new ArrayList<>(); | ||
| List<String> excludeForUnmappedFetch = new ArrayList<>(); | ||
|
|
||
| for (FieldAndFormat fieldAndFormat : fieldAndFormats) { | ||
| String fieldPattern = fieldAndFormat.field; | ||
| String format = fieldAndFormat.format; | ||
|
|
||
| Collection<String> concreteFields = context.simpleMatchToIndexNames(fieldPattern); | ||
| originalPattern.add(fieldAndFormat.field); | ||
| for (String field : concreteFields) { | ||
| MappedFieldType ft = context.getFieldType(field); | ||
| if (ft == null || context.isMetadataField(field)) { | ||
| continue; | ||
| } | ||
| ValueFetcher valueFetcher = ft.valueFetcher(context, searchLookup, format); | ||
| excludeForUnmappedFetch.add(field); | ||
| fieldContexts.add(new FieldContext(field, valueFetcher)); | ||
| } | ||
| if (fieldPattern.charAt(fieldPattern.length() - 1) != '*') { | ||
| // not a prefix pattern, exclude potential sub-fields when fetching unmapped fields | ||
| excludeForUnmappedFetch.add(fieldPattern + ".*"); | ||
| } | ||
| } | ||
| Function<Map<String, ?>, Map<String, Object>> filter = XContentMapValues.filter( | ||
|
There was a problem hiding this comment. Re-using some source filtering logic seems to be a good fit. A couple ideas:
There was a problem hiding this comment. Thanks for the feedback, I'll need to better understand at how XContentMapValues#filter works to see if and how this could work. |
||
| originalPattern.toArray(new String[originalPattern.size()]), | ||
| excludeForUnmappedFetch.toArray(new String[excludeForUnmappedFetch.size()]) | ||
| ); | ||
|
|
||
| return new FieldFetcher(fieldContexts, includeUnmapped, filter); | ||
| } | ||
|
|
||
| private final List<FieldContext> fieldContexts; | ||
| private final boolean includeUnmapped; | ||
| private final Function<Map<String, ?>, Map<String, Object>> filter; | ||
|
|
||
| private FieldFetcher( | ||
| List<FieldContext> fieldContexts, | ||
| boolean includeUnmapped, | ||
| Function<Map<String, ?>, Map<String, Object>> filter | ||
| ) { | ||
| this.fieldContexts = fieldContexts; | ||
| this.includeUnmapped = includeUnmapped; | ||
| this.filter = filter; | ||
| } | ||
|
|
||
| public Map<String, DocumentField> fetch(SourceLookup sourceLookup, Set<String> ignoredFields) throws IOException { | ||
|
|
@@ -85,9 +109,37 @@ public Map<String, DocumentField> fetch(SourceLookup sourceLookup, Set<String> i | |
| documentFields.put(field, new DocumentField(field, parsedValues)); | ||
| } | ||
| } | ||
| if (includeUnmapped) { | ||
| Map<String, Object> unmappedFieldsToAdd = filter.apply(sourceLookup.loadSourceIfNeeded()); | ||
| collectLeafValues(unmappedFieldsToAdd, documentFields); | ||
| } | ||
| return documentFields; | ||
| } | ||
|
|
||
| static void collectLeafValues(Map<String, Object> map, Map<String, DocumentField> documentFields) { | ||
| collectAllPaths("", map, documentFields); | ||
| } | ||
|
|
||
| private static void collectAllPaths(String prefix, Map<String, Object> source, Map<String, DocumentField> documentFields) { | ||
| for (String key : source.keySet()) { | ||
| Object value = source.get(key); | ||
| String currentPath = prefix + key; | ||
| if (value instanceof Map) { | ||
| collectAllPaths(currentPath + ".", (Map<String, Object>) value, documentFields); | ||
| } else { | ||
| DocumentField f; | ||
|
There was a problem hiding this comment. I think this might miss lists of objects? Looking at There was a problem hiding this comment. I will take a look but though I have a test for this (essentially array values?) There was a problem hiding this comment. Ah, do you mean walking e.g. Lists of objects to their leaf values? e.g. How does the current "fields" lookup work for this, e.g. what does the path look like for the "f1" value= "foo.1.f1" ? Maybe I'm just confused here. There was a problem hiding this comment. Sorry for the confusion, I read the logic too quickly before. We won't miss anything, but the result structure could be surprising. For mapped fields, the 'fields' option always flattens arrays of objects. For example, given a document like a request for With this logic, for unmapped fields we'll return Perhaps this behavior is inconsistent, I think we'll still want to flatten arrays of objects (especially if we already flatten objects when they're not in array?) I'm definitely up for discussing this though, it's a question we noted on the original issue. There was a problem hiding this comment. I understand now and totally agree. I think I ran into this working on improvements on Friday anyway, should be part of my next update here. |
||
| if (value instanceof List) { | ||
| f = new DocumentField(currentPath, (List) value); | ||
| } else { | ||
| f = new DocumentField(currentPath, Collections.singletonList(value)); | ||
| } | ||
| if (f.getValue() != null) { | ||
| documentFields.put(currentPath, f); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public void setNextReader(LeafReaderContext readerContext) { | ||
| for (FieldContext field : fieldContexts) { | ||
| field.valueFetcher.setNextReader(readerContext); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to think about: if we end up making this configurable through a flag like
include_unmapped, we could introduce a top-level parameter as we do here, or instead support the flag alongside each field pattern (so it'd be part ofFieldAndFormat).