From 113af7996cb8a9fd84371834cccfe4a5384ab4e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 8 Nov 2018 17:04:40 +0100 Subject: [PATCH] Make limit on number of expanded fields configurable (#35284) 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/multi-match-query.asciidoc | 4 ++- .../query-dsl/query-string-query.asciidoc | 7 +++-- .../simple-query-string-query.asciidoc | 7 +++-- .../index/search/QueryParserHelper.java | 14 +++++---- .../search/query/QueryStringIT.java | 28 +++++++++++++---- .../search/query/SimpleQueryStringIT.java | 30 +++++++++++++++---- 6 files changed, 68 insertions(+), 22 deletions(-) diff --git a/docs/reference/query-dsl/multi-match-query.asciidoc b/docs/reference/query-dsl/multi-match-query.asciidoc index 296689db289..512eee4900b 100644 --- a/docs/reference/query-dsl/multi-match-query.asciidoc +++ b/docs/reference/query-dsl/multi-match-query.asciidoc @@ -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` <> +which defaults to 1024. [[multi-match-types]] [float] diff --git a/docs/reference/query-dsl/query-string-query.asciidoc b/docs/reference/query-dsl/query-string-query.asciidoc index 08465278a67..f80cd2e8e93 100644 --- a/docs/reference/query-dsl/query-string-query.asciidoc +++ b/docs/reference/query-dsl/query-string-query.asciidoc @@ -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` <> +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 99fbc131c1b..4ec97ff49bd 100644 --- a/docs/reference/query-dsl/simple-query-string-query.asciidoc +++ b/docs/reference/query-dsl/simple-query-string-query.asciidoc @@ -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` <> +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 d3bac583eac..399e610a54e 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.common.regex.Regex; 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 final class QueryParserHelper { !multiField, !allField, fieldSuffix); resolvedFields.putAll(fieldMap); } - checkForTooManyFields(resolvedFields); + checkForTooManyFields(resolvedFields, context); return resolvedFields; } @@ -141,7 +142,7 @@ public final class QueryParserHelper { 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 final class QueryParserHelper { } 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 8a09e5a919a..458ec86af59 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.Operator; 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 @@ import static org.hamcrest.Matchers.equalTo; 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"); @@ -58,6 +67,14 @@ public class QueryStringIT extends ESIntegTestCase { 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 class QueryStringIT extends ESIntegTestCase { 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,10 +278,11 @@ public class QueryStringIT extends ESIntegTestCase { 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, () -> { @@ -272,11 +290,11 @@ public class QueryStringIT extends ESIntegTestCase { 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 { 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 598f5625588..197f34cf879 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.AnalysisPlugin; 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 @@ import static org.hamcrest.Matchers.equalTo; * 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> nodePlugins() { return Collections.singletonList(MockAnalysisPlugin.class); @@ -553,13 +571,12 @@ public class SimpleQueryStringIT extends ESIntegTestCase { 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 @@ -567,10 +584,11 @@ public class SimpleQueryStringIT extends ESIntegTestCase { 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, () -> { @@ -578,11 +596,11 @@ public class SimpleQueryStringIT extends ESIntegTestCase { 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 {