Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

/**
Expand All @@ -85,7 +84,6 @@ public class ScriptScoreBenchmark {
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null, null))
);
private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
private final Map<String, Set<String>> sourcePaths = Map.of("n", Set.of("n"));
private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();
private final SearchLookup lookup = new SearchLookup(
fieldTypes::get,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Objects;

Expand Down Expand Up @@ -121,7 +120,7 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
@Override
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
if (hasDocValues() == false) {
return (lookup, doc, ignoredValues) -> List.of();
return ValueFetcher.EMPTY;
}
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;

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

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.elasticsearch.index.query.SearchExecutionContext;

import java.util.Collections;
import java.util.List;

public class SizeFieldMapper extends MetadataFieldMapper {
public static final String NAME = "_size";
Expand Down Expand Up @@ -57,7 +56,7 @@ private static class SizeFieldType extends NumberFieldType {
@Override
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
if (hasDocValues() == false) {
return (lookup, doc, ignoredValues) -> List.of();
return ValueFetcher.EMPTY;
}
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public Status needsField(FieldInfo fieldInfo) {
}

private void addValue(Object value) {
destination.add(field.valueForDisplay(value));
destination.add(value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my curiosity, Was this call (#valueForDisplay) not necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's moved into FieldLookup.setValues(), mainly because there are now two paths that can set it (here in the stored fields provider and also in the fetchphase pre-loaded implementation)

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.script.CompositeFieldScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.xcontent.XContentBuilder;

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

@Override
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
return new DocValueFetcher(docValueFormat(format, null), context.getForField(this, FielddataOperation.SEARCH));
return new DocValueFetcher(
docValueFormat(format, null),
context.getForField(this, FielddataOperation.SEARCH),
StoredFieldsSpec.NEEDS_SOURCE // for now we assume runtime fields need source
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.Source;

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

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
}

/**
* Given a value that has been extracted from a document's source, parse it into a standard
* format. This parsing logic should closely mirror the value parsing in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.index.fielddata.FormattedDocValues;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.Source;

import java.io.IOException;
Expand All @@ -23,14 +24,22 @@
/**
* Value fetcher that loads from doc values.
*/
// TODO rename this? It doesn't load from doc values, it loads from fielddata
// Which might be doc values, but might not be...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, so FormattedDocValues can actually come from a stored field?

public final class DocValueFetcher implements ValueFetcher {
private final DocValueFormat format;
private final IndexFieldData<?> ifd;
private FormattedDocValues formattedDocValues;
private final StoredFieldsSpec storedFieldsSpec;

public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd) {
public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd, StoredFieldsSpec storedFieldsSpec) {
this.format = format;
this.ifd = ifd;
this.storedFieldsSpec = storedFieldsSpec;
}

public DocValueFetcher(DocValueFormat format, IndexFieldData<?> ifd) {
this(format, ifd, StoredFieldsSpec.NO_REQUIREMENTS);
}

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

@Override
public StoredFieldsSpec storedFieldsSpec() {
return storedFieldsSpec;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.script.GeoPointFieldScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.field.GeoPointDocValuesField;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.Source;
import org.elasticsearch.search.runtime.GeoPointScriptFieldDistanceFeatureQuery;
Expand Down Expand Up @@ -215,6 +216,11 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu
}
return formatter.apply(points);
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.Source;

import java.util.Collections;
Expand Down Expand Up @@ -85,6 +86,11 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format)
public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValues) {
return indexName;
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NO_REQUIREMENTS;
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.script.CompositeFieldScript;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.fetch.subphase.FieldAndFormat;
import org.elasticsearch.search.fetch.subphase.LookupField;
import org.elasticsearch.search.lookup.SearchLookup;
Expand Down Expand Up @@ -245,5 +246,10 @@ public DocumentField fetchDocumentField(String docName, Source source, int doc)
public void setNextReader(LeafReaderContext context) {
inputFieldValueFetcher.setNextReader(context);
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return inputFieldValueFetcher.storedFieldsSpec();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.common.document.DocumentField;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.fetch.subphase.FieldFetcher;
import org.elasticsearch.search.lookup.Source;

Expand Down Expand Up @@ -86,4 +87,9 @@ private Map<String, Object> createSourceMapStub(Map<String, Object> filteredSour
public void setNextReader(LeafReaderContext context) {
this.nestedFieldFetcher.setNextReader(context);
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.Source;

import java.util.ArrayDeque;
Expand Down Expand Up @@ -93,6 +94,11 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu
return values;
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
}

/**
* Given a value that has been extracted from a document's source, parse it into a standard
* format. This parsing logic should closely mirror the value parsing in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
package org.elasticsearch.index.mapper;

import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.LeafSearchLookup;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.Source;

import java.io.IOException;
import java.util.List;
import java.util.Set;

/**
* Value fetcher that loads from stored values.
Expand All @@ -24,10 +26,12 @@ public class StoredValueFetcher implements ValueFetcher {
private final SearchLookup lookup;
private LeafSearchLookup leafSearchLookup;
private final String fieldname;
private final StoredFieldsSpec storedFieldsSpec;

public StoredValueFetcher(SearchLookup lookup, String fieldname) {
this.lookup = lookup;
this.fieldname = fieldname;
this.storedFieldsSpec = new StoredFieldsSpec(false, false, Set.of(fieldname));
}

@Override
Expand All @@ -53,4 +57,8 @@ protected List<Object> parseStoredValues(List<Object> values) {
return values;
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return storedFieldsSpec;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.common.document.DocumentField;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.fetch.subphase.FetchFieldsPhase;
import org.elasticsearch.search.lookup.Source;

Expand Down Expand Up @@ -66,4 +67,35 @@ default DocumentField fetchDocumentField(String docName, Source lookup, int doc)
* Update the leaf reader used to fetch values.
*/
default void setNextReader(LeafReaderContext context) {}

/**
* The stored field or source requirements of this value fetcher
*/
StoredFieldsSpec storedFieldsSpec();

ValueFetcher EMPTY = new ValueFetcher() {
@Override
public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValues) {
return List.of();
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NO_REQUIREMENTS;
}
};

static ValueFetcher singleton(Object value) {
return new ValueFetcher() {
@Override
public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValues) {
return List.of(value);
}

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NO_REQUIREMENTS;
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.NestedDocuments;
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
import org.elasticsearch.search.lookup.LeafFieldLookupProvider;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.SourceProvider;
import org.elasticsearch.transport.RemoteClusterAware;
Expand Down Expand Up @@ -494,27 +495,31 @@ public SearchLookup lookup() {
SourceProvider sourceProvider = isSourceSynthetic()
? (ctx, doc) -> { throw new IllegalArgumentException("Cannot access source from scripts in synthetic mode"); }
: SourceProvider.fromStoredFields();
setSourceProvider(sourceProvider);
setLookupProviders(sourceProvider, LeafFieldLookupProvider.fromStoredFields());
}
return this.lookup;
}

/**
* Replace the standard source provider on the SearchLookup
* Replace the standard source provider and field lookup provider on the SearchLookup
*
* Note that this will replace the current SearchLookup with a new one, but will not update
* the source provider on previously build lookups. This method should only be called before
* IndexReader access by the current context
*/
public void setSourceProvider(SourceProvider sourceProvider) {
public void setLookupProviders(
SourceProvider sourceProvider,
Function<LeafReaderContext, LeafFieldLookupProvider> fieldLookupProvider
) {
// TODO can we assert that this is only called during FetchPhase?
this.lookup = new SearchLookup(
this::getFieldType,
(fieldType, searchLookup, fielddataOperation) -> indexFieldDataLookup.apply(
fieldType,
new FieldDataContext(fullyQualifiedIndex.getName(), searchLookup, this::sourcePath, fielddataOperation)
),
sourceProvider
sourceProvider,
fieldLookupProvider
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,8 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
for (org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField field : source.scriptFields()) {
FieldScript.Factory factory = scriptService.compile(field.script(), FieldScript.CONTEXT);
SearchLookup lookup = context.getSearchExecutionContext().lookup();
// TODO delay this construction until the FetchPhase is executed so that we can
// use the more efficient lookup built there
FieldScript.LeafFactory searchScript = factory.newFactory(field.script().getParams(), lookup);
context.scriptFields().add(new ScriptField(field.fieldName(), searchScript, field.ignoreFailure()));
}
Expand Down
Loading