Consolidate validation for 'docvalue_fields'. (#60065)

This improves modularity and also fixes some issues when `docvalues_fields` is
used within `inner_hits` or the `top_hits` agg:
* We previously didn't resolve wildcards in field names.
* We also forgot to enforce the limit `index.max_docvalue_fields_search`.
This commit is contained in:
Julie Tibshirani 2020-07-22 17:26:58 -07:00 committed by GitHub
parent 67b07ec386
commit aa57bbd422
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 45 additions and 21 deletions

View File

@ -17,4 +17,16 @@ See also <<release-highlights>> and <<es-release-notes>>.
//tag::notable-breaking-changes[]
[discrete]
[[breaking_710_search_changes]]
=== Search Changes
[discrete]
==== Missing checks for `index.max_docvalue_fields_search`
The setting `index.max_docvalue_fields_search` limits the number of fields that
can be retrieved through `docvalue_fields` in a search. Previously we only
checked this limit when `docvalue_fields` appeared in the top-level search
request body. Now we also enforce the limit when loading doc value fields in
an `inner_hits` section or in a `top_hits` aggregation.
//end::notable-breaking-changes[]

View File

@ -165,7 +165,7 @@ public class InnerHitsIT extends ESIntegTestCase {
.setQuery(nestedQuery("comments", matchQuery("comments.message", "fox"), ScoreMode.Avg).innerHit(
new InnerHitBuilder().setHighlightBuilder(new HighlightBuilder().field("comments.message"))
.setExplain(true)
.addDocValueField("comments.message")
.addDocValueField("comments.mes*")
.addScriptField("script",
new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", Collections.emptyMap()))
.setSize(1))).get();

View File

@ -88,7 +88,9 @@ public abstract class InnerHitContextBuilder {
innerHitsContext.storedFieldsContext(innerHitBuilder.getStoredFieldsContext());
}
if (innerHitBuilder.getDocValueFields() != null) {
innerHitsContext.docValuesContext(new FetchDocValuesContext(innerHitBuilder.getDocValueFields()));
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(
queryShardContext.getMapperService(), innerHitBuilder.getDocValueFields());
innerHitsContext.docValuesContext(docValuesContext);
}
if (innerHitBuilder.getScriptFields() != null) {
for (SearchSourceBuilder.ScriptField field : innerHitBuilder.getScriptFields()) {

View File

@ -114,8 +114,6 @@ import org.elasticsearch.threadpool.ThreadPool.Names;
import org.elasticsearch.transport.TransportRequest;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@ -917,21 +915,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
context.fetchSourceContext(source.fetchSource());
}
if (source.docValueFields() != null) {
List<FetchDocValuesContext.FieldAndFormat> docValueFields = new ArrayList<>();
for (FetchDocValuesContext.FieldAndFormat format : source.docValueFields()) {
Collection<String> fieldNames = context.mapperService().simpleMatchToFullName(format.field);
for (String fieldName: fieldNames) {
docValueFields.add(new FetchDocValuesContext.FieldAndFormat(fieldName, format.format));
}
}
int maxAllowedDocvalueFields = context.mapperService().getIndexSettings().getMaxDocvalueFields();
if (docValueFields.size() > maxAllowedDocvalueFields) {
throw new IllegalArgumentException(
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [" + maxAllowedDocvalueFields
+ "] but was [" + docValueFields.size() + "]. This limit can be set by changing the ["
+ IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey() + "] index level setting.");
}
context.docValuesContext(new FetchDocValuesContext(docValueFields));
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(context.mapperService(), source.docValueFields());
context.docValuesContext(docValuesContext);
}
if (source.highlighter() != null) {
HighlightBuilder highlightBuilder = source.highlighter();

View File

@ -106,7 +106,8 @@ class TopHitsAggregatorFactory extends AggregatorFactory {
subSearchContext.storedFieldsContext(storedFieldsContext);
}
if (docValueFields != null) {
subSearchContext.docValuesContext(new FetchDocValuesContext(docValueFields));
FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(searchContext.mapperService(), docValueFields);
subSearchContext.docValuesContext(docValuesContext);
}
for (ScriptFieldsContext.ScriptField field : scriptFields) {
subSearchContext.scriptFields().add(field);

View File

@ -28,8 +28,12 @@ import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.MapperService;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
@ -113,7 +117,27 @@ public class FetchDocValuesContext {
private final List<FieldAndFormat> fields;
public FetchDocValuesContext(List<FieldAndFormat> fields) {
public static FetchDocValuesContext create(MapperService mapperService,
List<FieldAndFormat> fieldPatterns) {
List<FieldAndFormat> fields = new ArrayList<>();
for (FieldAndFormat field : fieldPatterns) {
Collection<String> fieldNames = mapperService.simpleMatchToFullName(field.field);
for (String fieldName: fieldNames) {
fields.add(new FieldAndFormat(fieldName, field.format));
}
}
int maxAllowedDocvalueFields = mapperService.getIndexSettings().getMaxDocvalueFields();
if (fields.size() > maxAllowedDocvalueFields) {
throw new IllegalArgumentException(
"Trying to retrieve too many docvalue_fields. Must be less than or equal to: [" + maxAllowedDocvalueFields
+ "] but was [" + fields.size() + "]. This limit can be set by changing the ["
+ IndexSettings.MAX_DOCVALUE_FIELDS_SEARCH_SETTING.getKey() + "] index level setting.");
}
return new FetchDocValuesContext(fields);
}
FetchDocValuesContext(List<FieldAndFormat> fields) {
this.fields = fields;
}