Don't expand default_field in query_string before required (#55158) (#55650)

Currently QueryStringQueryParser already checks if the field limit is breached
at construction time, which e.g. leads to errors if the default field is set to
"*" or the default isn't used and there are more fields than the limit, even if the
query itself does not use all these fields.
This change moves this check to happen after query parsing. QueryStringQueryParser now
keeps track of the fields that are actually resolved while parsing. The size of
that set is later used to check against the limit set by the
`indices.query.bool.max_clause_count` setting.

Backport of #55158
This commit is contained in:
Christoph Büscher 2020-04-23 13:25:23 +02:00 committed by GitHub
parent 033e870d97
commit ed0ced9290
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 106 additions and 73 deletions

View File

@ -20,6 +20,7 @@
package org.elasticsearch.index.search;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryShardContext;
@ -79,7 +80,7 @@ public final class QueryParserHelper {
* The original name of the field is kept if adding the suffix to the field name does not point to a valid field
* in the mapping.
*/
public static Map<String, Float> resolveMappingFields(QueryShardContext context,
static Map<String, Float> resolveMappingFields(QueryShardContext context,
Map<String, Float> fieldsAndWeights,
String fieldSuffix) {
Map<String, Float> resolvedFields = new HashMap<>();
@ -97,26 +98,10 @@ public final class QueryParserHelper {
resolvedFields.put(field.getKey(), boost);
}
}
checkForTooManyFields(resolvedFields, context);
checkForTooManyFields(resolvedFields.size(), context, null);
return resolvedFields;
}
/**
* Resolves the provided pattern or field name from the {@link QueryShardContext} and return a map of
* the expanded fields with their original boost.
* @param context The context of the query
* @param fieldOrPattern The field name or the pattern to resolve
* @param weight The weight for the field
* @param acceptAllTypes Whether all field type should be added when a pattern is expanded.
* If false, only searchable field types are added.
* @param acceptMetadataField Whether metadata fields should be added when a pattern is expanded.
*/
public static Map<String, Float> resolveMappingField(QueryShardContext context, String fieldOrPattern, float weight,
boolean acceptAllTypes, boolean acceptMetadataField) {
return resolveMappingField(context, fieldOrPattern, weight, acceptAllTypes, acceptMetadataField, null);
}
/**
* Resolves the provided pattern or field name from the {@link QueryShardContext} and return a map of
* the expanded fields with their original boost.
@ -130,7 +115,7 @@ public final class QueryParserHelper {
* The original name of the field is kept if adding the suffix to the field name does not point to a valid field
* in the mapping.
*/
public static Map<String, Float> resolveMappingField(QueryShardContext context, String fieldOrPattern, float weight,
static Map<String, Float> resolveMappingField(QueryShardContext context, String fieldOrPattern, float weight,
boolean acceptAllTypes, boolean acceptMetadataField, String fieldSuffix) {
Set<String> allFields = context.simpleMatchToIndexNames(fieldOrPattern);
Map<String, Float> fields = new HashMap<>();
@ -170,15 +155,18 @@ public final class QueryParserHelper {
float w = fields.getOrDefault(fieldName, 1.0F);
fields.put(fieldName, w * weight);
}
checkForTooManyFields(fields, context);
return fields;
}
private static void checkForTooManyFields(Map<String, Float> fields, QueryShardContext context) {
static void checkForTooManyFields(int numberOfFields, QueryShardContext context, @Nullable String inputPattern) {
Integer limit = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings());
if (fields.size() > limit) {
throw new IllegalArgumentException("field expansion matches too many fields, limit: " + limit + ", got: " + fields.size());
if (numberOfFields > limit) {
StringBuilder errorMsg = new StringBuilder("field expansion ");
if (inputPattern != null) {
errorMsg.append("for [" + inputPattern + "] ");
}
errorMsg.append("matches too many fields, limit: " + limit + ", got: " + numberOfFields);
throw new IllegalArgumentException(errorMsg.toString());
}
}

View File

@ -66,6 +66,7 @@ import java.util.Map;
import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded;
import static org.elasticsearch.common.lucene.search.Queries.newLenientFieldQuery;
import static org.elasticsearch.common.lucene.search.Queries.newUnmappedFieldQuery;
import static org.elasticsearch.index.search.QueryParserHelper.checkForTooManyFields;
import static org.elasticsearch.index.search.QueryParserHelper.resolveMappingField;
import static org.elasticsearch.index.search.QueryParserHelper.resolveMappingFields;
@ -132,13 +133,13 @@ public class QueryStringQueryParser extends XQueryParser {
}
/**
* Defaults to all queryiable fields extracted from the mapping for query terms
* Defaults to all queryable fields extracted from the mapping for query terms
* @param context The query shard context
* @param lenient If set to `true` will cause format based failures (like providing text to a numeric field) to be ignored.
*/
public QueryStringQueryParser(QueryShardContext context, boolean lenient) {
this(context, "*",
resolveMappingField(context, "*", 1.0f, false, false),
resolveMappingField(context, "*", 1.0f, false, false, null),
lenient, context.getMapperService().searchAnalyzer());
}
@ -268,21 +269,24 @@ public class QueryStringQueryParser extends XQueryParser {
}
private Map<String, Float> extractMultiFields(String field, boolean quoted) {
Map<String, Float> extractedFields;
if (field != null) {
boolean allFields = Regex.isMatchAllPattern(field);
if (allFields && this.field != null && this.field.equals(field)) {
// "*" is the default field
return fieldsAndWeights;
extractedFields = fieldsAndWeights;
}
boolean multiFields = Regex.isSimpleMatchPattern(field);
// Filters unsupported fields if a pattern is requested
// Filters metadata fields if all fields are requested
return resolveMappingField(context, field, 1.0f, !allFields, !multiFields, quoted ? quoteFieldSuffix : null);
extractedFields = resolveMappingField(context, field, 1.0f, !allFields, !multiFields, quoted ? quoteFieldSuffix : null);
} else if (quoted && quoteFieldSuffix != null) {
return resolveMappingFields(context, fieldsAndWeights, quoteFieldSuffix);
extractedFields = resolveMappingFields(context, fieldsAndWeights, quoteFieldSuffix);
} else {
return fieldsAndWeights;
extractedFields = fieldsAndWeights;
}
checkForTooManyFields(extractedFields.size(), this.context, field);
return extractedFields;
}
@Override

View File

@ -263,38 +263,6 @@ public class QueryStringIT extends ESIntegTestCase {
assertThat(e.getCause().getMessage(), containsString("unit [D] not supported for date math [-2D]"));
}
public void testLimitOnExpandedFields() throws Exception {
XContentBuilder builder = jsonBuilder();
builder.startObject();
builder.startObject("type1");
builder.startObject("properties");
for (int i = 0; i < CLUSTER_MAX_CLAUSE_COUNT + 1; i++) {
builder.startObject("field" + i).field("type", "text").endObject();
}
builder.endObject(); // properties
builder.endObject(); // type1
builder.endObject();
assertAcked(prepareCreate("toomanyfields")
.setSettings(Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(),
CLUSTER_MAX_CLAUSE_COUNT + 100))
.addMapping("type1", builder));
client().prepareIndex("toomanyfields", "type1", "1").setSource("field1", "foo bar baz").get();
refresh();
Exception e = expectThrows(Exception.class, () -> {
QueryStringQueryBuilder qb = queryStringQuery("bar");
if (randomBoolean()) {
qb.defaultField("*");
}
client().prepareSearch("toomanyfields").setQuery(qb).get();
});
assertThat(ExceptionsHelper.unwrap(e, IllegalArgumentException.class).getMessage(),
containsString("field expansion matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT + ", got: "
+ (CLUSTER_MAX_CLAUSE_COUNT + 1)));
}
// The only expectation for this test is to not throw exception
public void testLimitOnExpandedFieldsButIgnoreUnmappedFields() throws Exception {
XContentBuilder builder = jsonBuilder();
@ -324,6 +292,74 @@ public class QueryStringIT extends ESIntegTestCase {
client().prepareSearch("ignoreunmappedfields").setQuery(qb).get();
}
public void testLimitOnExpandedFields() throws Exception {
XContentBuilder builder = jsonBuilder();
builder.startObject();
{
builder.startObject("_doc");
{
builder.startObject("properties");
{
for (int i = 0; i < CLUSTER_MAX_CLAUSE_COUNT; i++) {
builder.startObject("field_A" + i).field("type", "text").endObject();
builder.startObject("field_B" + i).field("type", "text").endObject();
}
builder.endObject();
}
builder.endObject();
}
builder.endObject();
}
assertAcked(prepareCreate("testindex")
.setSettings(Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(),
CLUSTER_MAX_CLAUSE_COUNT + 100))
.addMapping("_doc", builder));
client().prepareIndex("testindex", "_doc", "1").setSource("field_A0", "foo bar baz").get();
refresh();
// single field shouldn't trigger the limit
doAssertOneHitForQueryString("field_A0:foo");
// expanding to the limit should work
doAssertOneHitForQueryString("field_A\\*:foo");
// expanding two blocks to the limit still works
doAssertOneHitForQueryString("field_A\\*:foo field_B\\*:bar");
// adding a non-existing field on top shouldn't overshoot the limit
doAssertOneHitForQueryString("field_A\\*:foo unmapped:something");
// the following should exceed the limit
doAssertLimitExceededException("foo", CLUSTER_MAX_CLAUSE_COUNT * 2, "*");
doAssertLimitExceededException("*:foo", CLUSTER_MAX_CLAUSE_COUNT * 2, "*");
doAssertLimitExceededException("field_\\*:foo", CLUSTER_MAX_CLAUSE_COUNT * 2, "field_*");
}
private void doAssertOneHitForQueryString(String queryString) {
QueryStringQueryBuilder qb = queryStringQuery(queryString);
if (randomBoolean()) {
qb.defaultField("*");
}
SearchResponse response = client().prepareSearch("testindex").setQuery(qb).get();
assertHitCount(response, 1);
}
private void doAssertLimitExceededException(String queryString, int exceedingFieldCount, String inputFieldPattern) {
Exception e = expectThrows(Exception.class, () -> {
QueryStringQueryBuilder qb = queryStringQuery(queryString);
if (randomBoolean()) {
qb.defaultField("*");
}
client().prepareSearch("testindex").setQuery(qb).get();
});
assertThat(
ExceptionsHelper.unwrap(e, IllegalArgumentException.class).getMessage(),
containsString("field expansion for [" + inputFieldPattern + "] matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT
+ ", got: " + exceedingFieldCount
)
);
}
public void testFieldAlias() throws Exception {
List<IndexRequestBuilder> indexRequests = new ArrayList<>();
indexRequests.add(client().prepareIndex("test", "_doc", "1").setSource("f3", "text", "f2", "one"));

View File

@ -22,6 +22,7 @@ package org.elasticsearch.search.query;
import org.apache.lucene.analysis.TokenFilter;
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
@ -36,7 +37,7 @@ import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.Operator;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.SimpleQueryStringBuilder;
import org.elasticsearch.index.query.QueryStringQueryBuilder;
import org.elasticsearch.index.query.SimpleQueryStringFlag;
import org.elasticsearch.plugins.AnalysisPlugin;
import org.elasticsearch.plugins.Plugin;
@ -590,16 +591,20 @@ public class SimpleQueryStringIT extends ESIntegTestCase {
client().prepareIndex("toomanyfields", "type1", "1").setSource("field1", "foo bar baz").get();
refresh();
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> {
SimpleQueryStringBuilder qb = simpleQueryStringQuery("bar");
if (randomBoolean()) {
qb.field("*");
}
client().prepareSearch("toomanyfields").setQuery(qb).get();
});
assertThat(e.getDetailedMessage(),
containsString("field expansion matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT + ", got: "
+ (CLUSTER_MAX_CLAUSE_COUNT + 1)));
doAssertLimitExceededException("*", CLUSTER_MAX_CLAUSE_COUNT + 1);
doAssertLimitExceededException("field*", CLUSTER_MAX_CLAUSE_COUNT + 1);
}
private void doAssertLimitExceededException(String field, int exceedingFieldCount) {
Exception e = expectThrows(Exception.class, () -> {
QueryStringQueryBuilder qb = queryStringQuery("bar");
qb.field(field);
client().prepareSearch("toomanyfields").setQuery(qb).get();
});
assertThat(
ExceptionsHelper.unwrap(e, IllegalArgumentException.class).getMessage(),
containsString("field expansion matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT + ", got: " + exceedingFieldCount)
);
}
public void testFieldAlias() throws Exception {