Skip to content

Commit

Permalink
Fields API to allow fetching values when _source is disabled (#87267)
Browse files Browse the repository at this point in the history
Back when we introduced the fields parameter to the search API, it could only fetch values from _source, hence
the corresponding sub-fetch phase fails early whenever _source is disabled. Today though runtime fields can
be retrieved from a separate value fetcher that reads from fielddata, and metadata fields can be retrieved
from stored fields. These two scenarios currently throw an unnecessary error whenever _source is disabled.

This commit removes the check for disabled _source, so that runtime fields and metadata fields can be retrieved even when _source is disabled. Fields that need to be loaded from _source are simply skipped whenever _source is disabled, similar to when a field is not found in _source.

Closes #87072
  • Loading branch information
javanna committed Jun 2, 2022
1 parent 4a3ce31 commit 50793a6
Show file tree
Hide file tree
Showing 14 changed files with 321 additions and 360 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/87267.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 87267
summary: Fields API to allow fetching values when `_source` is disabled
area: Search
type: bug
issues:
- 87072
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Other mapping options are also respected, including
The `fields` option returns values in the way that matches how {es} indexes
them. For standard fields, this means that the `fields` option looks in
`_source` to find the values, then parses and formats them using the mappings.
Selected fields that can't be found in `_source` are skipped.

[discrete]
[[search-fields-request]]
Expand Down
1 change: 1 addition & 0 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task ->
task.skipTest("search.aggregation/20_terms/string profiler via map", "The profiler results aren't backwards compatible.")
task.skipTest("search.aggregation/20_terms/numeric profiler", "The profiler results aren't backwards compatible.")
task.skipTest("migration/10_get_feature_upgrade_status/Get feature upgrade status", "Awaits backport")
task.skipTest("search/330_fetch_fields/Test disable source", "Error no longer thrown")

task.replaceValueInMatch("_type", "_doc")
task.addAllowedWarningRegex("\\[types removal\\].*")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
setup:
- skip:
version: " - 7.9.99"
reason: "the 'fields' parameter was added in 7.10"

---
"Test basic field retrieval":
- do:
Expand Down Expand Up @@ -95,6 +90,10 @@ setup:

---
"Test disable source":
- skip:
version: " - 8.3.99"
reason: "behaviour of fields when source is disabled changed in 8.4.0"

- do:
indices.create:
index: test
Expand All @@ -107,27 +106,29 @@ setup:
properties:
keyword:
type: keyword
location:
type: geo_point

- do:
index:
index: test
id: "1"
body:
keyword: [ "a" ]
location: [12, 12]

- do:
indices.refresh:
index: [ test ]

- do:
catch: bad_request
search:
index: test
body:
fields: [keyword]
- match: { error.root_cause.0.type: "illegal_argument_exception" }
- match: { error.root_cause.0.reason: "Unable to retrieve the requested [fields] since _source is disabled
in the mappings for index [test]" }
fields: [_id,keyword,location]

- length: { hits.hits.0.fields: 1 }
- match: { hits.hits.0.fields._id.0: "1" }

---
"Test ignore malformed":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.search.lookup.SourceLookup;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;

Expand All @@ -38,7 +39,7 @@ public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context)
* @param nullValue A optional substitute value if the _source value is 'null'.
*/
public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context, Object nullValue) {
this.sourcePaths = context.sourcePath(fieldName);
this.sourcePaths = context.isSourceEnabled() ? context.sourcePath(fieldName) : Collections.emptySet();
this.nullValue = nullValue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Queue;
import java.util.Set;
Expand All @@ -38,14 +39,14 @@ public SourceValueFetcher(String fieldName, SearchExecutionContext context) {
* @param nullValue An optional substitute value if the _source value is 'null'.
*/
public SourceValueFetcher(String fieldName, SearchExecutionContext context, Object nullValue) {
this(context.sourcePath(fieldName), nullValue);
this(context.isSourceEnabled() ? context.sourcePath(fieldName) : Collections.emptySet(), nullValue);
}

/**
* @param sourcePaths The paths to pull source values from
* @param nullValue An optional substitute value if the _source value is `null`
*/
public SourceValueFetcher(Set<String> sourcePaths, Object nullValue) {
private SourceValueFetcher(Set<String> sourcePaths, Object nullValue) {
this.sourcePaths = sourcePaths;
this.nullValue = nullValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

/**
* A fetch sub-phase for high-level field retrieval. Given a list of fields, it
* retrieves the field values from _source and returns them as document fields.
* retrieves the field values through the relevant {@link org.elasticsearch.index.mapper.ValueFetcher}
* and returns them as document fields.
*/
public final class FetchFieldsPhase implements FetchSubPhase {
@Override
Expand All @@ -31,15 +32,6 @@ public FetchSubPhaseProcessor getProcessor(FetchContext fetchContext) {
return null;
}

if (fetchContext.getSearchExecutionContext().isSourceEnabled() == false) {
throw new IllegalArgumentException(
"Unable to retrieve the requested [fields] since _source is disabled "
+ "in the mappings for index ["
+ fetchContext.getIndexName()
+ "]"
);
}

FieldFetcher fieldFetcher = FieldFetcher.create(fetchContext.getSearchExecutionContext(), fetchFieldsContext.fields());

return new FetchSubPhaseProcessor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

/**
* 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.
* Then given a specific document, it can retrieve the corresponding fields through their corresponding {@link ValueFetcher}s.
*/
public class FieldFetcher {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,19 @@
import org.apache.lucene.search.IndexSearcher;
import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.document.DocumentField;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.fetch.subphase.FieldAndFormat;
import org.elasticsearch.search.fetch.subphase.FieldFetcher;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.xcontent.XContentBuilder;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.Map;

import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class PlaceHolderFieldMapperTests extends MapperServiceTestCase {

Expand Down Expand Up @@ -61,15 +62,17 @@ public void testFetchValue() throws Exception {
);
}, iw -> {
SearchLookup lookup = new SearchLookup(mapperService::fieldType, fieldDataLookup());
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.lookup()).thenReturn(lookup);
when(searchExecutionContext.sourcePath("field")).thenReturn(Set.of("field"));
ValueFetcher valueFetcher = mapperService.fieldType("field").valueFetcher(searchExecutionContext, null);
SearchExecutionContext searchExecutionContext = createSearchExecutionContext(mapperService);
FieldFetcher fieldFetcher = FieldFetcher.create(
searchExecutionContext,
Collections.singletonList(new FieldAndFormat("field", null))
);
IndexSearcher searcher = newSearcher(iw);
LeafReaderContext context = searcher.getIndexReader().leaves().get(0);
lookup.source().setSegmentAndDocument(context, 0);
valueFetcher.setNextReader(context);
assertEquals(List.of("value"), valueFetcher.fetchValues(lookup.source(), new ArrayList<>()));
Map<String, DocumentField> fields = fieldFetcher.fetch(lookup.source());
assertEquals(1, fields.size());
assertEquals(List.of("value"), fields.get("field").getValues());
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ public void testFetchSourceValue() throws IOException {
Map<String, Object> sourceValue = Map.of("key", "value");

SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.isSourceEnabled()).thenReturn(true);
when(searchExecutionContext.sourcePath("field.key")).thenReturn(Set.of("field.key"));

ValueFetcher fetcher = ft.valueFetcher(searchExecutionContext, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
import org.elasticsearch.index.mapper.LongFieldScriptTests;
import org.elasticsearch.index.mapper.LuceneDocument;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MapperServiceTestCase;
Expand All @@ -29,6 +31,8 @@
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.SourceLookup;
import org.elasticsearch.xcontent.XContentBuilder;
Expand Down Expand Up @@ -1070,6 +1074,103 @@ public void testTooManyUnmappedFieldWildcardPattern() throws IOException {
expectThrows(TooComplexToDeterminizeException.class, () -> fetchFields(mapperService, source, fieldAndFormatList));
}

public void testFetchFromSourceWithSourceDisabled() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder();
mapping.startObject();
{
mapping.startObject("_doc");
{
mapping.startObject("_source").field("enabled", false).endObject();
mapping.startObject("properties");
{
mapping.startObject("field").field("type", "keyword").endObject();
mapping.startObject("location").field("type", "geo_point").endObject();
}
mapping.endObject();
}
mapping.endObject();
}
mapping.endObject();

MapperService mapperService = createMapperService(mapping);
{
Map<String, DocumentField> fields = fetchFields(mapperService, null, "field");
assertEquals(0, fields.size());
}
{
Map<String, DocumentField> fields = fetchFields(mapperService, null, "location");
assertEquals(0, fields.size());
}
}

public void testFetchRuntimeFieldWithSourceDisabled() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder();
mapping.startObject();
{
mapping.startObject("_doc");
{
mapping.startObject("_source").field("enabled", false).endObject();
mapping.startObject("runtime");
{
mapping.startObject("runtime_field").field("type", "long").field("script", "emit(1);").endObject();
}
mapping.endObject();
}
mapping.endObject();
}
mapping.endObject();

MapperService mapperService = createMapperService(mapping);
SearchExecutionContext searchExecutionContext = newSearchExecutionContext(
mapperService,
(ft, index, sl) -> fieldDataLookup().apply(ft, sl)
);
withLuceneIndex(mapperService, iw -> iw.addDocument(new LuceneDocument()), iw -> {
FieldFetcher fieldFetcher = FieldFetcher.create(searchExecutionContext, fieldAndFormatList("runtime_field", null, false));
IndexSearcher searcher = newSearcher(iw);
LeafReaderContext readerContext = searcher.getIndexReader().leaves().get(0);
fieldFetcher.setNextReader(readerContext);
Map<String, DocumentField> fields = fieldFetcher.fetch(new SourceLookup());
assertEquals(1, fields.size());
DocumentField field = fields.get("runtime_field");
assertEquals(1L, (long) field.getValue());
});
}

public void testFetchMetadataFieldWithSourceDisabled() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder();
mapping.startObject();
{
mapping.startObject("_doc");
{
mapping.startObject("_source").field("enabled", false).endObject();
}
mapping.endObject();
}
mapping.endObject();

MapperService mapperService = createMapperService(mapping);
SearchExecutionContext searchExecutionContext = newSearchExecutionContext(
mapperService,
(ft, index, sl) -> fieldDataLookup().apply(ft, sl)
);
withLuceneIndex(mapperService, iw -> {
ParsedDocument parsedDocument = mapperService.documentMapper().parse(source("{}"));
iw.addDocument(parsedDocument.rootDoc());
}, iw -> {
FieldFetcher fieldFetcher = FieldFetcher.create(searchExecutionContext, fieldAndFormatList("_id", null, false));
IndexSearcher searcher = newSearcher(iw);
LeafReaderContext readerContext = searcher.getIndexReader().leaves().get(0);
fieldFetcher.setNextReader(readerContext);
SourceLookup sourceLookup = new SourceLookup();
sourceLookup.setSegmentAndDocument(readerContext, 0);
Map<String, DocumentField> fields = fieldFetcher.fetch(sourceLookup);
assertEquals(1, fields.size());
DocumentField field = fields.get("_id");
assertEquals("1", field.getValue());
});
}

private List<FieldAndFormat> fieldAndFormatList(String name, String format, boolean includeUnmapped) {
return Collections.singletonList(new FieldAndFormat(name, format, includeUnmapped));
}
Expand All @@ -1081,10 +1182,11 @@ private Map<String, DocumentField> fetchFields(MapperService mapperService, XCon

private static Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, List<FieldAndFormat> fields)
throws IOException {

SourceLookup sourceLookup = new SourceLookup();
sourceLookup.setSource(BytesReference.bytes(source));

SourceLookup sourceLookup = null;
if (source != null) {
sourceLookup = new SourceLookup();
sourceLookup.setSource(BytesReference.bytes(source));
}
FieldFetcher fieldFetcher = FieldFetcher.create(newSearchExecutionContext(mapperService), fields);
return fieldFetcher.fetch(sourceLookup);
}
Expand Down Expand Up @@ -1169,4 +1271,10 @@ private static SearchExecutionContext newSearchExecutionContext(
emptyMap()
);
}

@Override
@SuppressWarnings("unchecked")
protected <T> T compileScript(Script script, ScriptContext<T> context) {
return (T) LongFieldScriptTests.DUMMY;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV
public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceValue, String format) throws IOException {
String field = fieldType.name();
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.isSourceEnabled()).thenReturn(true);
when(searchExecutionContext.sourcePath(field)).thenReturn(Set.of(field));

ValueFetcher fetcher = fieldType.valueFetcher(searchExecutionContext, format);
Expand All @@ -62,6 +63,7 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV
public static List<?> fetchSourceValues(MappedFieldType fieldType, Object... values) throws IOException {
String field = fieldType.name();
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.isSourceEnabled()).thenReturn(true);
when(searchExecutionContext.sourcePath(field)).thenReturn(Set.of(field));

ValueFetcher fetcher = fieldType.valueFetcher(searchExecutionContext, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,10 @@ protected void assertFetch(MapperService mapperService, String field, Object val
ft.fielddataBuilder("test", () -> null).build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService())
);
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.isSourceEnabled()).thenReturn(true);
when(searchExecutionContext.sourcePath(field)).thenReturn(Set.of(field));
when(searchExecutionContext.getForField(ft)).thenAnswer(
inv -> { return fieldDataLookup().apply(ft, () -> { throw new UnsupportedOperationException(); }); }
inv -> fieldDataLookup().apply(ft, () -> { throw new UnsupportedOperationException(); })
);
ValueFetcher nativeFetcher = ft.valueFetcher(searchExecutionContext, format);
ParsedDocument doc = mapperService.documentMapper().parse(source);
Expand Down

0 comments on commit 50793a6

Please sign in to comment.