diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java index b9a68a555375..81124840ae79 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java @@ -34,7 +34,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -48,9 +48,10 @@ public static FieldFetcher create(QueryShardContext context, SearchLookup searchLookup, Collection fieldAndFormats) { - List fieldContexts = new ArrayList<>(); + // Using a LinkedHashMap so fields are returned in the order requested. + // We won't formally guarantee this but but its good for readability of the response + Map fieldContexts = new LinkedHashMap<>(); List unmappedFetchPattern = new ArrayList<>(); - Set mappedToExclude = new HashSet<>(); boolean includeUnmapped = false; for (FieldAndFormat fieldAndFormat : fieldAndFormats) { @@ -68,8 +69,7 @@ public static FieldFetcher create(QueryShardContext context, continue; } ValueFetcher valueFetcher = ft.valueFetcher(context, format); - mappedToExclude.add(field); - fieldContexts.add(new FieldContext(field, valueFetcher)); + fieldContexts.put(field, new FieldContext(field, valueFetcher)); } } CharacterRunAutomaton unmappedFetchAutomaton = new CharacterRunAutomaton(Automata.makeEmpty()); @@ -78,29 +78,26 @@ public static FieldFetcher create(QueryShardContext context, Regex.simpleMatchToAutomaton(unmappedFetchPattern.toArray(new String[unmappedFetchPattern.size()])) ); } - return new FieldFetcher(fieldContexts, unmappedFetchAutomaton, mappedToExclude, includeUnmapped); + return new FieldFetcher(fieldContexts, unmappedFetchAutomaton, includeUnmapped); } - private final List fieldContexts; + private final Map fieldContexts; private final CharacterRunAutomaton unmappedFetchAutomaton; - private final Set mappedToExclude; private final boolean includeUnmapped; private FieldFetcher( - List fieldContexts, + Map fieldContexts, CharacterRunAutomaton unmappedFetchAutomaton, - Set mappedToExclude, boolean includeUnmapped ) { this.fieldContexts = fieldContexts; this.unmappedFetchAutomaton = unmappedFetchAutomaton; - this.mappedToExclude = mappedToExclude; this.includeUnmapped = includeUnmapped; } public Map fetch(SourceLookup sourceLookup, Set ignoredFields) throws IOException { Map documentFields = new HashMap<>(); - for (FieldContext context : fieldContexts) { + for (FieldContext context : fieldContexts.values()) { String field = context.fieldName; if (ignoredFields.contains(field)) { continue; @@ -141,7 +138,7 @@ private void collectUnmapped(Map documentFields, Map) value, currentPath, currentState); } else { // we have a leaf value - if (this.unmappedFetchAutomaton.isAccept(currentState) && this.mappedToExclude.contains(currentPath) == false) { + if (this.unmappedFetchAutomaton.isAccept(currentState) && this.fieldContexts.containsKey(currentPath) == false) { if (value != null) { DocumentField currentEntry = documentFields.get(currentPath); if (currentEntry == null) { @@ -170,7 +167,7 @@ private void collectUnmappedList(Map documentFields, Iter } else if (value instanceof List) { // weird case, but can happen for objects with "enabled" : "false" collectUnmappedList(documentFields, (List) value, parentPath, lastState); - } else if (this.unmappedFetchAutomaton.isAccept(lastState) && this.mappedToExclude.contains(parentPath) == false) { + } else if (this.unmappedFetchAutomaton.isAccept(lastState) && this.fieldContexts.containsKey(parentPath) == false) { list.add(value); } } @@ -192,7 +189,7 @@ private static int step(CharacterRunAutomaton automaton, String key, int state) } public void setNextReader(LeafReaderContext readerContext) { - for (FieldContext field : fieldContexts) { + for (FieldContext field : fieldContexts.values()) { field.valueFetcher.setNextReader(readerContext); } } diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java index 0178d8cf38e5..1f92c773021f 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java @@ -36,6 +36,7 @@ import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -659,6 +660,32 @@ public void testUnmappedFieldsWildcard() throws IOException { assertThat(fields.get("unmapped_object.b").getValue(), equalTo("bar")); } + public void testLastFormatWins() throws IOException { + MapperService mapperService = createMapperService(); + + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .startArray("date_field") + .value("2011-11-11T11:11:11") + .value("2012-12-12T12:12:12") + .endArray() + .endObject(); + + List ff = new ArrayList<>(); + ff.add(new FieldAndFormat("date_field", "year", false)); + Map fields = fetchFields(mapperService, source, ff, null); + assertThat(fields.size(), equalTo(1)); + assertThat(fields.get("date_field").getValues().size(), equalTo(2)); + assertThat(fields.get("date_field").getValues().get(0), equalTo("2011")); + assertThat(fields.get("date_field").getValues().get(1), equalTo("2012")); + + ff.add(new FieldAndFormat("date_field", "hour", false)); + fields = fetchFields(mapperService, source, ff, null); + assertThat(fields.size(), equalTo(1)); + assertThat(fields.get("date_field").getValues().size(), equalTo(2)); + assertThat(fields.get("date_field").getValues().get(0), equalTo("11")); + assertThat(fields.get("date_field").getValues().get(1), equalTo("12")); + } + private List fieldAndFormatList(String name, String format, boolean includeUnmapped) { return Collections.singletonList(new FieldAndFormat(name, format, includeUnmapped)); }