Skip to content

Commit 131da70

Browse files
authored
ValueFetchers now return a StoredFieldsSpec (#94820)
This allows us to be more conservative about what needs to be loaded when using the fields API, and opens up the possibility of avoiding using stored fields or source altogether if we can use doc values to fetch values. This commit also uses this new information from ValueFetchers to more efficiently preload stored fields for the `fields` API, while still allowing the lazy loading of individual fields if they are asked for by scripts or runtime fields which cannot be introspected.
1 parent 431a7b2 commit 131da70

File tree

34 files changed

+382
-109
lines changed

34 files changed

+382
-109
lines changed

benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import java.nio.file.Path;
5959
import java.util.List;
6060
import java.util.Map;
61-
import java.util.Set;
6261
import java.util.concurrent.TimeUnit;
6362

6463
/**
@@ -85,7 +84,6 @@ public class ScriptScoreBenchmark {
8584
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null, null))
8685
);
8786
private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
88-
private final Map<String, Set<String>> sourcePaths = Map.of("n", Set.of("n"));
8987
private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();
9088
private final SearchLookup lookup = new SearchLookup(
9189
fieldTypes::get,

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.xcontent.XContentBuilder;
2525

2626
import java.io.IOException;
27-
import java.util.List;
2827
import java.util.Map;
2928
import java.util.Objects;
3029

@@ -121,7 +120,7 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
121120
@Override
122121
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
123122
if (hasDocValues() == false) {
124-
return (lookup, doc, ignoredValues) -> List.of();
123+
return ValueFetcher.EMPTY;
125124
}
126125
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
127126
}

modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentIdFieldMapper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3030

3131
import java.util.Collections;
32-
import java.util.List;
3332
import java.util.Map;
3433

3534
/**
@@ -75,7 +74,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
7574
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
7675
// Although this is an internal field, we return it in the list of all field types. So we
7776
// provide an empty value fetcher here instead of throwing an error.
78-
return (lookup, doc, ignoredValues) -> List.of();
77+
return ValueFetcher.EMPTY;
7978
}
8079

8180
@Override

plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.index.query.SearchExecutionContext;
2222

2323
import java.util.Collections;
24-
import java.util.List;
2524

2625
public class SizeFieldMapper extends MetadataFieldMapper {
2726
public static final String NAME = "_size";
@@ -57,7 +56,7 @@ private static class SizeFieldType extends NumberFieldType {
5756
@Override
5857
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
5958
if (hasDocValues() == false) {
60-
return (lookup, doc, ignoredValues) -> List.of();
59+
return ValueFetcher.EMPTY;
6160
}
6261
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
6362
}

server/src/main/java/org/elasticsearch/index/fieldvisitor/SingleFieldsVisitor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public Status needsField(FieldInfo fieldInfo) {
4646
}
4747

4848
private void addValue(Object value) {
49-
destination.add(field.valueForDisplay(value));
49+
destination.add(value);
5050
}
5151

5252
@Override

server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.script.CompositeFieldScript;
2424
import org.elasticsearch.script.Script;
2525
import org.elasticsearch.script.ScriptContext;
26+
import org.elasticsearch.search.fetch.StoredFieldsSpec;
2627
import org.elasticsearch.search.lookup.SearchLookup;
2728
import org.elasticsearch.xcontent.XContentBuilder;
2829

@@ -176,7 +177,11 @@ protected final void applyScriptContext(SearchExecutionContext context) {
176177

177178
@Override
178179
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
179-
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
180+
return new DocValueFetcher(
181+
docValueFormat(format, null),
182+
context.getForField(this, FielddataOperation.SEARCH),
183+
StoredFieldsSpec.NEEDS_SOURCE // for now we assume runtime fields need source
184+
);
180185
}
181186

182187
/**

server/src/main/java/org/elasticsearch/index/mapper/ArraySourceValueFetcher.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.elasticsearch.core.Nullable;
1212
import org.elasticsearch.index.query.SearchExecutionContext;
13+
import org.elasticsearch.search.fetch.StoredFieldsSpec;
1314
import org.elasticsearch.search.lookup.Source;
1415

1516
import java.util.ArrayList;
@@ -71,6 +72,11 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu
7172
return values;
7273
}
7374

75+
@Override
76+
public StoredFieldsSpec storedFieldsSpec() {
77+
return StoredFieldsSpec.NEEDS_SOURCE;
78+
}
79+
7480
/**
7581
* Given a value that has been extracted from a document's source, parse it into a standard
7682
* format. This parsing logic should closely mirror the value parsing in

server/src/main/java/org/elasticsearch/index/mapper/DocValueFetcher.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.index.fielddata.FormattedDocValues;
1313
import org.elasticsearch.index.fielddata.IndexFieldData;
1414
import org.elasticsearch.search.DocValueFormat;
15+
import org.elasticsearch.search.fetch.StoredFieldsSpec;
1516
import org.elasticsearch.search.lookup.Source;
1617

1718
import java.io.IOException;
@@ -23,14 +24,22 @@
2324
/**
2425
* Value fetcher that loads from doc values.
2526
*/
27+
// TODO rename this? It doesn't load from doc values, it loads from fielddata
28+
// Which might be doc values, but might not be...
2629
public final class DocValueFetcher implements ValueFetcher {
2730
private final DocValueFormat format;
2831
private final IndexFieldData<?> ifd;
2932
private FormattedDocValues formattedDocValues;
33+
private final StoredFieldsSpec storedFieldsSpec;
3034

31-
public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd) {
35+
public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd, StoredFieldsSpec storedFieldsSpec) {
3236
this.format = format;
3337
this.ifd = ifd;
38+
this.storedFieldsSpec = storedFieldsSpec;
39+
}
40+
41+
public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd) {
42+
this(format, ifd, StoredFieldsSpec.NO_REQUIREMENTS);
3443
}
3544

3645
@Override
@@ -50,4 +59,8 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu
5059
return result;
5160
}
5261

62+
@Override
63+
public StoredFieldsSpec storedFieldsSpec() {
64+
return storedFieldsSpec;
65+
}
5366
}

server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.script.GeoPointFieldScript;
2929
import org.elasticsearch.script.Script;
3030
import org.elasticsearch.script.field.GeoPointDocValuesField;
31+
import org.elasticsearch.search.fetch.StoredFieldsSpec;
3132
import org.elasticsearch.search.lookup.SearchLookup;
3233
import org.elasticsearch.search.lookup.Source;
3334
import org.elasticsearch.search.runtime.GeoPointScriptFieldDistanceFeatureQuery;
@@ -215,6 +216,11 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu
215216
}
216217
return formatter.apply(points);
217218
}
219+
220+
@Override
221+
public StoredFieldsSpec storedFieldsSpec() {
222+
return StoredFieldsSpec.NEEDS_SOURCE;
223+
}
218224
};
219225
}
220226
}

server/src/main/java/org/elasticsearch/index/mapper/IndexFieldMapper.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.index.query.SearchExecutionContext;
2020
import org.elasticsearch.script.field.DelegateDocValuesField;
2121
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
22+
import org.elasticsearch.search.fetch.StoredFieldsSpec;
2223
import org.elasticsearch.search.lookup.Source;
2324

2425
import java.util.Collections;
@@ -85,6 +86,11 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format)
8586
public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValues) {
8687
return indexName;
8788
}
89+
90+
@Override
91+
public StoredFieldsSpec storedFieldsSpec() {
92+
return StoredFieldsSpec.NO_REQUIREMENTS;
93+
}
8894
};
8995
}
9096

0 commit comments

Comments
 (0)