From 96e915e80dba78459c9d1a4d098e1b216b75e032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 5 Nov 2018 21:06:47 +0100 Subject: [PATCH] Make limit on number of expanded fields configurable 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 --- .../query-dsl/query-string-query.asciidoc | 5 ++-- .../simple-query-string-query.asciidoc | 5 ++-- .../index/search/QueryParserHelper.java | 14 +++++----- .../search/query/QueryStringIT.java | 25 +++++++++++++++--- .../search/query/SimpleQueryStringIT.java | 26 ++++++++++++++++--- 5 files changed, 59 insertions(+), 16 deletions(-) diff --git a/docs/reference/query-dsl/query-string-query.asciidoc b/docs/reference/query-dsl/query-string-query.asciidoc index 08465278a67dd..5aa18ad9a359c 100644 --- a/docs/reference/query-dsl/query-string-query.asciidoc +++ b/docs/reference/query-dsl/query-string-query.asciidoc @@ -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` <> 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 diff --git a/docs/reference/query-dsl/simple-query-string-query.asciidoc b/docs/reference/query-dsl/simple-query-string-query.asciidoc index 99fbc131c1be3..4c7dbb60e9519 100644 --- a/docs/reference/query-dsl/simple-query-string-query.asciidoc +++ b/docs/reference/query-dsl/simple-query-string-query.asciidoc @@ -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` <> +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 diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java index d3bac583eac68..399e610a54edf 100644 --- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java +++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java @@ -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; @@ -85,7 +86,7 @@ public static Map resolveMappingFields(QueryShardContext context, !multiField, !allField, fieldSuffix); resolvedFields.putAll(fieldMap); } - checkForTooManyFields(resolvedFields); + checkForTooManyFields(resolvedFields, context); return resolvedFields; } @@ -141,7 +142,7 @@ public static Map 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) { @@ -150,13 +151,14 @@ public static Map resolveMappingField(QueryShardContext context, } fields.put(fieldName, weight); } - checkForTooManyFields(fields); + checkForTooManyFields(fields, context); return fields; } - private static void checkForTooManyFields(Map fields) { - if (fields.size() > 1024) { - throw new IllegalArgumentException("field expansion matches too many fields, limit: 1024, got: " + fields.size()); + private static void checkForTooManyFields(Map 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()); } } } diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java index 8a09e5a919a8d..2233619c14b3b 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java @@ -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; @@ -51,6 +53,13 @@ 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"); @@ -58,6 +67,14 @@ public void setup() throws Exception { 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 reqs = new ArrayList<>(); reqs.add(client().prepareIndex("test", "_doc", "1").setSource("f1", "foo bar baz")); @@ -253,7 +270,7 @@ 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 @@ -261,7 +278,8 @@ public void testLimitOnExpandedFields() throws Exception { 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(); @@ -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 { diff --git a/server/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java index 598f562558865..19ed36592a5eb 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java @@ -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; @@ -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> nodePlugins() { return Collections.singletonList(MockAnalysisPlugin.class); @@ -559,7 +577,7 @@ 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 @@ -567,7 +585,8 @@ public void testLimitOnExpandedFields() throws Exception { 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(); @@ -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 {