Skip to content

Commit

Permalink
Make limit on number of expanded fields configurable (#35284)
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 #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 #34778
  • Loading branch information
Christoph Büscher authored Nov 8, 2018
1 parent 55c6481 commit 113af79
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 22 deletions.
4 changes: 3 additions & 1 deletion docs/reference/query-dsl/multi-match-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ index settings, which in turn defaults to `*`. `*` extracts all fields in the ma
are eligible to term queries and filters the metadata fields. All extracted fields are then
combined to build a query.

WARNING: There is a limit of no more than 1024 fields being queried at once.
WARNING: There is a limit on the number of fields that can be queried
at once. It is defined by the `indices.query.bool.max_clause_count` <<search-settings>>
which defaults to 1024.

[[multi-match-types]]
[float]
Expand Down
7 changes: 5 additions & 2 deletions docs/reference/query-dsl/query-string-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ 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.

WARNING: There is a limit on the number of fields that can be queried
at once. It 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
7 changes: 5 additions & 2 deletions docs/reference/query-dsl/simple-query-string-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ 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.

WARNING: There is a limit on the number of fields that can be queried
at once. It 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
Original file line number Diff line number Diff line change
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());
}
}
}
Original file line number Diff line number Diff line change
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(50, 100);
}

@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,30 +270,31 @@ 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();
client().prepareIndex("toomanyfields", "type1", "1").setSource("field1", "foo bar baz").get();
refresh();

Exception e = expectThrows(Exception.class, () -> {
QueryStringQueryBuilder qb = queryStringQuery("bar");
if (randomBoolean()) {
qb.useAllFields(true);
}
logger.info("--> using {}", qb);
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
Original file line number Diff line number Diff line change
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(50, 100);
}

@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 @@ -553,36 +571,36 @@ public void testAllFieldsWithSpecifiedLeniency() throws IOException {
containsString("NumberFormatException[For input string: \"foo123\"]"));
}


public void testLimitOnExpandedFields() throws Exception {
XContentBuilder builder = jsonBuilder();
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();
client().prepareIndex("toomanyfields", "type1", "1").setSource("field1", "foo bar baz").get();
refresh();

Exception e = expectThrows(Exception.class, () -> {
SimpleQueryStringBuilder qb = simpleQueryStringQuery("bar");
if (randomBoolean()) {
qb.useAllFields(true);
}
logger.info("--> using {}", qb);
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 113af79

Please sign in to comment.