Browse Source

Handle ignored fields directly in SourceValueFetcher (#68738)

Currently, the value fetcher framework handles ignored fields by reading
the stored values of the _ignored metadata field, and passing these through
on calls to fetchValues(). However, this means that if a document has multiple
values indexed for a field, and one malformed value, then the fields API will
ignore everything, including the valid values, and return an empty list for this
document.

If a document source contains a malformed value, then it must have been
ignored at index time. Therefore, we can safely assume that if we get an
exception parsing values from source at fetch time, they were also ignored
at index time and they can be skipped. This commit moves this exception
handling directly into SourceValueFetcher and ArraySourceValueFetcher,
removing the need to inspect the _ignored metadata and fixing the case
of mixed valid and invalid values.
Alan Woodward 4 years ago
parent
commit
8fba6e4a6d
19 changed files with 139 additions and 104 deletions
  1. 1 1
      modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java
  2. 39 4
      rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml
  3. 8 7
      server/src/main/java/org/elasticsearch/index/mapper/ArraySourceValueFetcher.java
  4. 1 2
      server/src/main/java/org/elasticsearch/index/mapper/DocValueFetcher.java
  5. 2 3
      server/src/main/java/org/elasticsearch/index/mapper/NestedValueFetcher.java
  6. 11 10
      server/src/main/java/org/elasticsearch/index/mapper/SourceValueFetcher.java
  7. 1 4
      server/src/main/java/org/elasticsearch/index/mapper/ValueFetcher.java
  8. 1 2
      server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java
  9. 1 18
      server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java
  10. 2 3
      server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java
  11. 1 1
      server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java
  12. 4 0
      server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java
  13. 1 0
      server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
  14. 46 40
      server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java
  15. 12 1
      test/framework/src/main/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java
  16. 1 1
      test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
  17. 2 2
      x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java
  18. 4 4
      x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldTypeTests.java
  19. 1 1
      x-pack/plugin/mapper-flattened/src/main/java/org/elasticsearch/xpack/flattened/mapper/FlattenedFieldMapper.java

+ 1 - 1
modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java

@@ -83,7 +83,7 @@ public class TokenCountFieldMapper extends FieldMapper {
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
             if (hasDocValues() == false) {
-                return (lookup, ignoredFields) -> List.of();
+                return lookup -> List.of();
             }
             return new DocValueFetcher(docValueFormat(format, null), context.getForField(this));
         }

+ 39 - 4
rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml

@@ -335,7 +335,7 @@ Test unmapped field:
               - f1
               - { "field" : "f4", "include_unmapped" : true }
   -  match:
-        hits.hits.0.fields.f1: 
+        hits.hits.0.fields.f1:
         - some text
   -  match:
         hits.hits.0.fields.f4:
@@ -347,7 +347,7 @@ Test unmapped field:
               fields:
               - { "field" : "f*", "include_unmapped" : true }
   -  match:
-        hits.hits.0.fields.f1: 
+        hits.hits.0.fields.f1:
         - some text
   -  match:
         hits.hits.0.fields.f2\.a:
@@ -381,7 +381,7 @@ Test unmapped fields inside disabled objects:
            id: 1
            refresh: true
            body:
-              f1: 
+              f1:
                  - some text
                  - a: b
                  -
@@ -809,9 +809,44 @@ Test nested field with sibling field resolving to DocValueFetcher:
         hits.hits.0.fields.owner:
           - "Anna Ott"
   - match:
-        hits.hits.0.fields.owner\.length: 
+        hits.hits.0.fields.owner\.length:
         - 2
   - match:
         hits.hits.0.fields.products.0: { "manufacturer" : ["Supersoft"]}
   - match:
         hits.hits.0.fields.products.1: { "manufacturer" : ["HyperSmart"]}
+
+---
+"Test ignores malformed values while returning valid ones":
+  - skip:
+      version: ' - 8.0.0'
+      reason:  'Added in 8.0 - change on backport'
+  - do:
+      indices.create:
+        index: test
+        body:
+          mappings:
+            properties:
+              number:
+                type: long
+                ignore_malformed: true
+  - do:
+      index:
+        index: test
+        id: 1
+        refresh: true
+        body:
+          number: [ 1, 2, "3", "four", 5, 6 ]
+
+  - do:
+      search:
+        index: test
+        body:
+          fields: [ "*" ]
+
+  - length: { hits.hits.0.fields.number : 5 }
+  - match: { hits.hits.0.fields.number.0 : 1 }
+  - match: { hits.hits.0.fields.number.1 : 2 }
+  - match: { hits.hits.0.fields.number.2 : 3 }
+  - match: { hits.hits.0.fields.number.3 : 5 }
+  - match: { hits.hits.0.fields.number.4 : 6 }

+ 8 - 7
server/src/main/java/org/elasticsearch/index/mapper/ArraySourceValueFetcher.java

@@ -27,7 +27,6 @@ import java.util.Set;
 public abstract class ArraySourceValueFetcher implements ValueFetcher {
     private final Set<String> sourcePaths;
     private final @Nullable Object nullValue;
-    private final String fieldName;
 
     public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context) {
         this(fieldName, context, null);
@@ -41,21 +40,23 @@ public abstract class ArraySourceValueFetcher implements ValueFetcher {
     public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context, Object nullValue) {
         this.sourcePaths = context.sourcePath(fieldName);
         this.nullValue = nullValue;
-        this.fieldName = fieldName;
     }
 
     @Override
-    public List<Object> fetchValues(SourceLookup lookup, Set<String> ignoredFields) {
+    public List<Object> fetchValues(SourceLookup lookup) {
         List<Object> values = new ArrayList<>();
-        if (ignoredFields.contains(fieldName)) {
-            return values;
-        }
         for (String path : sourcePaths) {
             Object sourceValue = lookup.extractValue(path, nullValue);
             if (sourceValue == null) {
                 return List.of();
             }
-            values.addAll((List<?>) parseSourceValue(sourceValue));
+            try {
+                values.addAll((List<?>) parseSourceValue(sourceValue));
+            } catch (Exception e) {
+                // if parsing fails here then it would have failed at index time
+                // as well, meaning that we must be ignoring malformed values.
+                // So ignore it here too.
+            }
         }
         return values;
     }

+ 1 - 2
server/src/main/java/org/elasticsearch/index/mapper/DocValueFetcher.java

@@ -17,7 +17,6 @@ import org.elasticsearch.search.lookup.SourceLookup;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 
 import static java.util.Collections.emptyList;
 
@@ -40,7 +39,7 @@ public final class DocValueFetcher implements ValueFetcher {
     }
 
     @Override
-    public List<Object> fetchValues(SourceLookup lookup, Set<String> ignoredFields) throws IOException {
+    public List<Object> fetchValues(SourceLookup lookup) throws IOException {
         if (false == formattedDocValues.advanceExact(lookup.docId())) {
             return emptyList();
         }

+ 2 - 3
server/src/main/java/org/elasticsearch/index/mapper/NestedValueFetcher.java

@@ -19,7 +19,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 public class NestedValueFetcher implements ValueFetcher {
 
@@ -39,7 +38,7 @@ public class NestedValueFetcher implements ValueFetcher {
     }
 
     @Override
-    public List<Object> fetchValues(SourceLookup lookup, Set<String> ignoreFields) throws IOException {
+    public List<Object> fetchValues(SourceLookup lookup) throws IOException {
         List<Object> nestedEntriesToReturn = new ArrayList<>();
         Map<String, Object> filteredSource = new HashMap<>();
         Map<String, Object> stub = createSourceMapStub(filteredSource);
@@ -53,7 +52,7 @@ public class NestedValueFetcher implements ValueFetcher {
             SourceLookup nestedSourceLookup = new SourceLookup();
             nestedSourceLookup.setSource(filteredSource);
 
-            Map<String, DocumentField> fetchResult = nestedFieldFetcher.fetch(nestedSourceLookup, ignoreFields);
+            Map<String, DocumentField> fetchResult = nestedFieldFetcher.fetch(nestedSourceLookup);
 
             Map<String, Object> nestedEntry = new HashMap<>();
             for (DocumentField field : fetchResult.values()) {

+ 11 - 10
server/src/main/java/org/elasticsearch/index/mapper/SourceValueFetcher.java

@@ -28,29 +28,23 @@ import java.util.Set;
 public abstract class SourceValueFetcher implements ValueFetcher {
     private final Set<String> sourcePaths;
     private final @Nullable Object nullValue;
-    private final String fieldName;
 
     public SourceValueFetcher(String fieldName, SearchExecutionContext context) {
         this(fieldName, context, null);
     }
 
     /**
-     * @param fieldName The name of the field.
      * @param context The query shard context
      * @param nullValue A optional substitute value if the _source value is 'null'.
      */
     public SourceValueFetcher(String fieldName, SearchExecutionContext context, Object nullValue) {
         this.sourcePaths = context.sourcePath(fieldName);
         this.nullValue = nullValue;
-        this.fieldName = fieldName;
     }
 
     @Override
-    public List<Object> fetchValues(SourceLookup lookup, Set<String> ignoredFields) {
+    public List<Object> fetchValues(SourceLookup lookup) {
         List<Object> values = new ArrayList<>();
-        if (ignoredFields.contains(fieldName)) {
-            return values;
-        }
         for (String path : sourcePaths) {
             Object sourceValue = lookup.extractValue(path, nullValue);
             if (sourceValue == null) {
@@ -66,9 +60,16 @@ public abstract class SourceValueFetcher implements ValueFetcher {
                 if (value instanceof List) {
                     queue.addAll((List<?>) value);
                 } else {
-                    Object parsedValue = parseSourceValue(value);
-                    if (parsedValue != null) {
-                        values.add(parsedValue);
+                    try {
+                        Object parsedValue = parseSourceValue(value);
+                        if (parsedValue != null) {
+                            values.add(parsedValue);
+                        }
+                    } catch (Exception e) {
+                        // if we get a parsing exception here, that means that the
+                        // value in _source would have also caused a parsing
+                        // exception at index time and the value ignored.
+                        // so ignore it here as well
                     }
                 }
             }

+ 1 - 4
server/src/main/java/org/elasticsearch/index/mapper/ValueFetcher.java

@@ -9,13 +9,11 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.index.LeafReaderContext;
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.search.fetch.subphase.FetchFieldsPhase;
 import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
 /**
  * A helper class for fetching field values during the {@link FetchFieldsPhase}. Each {@link MappedFieldType}
@@ -33,10 +31,9 @@ public interface ValueFetcher {
     * should not be relied on.
     *
     * @param lookup a lookup structure over the document's source.
-    * @param ignoredFields the fields in _ignored that have been ignored for this document because they were malformed
     * @return a list a standardized field values.
     */
-    List<Object> fetchValues(SourceLookup lookup, @Nullable Set<String> ignoredFields) throws IOException;
+    List<Object> fetchValues(SourceLookup lookup) throws IOException;
 
     /**
      * Update the leaf reader used to fetch values.

+ 1 - 2
server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java

@@ -18,7 +18,6 @@ import org.elasticsearch.search.fetch.FetchSubPhaseProcessor;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -71,7 +70,7 @@ public final class FetchDocValuesPhase implements FetchSubPhase {
                         // docValues fields will still be document fields, and put under "fields" section of a hit.
                         hit.hit().setDocumentField(f.field, hitField);
                     }
-                    hitField.getValues().addAll(f.fetcher.fetchValues(hit.sourceLookup(), Collections.emptySet()));
+                    hitField.getValues().addAll(f.fetcher.fetchValues(hit.sourceLookup()));
                 }
             }
         };

+ 1 - 18
server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java

@@ -10,7 +10,6 @@ package org.elasticsearch.search.fetch.subphase;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.elasticsearch.common.document.DocumentField;
-import org.elasticsearch.index.mapper.IgnoredFieldMapper;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.fetch.FetchContext;
 import org.elasticsearch.search.fetch.FetchSubPhase;
@@ -18,9 +17,7 @@ import org.elasticsearch.search.fetch.FetchSubPhaseProcessor;
 import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * A fetch sub-phase for high-level field retrieval. Given a list of fields, it
@@ -53,25 +50,11 @@ public final class FetchFieldsPhase implements FetchSubPhase {
                 SearchHit hit = hitContext.hit();
                 SourceLookup sourceLookup = hitContext.sourceLookup();
 
-                Set<String> ignoredFields = getIgnoredFields(hit);
-                Map<String, DocumentField> documentFields = fieldFetcher.fetch(sourceLookup, ignoredFields);
+                Map<String, DocumentField> documentFields = fieldFetcher.fetch(sourceLookup);
                 for (Map.Entry<String, DocumentField> entry : documentFields.entrySet()) {
                     hit.setDocumentField(entry.getKey(), entry.getValue());
                 }
             }
         };
     }
-
-    private Set<String> getIgnoredFields(SearchHit hit) {
-        DocumentField field = hit.field(IgnoredFieldMapper.NAME);
-        if (field == null) {
-            return Set.of();
-        }
-
-        Set<String> ignoredFields = new HashSet<>();
-        for (Object value : field.getValues()) {
-            ignoredFields.add((String) value);
-        }
-        return ignoredFields;
-    }
 }

+ 2 - 3
server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java

@@ -134,14 +134,13 @@ public class FieldFetcher {
         this.unmappedFieldsFetchAutomaton = unmappedFieldsFetchAutomaton;
     }
 
-    public Map<String, DocumentField> fetch(SourceLookup sourceLookup, Set<String> ignoredFields) throws IOException {
-        assert ignoredFields != null;
+    public Map<String, DocumentField> fetch(SourceLookup sourceLookup) throws IOException {
         Map<String, DocumentField> documentFields = new HashMap<>();
         for (FieldContext context : fieldContexts.values()) {
             String field = context.fieldName;
 
             ValueFetcher valueFetcher = context.valueFetcher;
-            List<Object> parsedValues = valueFetcher.fetchValues(sourceLookup, ignoredFields);
+            List<Object> parsedValues = valueFetcher.fetchValues(sourceLookup);
             if (parsedValues.isEmpty() == false) {
                 documentFields.put(field, new DocumentField(field, parsedValues));
             }

+ 1 - 1
server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightUtils.java

@@ -48,7 +48,7 @@ public final class HighlightUtils {
         }
         ValueFetcher fetcher = fieldType.valueFetcher(searchContext, null);
         fetcher.setNextReader(hitContext.readerContext());
-        return fetcher.fetchValues(hitContext.sourceLookup(), Collections.emptySet());
+        return fetcher.fetchValues(hitContext.sourceLookup());
     }
 
     public static class Encoders {

+ 4 - 0
server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java

@@ -41,5 +41,9 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase {
         sourceValue = "POINT (42.0 27.1)";
         assertEquals(List.of(jsonPoint), fetchSourceValue(mapper, sourceValue, null));
         assertEquals(List.of(wktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
+
+        // Test a malformed value
+        sourceValue = "malformed";
+        assertEquals(List.of(), fetchSourceValue(mapper, sourceValue, null));
     }
 }

+ 1 - 0
server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java

@@ -639,6 +639,7 @@ public class NumberFieldTypeTests extends FieldTypeTestCase {
             .fieldType();
         assertEquals(List.of(3), fetchSourceValue(mapper, 3.14));
         assertEquals(List.of(42), fetchSourceValue(mapper, "42.9"));
+        assertEquals(List.of(3, 42), fetchSourceValues(mapper, 3.14, "foo", "42.9"));
 
         MappedFieldType nullValueMapper = new NumberFieldMapper.Builder("field", NumberType.FLOAT, false, true)
             .nullValue(2.71f)

+ 46 - 40
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java

@@ -10,7 +10,6 @@ package org.elasticsearch.search.fetch.subphase;
 
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.document.DocumentField;
@@ -29,7 +28,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import static java.util.Collections.emptyMap;
 import static org.elasticsearch.common.xcontent.ObjectPath.eval;
@@ -51,7 +49,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
         List<FieldAndFormat> fieldAndFormats = List.of(
             new FieldAndFormat("field", null),
             new FieldAndFormat("object.field", null));
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormats, null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormats);
         assertThat(fields.size(), equalTo(2));
 
         DocumentField field = fields.get("field");
@@ -279,8 +277,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
 
         Map<String, DocumentField> fields = fetchFields(mapperService, source, List.of(
             new FieldAndFormat("field", null),
-            new FieldAndFormat("date_field", "yyyy/MM/dd")),
-            null);
+            new FieldAndFormat("date_field", "yyyy/MM/dd")));
         assertThat(fields.size(), equalTo(2));
 
         DocumentField field = fields.get("field");
@@ -290,6 +287,16 @@ public class FieldFetcherTests extends MapperServiceTestCase {
         assertNotNull(dateField);
         assertThat(dateField.getValues().size(), equalTo(1));
         assertThat(dateField.getValue(), equalTo("1990/12/29"));
+
+        // check that badly formed dates in source are just ignored when fetching
+        source = XContentFactory.jsonBuilder().startObject()
+            .field("field", "value")
+            .array("date_field", "1990-12-29T00:00:00.000Z", "baddate", "1991-12-29T00:00:00.000Z")
+            .endObject();
+        DocumentField dates
+            = fetchFields(mapperService, source, List.of(new FieldAndFormat("date_field", "yyyy/MM/dd"))).get("date_field");
+        assertThat(dates.getValues().size(), equalTo(2));
+        assertThat(dates, containsInAnyOrder(equalTo("1990/12/29"), equalTo("1991/12/29")));
     }
 
     public void testIgnoreAbove() throws IOException {
@@ -451,26 +458,26 @@ public class FieldFetcherTests extends MapperServiceTestCase {
                 .field("object.b", "bar")
             .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_f*", null, true), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_f*", null, true));
         assertThat(fields.size(), equalTo(3));
         assertThat(fields.keySet(), containsInAnyOrder("unmapped_f1", "unmapped_f2", "unmapped_f3"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("un*1", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("un*1", null, true));
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.keySet(), containsInAnyOrder("unmapped_f1"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("*thing*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("*thing*", null, true));
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.keySet(), containsInAnyOrder("something_else"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("null*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("null*", null, true));
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("object.a", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("object.a", null, true));
         assertThat(fields.size(), equalTo(1));
         assertEquals("foo", fields.get("object.a").getValues().get(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("object.b", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("object.b", null, true));
         assertThat(fields.size(), equalTo(1));
         assertEquals("bar", fields.get("object.b").getValues().get(0));
     }
@@ -480,7 +487,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
 
         XContentBuilder source = XContentFactory.jsonBuilder().startObject().array("unmapped_field", "foo", "bar").endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field", null, true), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field", null, true));
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.keySet(), containsInAnyOrder("unmapped_field"));
         DocumentField field = fields.get("unmapped_field");
@@ -503,10 +510,10 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .endArray()
             .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field", null, true), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field", null, true));
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f*", null, true));
         assertThat(fields.size(), equalTo(2));
         assertThat(fields.get("unmapped_field.f1").getValue(), equalTo("a"));
         assertThat(fields.get("unmapped_field.f2").getValue(), equalTo("b"));
@@ -526,19 +533,19 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .endArray()
             .endObject();
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f1", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f1", null, true));
         assertThat(fields.size(), equalTo(1));
         DocumentField field = fields.get("unmapped_field.f1");
         assertThat(field.getValues().size(), equalTo(2));
         assertThat(field.getValues(), hasItems("a", "b"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f2", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f2", null, true));
         assertThat(fields.size(), equalTo(1));
         field = fields.get("unmapped_field.f2");
         assertThat(field.getValues().size(), equalTo(4));
         assertThat(field.getValues(), hasItems(1, 2, 3, 4));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f3", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_field.f3", null, true));
         assertThat(fields.size(), equalTo(1));
         field = fields.get("unmapped_field.f3");
         assertThat(field.getValues().size(), equalTo(3));
@@ -590,7 +597,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .endArray()
           .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false));
         assertEquals(2, fields.size());
         assertThat(fields.keySet(), containsInAnyOrder("f1", "obj"));
         assertEquals("value1", fields.get("f1").getValue());
@@ -608,7 +615,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
         assertEquals("value3b", eval("f3.0", obj1));
         assertEquals("value4b", eval("inner_nested.0.f4.0", obj1));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("obj*", null, false), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("obj*", null, false));
         assertEquals(1, fields.size());
         assertThat(fields.keySet(), containsInAnyOrder("obj"));
         obj = fields.get("obj").getValues();
@@ -625,7 +632,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
         assertEquals("value3b", eval("f3.0", obj1));
         assertEquals("value4b", eval("inner_nested.0.f4.0", obj1));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("obj*", null, false), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("obj*", null, false));
         assertEquals(1, fields.size());
         assertThat(fields.keySet(), containsInAnyOrder("obj"));
         obj = fields.get("obj").getValues();
@@ -660,13 +667,13 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .field("obj.innerObj.f4", "unmapped_value_f4")
             .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false));
 
         // without unmapped fields this should only return "obj.f1"
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.keySet(), containsInAnyOrder("obj.f1"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, true));
         assertThat(fields.size(), equalTo(4));
         assertThat(fields.keySet(), containsInAnyOrder("obj.f1", "obj.f2", "obj.innerObj.f3", "obj.innerObj.f4"));
     }
@@ -697,11 +704,11 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .endArray()
         .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, false));
         // without unmapped fields this should return nothing
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("*", null, true));
         assertThat(fields.size(), equalTo(2));
         assertThat(fields.keySet(), containsInAnyOrder("obj", "obj.a"));
 
@@ -736,15 +743,15 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .field("f1", "malformed")
             .endObject();
 
-        // this should not return a field bc. f1 is in the ignored fields
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("*", null, true)), Set.of("f1"));
+        // this should not return a field bc. f1 is malformed
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("*", null, true)));
         assertThat(fields.size(), equalTo(0));
 
         // and this should neither
-        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("*", null, true)), Set.of("f1"));
+        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("*", null, true)));
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("f1", null, true)), Set.of("f1"));
+        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("f1", null, true)));
         assertThat(fields.size(), equalTo(0));
 
         // check this also does not overwrite with arrays
@@ -752,7 +759,7 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .array("f1", "malformed")
             .endObject();
 
-        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("f1", null, true)), Set.of("f1"));
+        fields = fetchFields(mapperService, source, List.of(new FieldAndFormat("f1", null, true)));
         assertThat(fields.size(), equalTo(0));
     }
 
@@ -766,24 +773,24 @@ public class FieldFetcherTests extends MapperServiceTestCase {
             .endObject()
             .endObject();
 
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object", null, true), null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object", null, true));
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmap*object", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmap*object", null, true));
         assertThat(fields.size(), equalTo(0));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.*", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.*", null, true));
         assertThat(fields.size(), equalTo(2));
         assertThat(fields.keySet(), containsInAnyOrder("unmapped_object.a", "unmapped_object.b"));
 
         assertThat(fields.get("unmapped_object.a").getValue(), equalTo("foo"));
         assertThat(fields.get("unmapped_object.b").getValue(), equalTo("bar"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.a", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.a", null, true));
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.get("unmapped_object.a").getValue(), equalTo("foo"));
 
-        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.b", null, true), null);
+        fields = fetchFields(mapperService, source, fieldAndFormatList("unmapped_object.b", null, true));
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.get("unmapped_object.b").getValue(), equalTo("bar"));
     }
@@ -800,14 +807,14 @@ public class FieldFetcherTests extends MapperServiceTestCase {
 
         List<FieldAndFormat> ff = new ArrayList<>();
         ff.add(new FieldAndFormat("date_field", "year", false));
-        Map<String, DocumentField> fields = fetchFields(mapperService, source, ff, null);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, ff);
         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);
+        fields = fetchFields(mapperService, source, ff);
         assertThat(fields.size(), equalTo(1));
         assertThat(fields.get("date_field").getValues().size(), equalTo(2));
         assertThat(fields.get("date_field").getValues().get(0), equalTo("11"));
@@ -820,21 +827,20 @@ public class FieldFetcherTests extends MapperServiceTestCase {
 
     private Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, String fieldPattern)
         throws IOException {
-        return fetchFields(mapperService, source, fieldAndFormatList(fieldPattern, null, false), null);
+        return fetchFields(mapperService, source, fieldAndFormatList(fieldPattern, null, false));
     }
 
     private static Map<String, DocumentField> fetchFields(
         MapperService mapperService,
         XContentBuilder source,
-        List<FieldAndFormat> fields,
-        @Nullable Set<String> ignoreFields
+        List<FieldAndFormat> fields
     ) throws IOException {
 
         SourceLookup sourceLookup = new SourceLookup();
         sourceLookup.setSource(BytesReference.bytes(source));
 
         FieldFetcher fieldFetcher = FieldFetcher.create(newSearchExecutionContext(mapperService), fields);
-        return fieldFetcher.fetch(sourceLookup, ignoreFields != null ? ignoreFields : Collections.emptySet());
+        return fieldFetcher.fetch(sourceLookup);
     }
 
     public MapperService createMapperService() throws IOException {

+ 12 - 1
test/framework/src/main/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java

@@ -47,6 +47,17 @@ public abstract class FieldTypeTestCase extends ESTestCase {
         ValueFetcher fetcher = fieldType.valueFetcher(searchExecutionContext, format);
         SourceLookup lookup = new SourceLookup();
         lookup.setSource(Collections.singletonMap(field, sourceValue));
-        return fetcher.fetchValues(lookup, Collections.emptySet());
+        return fetcher.fetchValues(lookup);
+    }
+
+    public static List<?> fetchSourceValues(MappedFieldType fieldType, Object... values) throws IOException {
+        String field = fieldType.name();
+        SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
+        when(searchExecutionContext.sourcePath(field)).thenReturn(Set.of(field));
+
+        ValueFetcher fetcher = fieldType.valueFetcher(searchExecutionContext, null);
+        SourceLookup lookup = new SourceLookup();
+        lookup.setSource(Collections.singletonMap(field, List.of(values)));
+        return fetcher.fetchValues(lookup);
     }
 }

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

@@ -309,7 +309,7 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
             LeafReaderContext context = searcher.getIndexReader().leaves().get(0);
             lookup.source().setSegmentAndDocument(context, 0);
             valueFetcher.setNextReader(context);
-            result.set(valueFetcher.fetchValues(lookup.source(), Collections.emptySet()));
+            result.set(valueFetcher.fetchValues(lookup.source()));
         });
         return result.get();
     }

+ 2 - 2
x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java

@@ -137,8 +137,8 @@ public class ConstantKeywordFieldMapper extends FieldMapper {
             }
 
             return value == null
-                ? (lookup, ignoredFields) -> List.of()
-                : (lookup, ignoredFields) -> List.of(value);
+                ? lookup -> List.of()
+                : lookup -> List.of(value);
         }
 
         @Override

+ 4 - 4
x-pack/plugin/mapper-constant-keyword/src/test/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldTypeTests.java

@@ -114,13 +114,13 @@ public class ConstantKeywordFieldTypeTests extends FieldTypeTestCase {
         SourceLookup nullValueLookup = new SourceLookup();
         nullValueLookup.setSource(Collections.singletonMap("field", null));
 
-        assertTrue(fetcher.fetchValues(missingValueLookup, Collections.emptySet()).isEmpty());
-        assertTrue(fetcher.fetchValues(nullValueLookup, Collections.emptySet()).isEmpty());
+        assertTrue(fetcher.fetchValues(missingValueLookup).isEmpty());
+        assertTrue(fetcher.fetchValues(nullValueLookup).isEmpty());
 
         MappedFieldType valued = new ConstantKeywordFieldMapper.ConstantKeywordFieldType("field", "foo");
         fetcher = valued.valueFetcher(null, null);
 
-        assertEquals(List.of("foo"), fetcher.fetchValues(missingValueLookup, Collections.emptySet()));
-        assertEquals(List.of("foo"), fetcher.fetchValues(nullValueLookup, Collections.emptySet()));
+        assertEquals(List.of("foo"), fetcher.fetchValues(missingValueLookup));
+        assertEquals(List.of("foo"), fetcher.fetchValues(nullValueLookup));
     }
 }

+ 1 - 1
x-pack/plugin/mapper-flattened/src/main/java/org/elasticsearch/xpack/flattened/mapper/FlattenedFieldMapper.java

@@ -259,7 +259,7 @@ public final class FlattenedFieldMapper extends DynamicKeyFieldMapper {
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
             // This is an internal field but it can match a field pattern so we return an empty list.
-            return (lookup, ignoredFields) -> List.of();
+            return lookup -> List.of();
         }
     }