Skip to content

Commit

Permalink
Make limit on number of expanded fields configurable
Browse files Browse the repository at this point in the history 
Currently we introduced a hard limit of 1024 to the number of fields a query can
be expanded to in elastic#26541. Instead of using a hard limit, we should make this
configurable. This change removes the hard limit check and uses the existing
`max_clause_count` setting instead.

Closes elastic#34778
  • Loading branch information
Christoph Büscher committed Nov 6, 2018
1 parent 85f8458 commit 96e915e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 16 deletions.
5 changes: 3 additions & 2 deletions docs/reference/query-dsl/query-string-query.asciidoc
View file
Expand Up @@ -60,8 +60,9 @@ The `query_string` top level parameters include:
specified. Defaults to the `index.query.default_field` index settings, which in
turn defaults to `*`. `*` extracts all fields in the mapping that are eligible
to term queries and filters the metadata fields. All extracted fields are then
combined to build a query when no prefix field is provided. There is a limit of
no more than 1024 fields being queried at once.
combined to build a query when no prefix field is provided. There is a limit on
the number of fields that can be queried at once which is defined by the
`indices.query.bool.max_clause_count` <<search-settings>> which defaults to 1024.

|`default_operator` |The default operator used if no explicit operator
is specified. For example, with a default operator of `OR`, the query
Expand Down
5 changes: 3 additions & 2 deletions docs/reference/query-dsl/simple-query-string-query.asciidoc
View file
Expand Up @@ -31,8 +31,9 @@ The `simple_query_string` top level parameters include:
|`fields` |The fields to perform the parsed query against. Defaults to the
`index.query.default_field` index settings, which in turn defaults to `*`. `*`
extracts all fields in the mapping that are eligible to term queries and filters
the metadata fields. There is a limit of no more than 1024 fields being queried
at once.
the metadata fields. There is a limit on the number of fields that can be queried
at once which is defined by the `indices.query.bool.max_clause_count` <<search-settings>>
which defaults to 1024.

|`default_operator` |The default operator used if no explicit operator
is specified. For example, with a default operator of `OR`, the query
Expand Down
14 changes: 8 additions & 6 deletions server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
View file
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.search.SearchModule;

import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -85,7 +86,7 @@ public static Map<String, Float> resolveMappingFields(QueryShardContext context,
!multiField, !allField, fieldSuffix);
resolvedFields.putAll(fieldMap);
}
checkForTooManyFields(resolvedFields);
checkForTooManyFields(resolvedFields, context);
return resolvedFields;
}

Expand Down Expand Up @@ -141,7 +142,7 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
if (acceptAllTypes == false) {
try {
fieldType.termQuery("", context);
} catch (QueryShardException |UnsupportedOperationException e) {
} catch (QueryShardException | UnsupportedOperationException e) {
// field type is never searchable with term queries (eg. geo point): ignore
continue;
} catch (IllegalArgumentException |ElasticsearchParseException e) {
Expand All @@ -150,13 +151,14 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
}
fields.put(fieldName, weight);
}
checkForTooManyFields(fields);
checkForTooManyFields(fields, context);
return fields;
}

private static void checkForTooManyFields(Map<String, Float> fields) {
if (fields.size() > 1024) {
throw new IllegalArgumentException("field expansion matches too many fields, limit: 1024, got: " + fields.size());
private static void checkForTooManyFields(Map<String, Float> fields, QueryShardContext context) {
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());
}
}
}
25 changes: 22 additions & 3 deletions server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
View file
Expand Up @@ -30,8 +30,10 @@
import org.elasticsearch.index.query.QueryStringQueryBuilder;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.test.ESIntegTestCase;
import org.junit.Before;
import org.junit.BeforeClass;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -51,13 +53,28 @@

public class QueryStringIT extends ESIntegTestCase {

private static int CLUSTER_MAX_CLAUSE_COUNT;

@BeforeClass
public static void createRandomClusterSetting() {
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(500, 1500);
}

@Before
public void setup() throws Exception {
String indexBody = copyToStringFromClasspath("/org/elasticsearch/search/query/all-query-index.json");
prepareCreate("test").setSource(indexBody, XContentType.JSON).get();
ensureGreen("test");
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.build();
}

public void testBasicAllQuery() throws Exception {
List<IndexRequestBuilder> reqs = new ArrayList<>();
reqs.add(client().prepareIndex("test", "_doc", "1").setSource("f1", "foo bar baz"));
Expand Down Expand Up @@ -253,15 +270,16 @@ public void testLimitOnExpandedFields() throws Exception {
builder.startObject();
builder.startObject("type1");
builder.startObject("properties");
for (int i = 0; i < 1025; i++) {
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(), 1200))
.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("field171", "foo bar baz").get();
Expand All @@ -276,7 +294,8 @@ public void testLimitOnExpandedFields() throws Exception {
client().prepareSearch("toomanyfields").setQuery(qb).get();
});
assertThat(ExceptionsHelper.detailedMessage(e),
containsString("field expansion matches too many fields, limit: 1024, got: 1025"));
containsString("field expansion matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT + ", got: "
+ (CLUSTER_MAX_CLAUSE_COUNT + 1)));
}

public void testFieldAlias() throws Exception {
Expand Down
26 changes: 23 additions & 3 deletions server/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java
View file
Expand Up @@ -42,8 +42,10 @@
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.test.ESIntegTestCase;
import org.junit.BeforeClass;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -76,6 +78,22 @@
* Tests for the {@code simple_query_string} query
*/
public class SimpleQueryStringIT extends ESIntegTestCase {

private static int CLUSTER_MAX_CLAUSE_COUNT;

@BeforeClass
public static void createRandomClusterSetting() {
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(500, 1500);
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.build();
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singletonList(MockAnalysisPlugin.class);
Expand Down Expand Up @@ -559,15 +577,16 @@ public void testLimitOnExpandedFields() throws Exception {
builder.startObject();
builder.startObject("type1");
builder.startObject("properties");
for (int i = 0; i < 1025; i++) {
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(), 1200))
.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("field171", "foo bar baz").get();
Expand All @@ -582,7 +601,8 @@ public void testLimitOnExpandedFields() throws Exception {
client().prepareSearch("toomanyfields").setQuery(qb).get();
});
assertThat(ExceptionsHelper.detailedMessage(e),
containsString("field expansion matches too many fields, limit: 1024, got: 1025"));
containsString("field expansion matches too many fields, limit: " + CLUSTER_MAX_CLAUSE_COUNT + ", got: "
+ (CLUSTER_MAX_CLAUSE_COUNT + 1)));
}

public void testFieldAlias() throws Exception {
Expand Down

0 comments on commit 96e915e

Please sign in to comment.