Browse Source

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
Christoph Büscher 7 years ago
parent
commit
113af7996c

+ 3 - 1
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` <<search-settings>>
+which defaults to 1024.
 
 [[multi-match-types]]
 [float]

+ 5 - 2
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` <<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

+ 5 - 2
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` <<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

+ 8 - 6
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<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());
         }
     }
 }

+ 23 - 5
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<IndexRequestBuilder> 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 {

+ 24 - 6
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<Class<? extends Plugin>> 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 {