Browse Source

percolator: Fail indexing percolator queries containing either a has_child or has_parent query.

Closes #2960
Martijn van Groningen 9 years ago
parent
commit
3fcb95b814

+ 19 - 10
modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java

@@ -53,6 +53,8 @@ import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.BoostingQueryBuilder;
 import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
+import org.elasticsearch.index.query.HasChildQueryBuilder;
+import org.elasticsearch.index.query.HasParentQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
@@ -269,7 +271,7 @@ public class PercolatorFieldMapper extends FieldMapper {
 
         XContentParser parser = context.parser();
         QueryBuilder queryBuilder = parseQueryBuilder(queryShardContext.newParseContext(parser), parser.getTokenLocation());
-        verifyRangeQueries(queryBuilder);
+        verifyQuery(queryBuilder);
         // Fetching of terms, shapes and indexed scripts happen during this rewrite:
         queryBuilder = queryBuilder.rewrite(queryShardContext);
 
@@ -356,19 +358,26 @@ public class PercolatorFieldMapper extends FieldMapper {
     }
 
     /**
-     * Fails if a range query with a date range is found based on current time
+     * Fails if a percolator contains an unsupported query. The following queries are not supported:
+     * 1) a range query with a date range based on current time
+     * 2) a has_child query
+     * 3) a has_parent query
      */
-    static void verifyRangeQueries(QueryBuilder queryBuilder) {
+    static void verifyQuery(QueryBuilder queryBuilder) {
         if (queryBuilder instanceof RangeQueryBuilder) {
             RangeQueryBuilder rangeQueryBuilder = (RangeQueryBuilder) queryBuilder;
             if (rangeQueryBuilder.from() instanceof String) {
                 String from = (String) rangeQueryBuilder.from();
                 String to = (String) rangeQueryBuilder.to();
                 if (from.contains("now") || to.contains("now")) {
-                    throw new IllegalArgumentException("Percolator queries containing time range queries based on the " +
-                            "current time are forbidden");
+                    throw new IllegalArgumentException("percolator queries containing time range queries based on the " +
+                            "current time is unsupported");
                 }
             }
+        } else if (queryBuilder instanceof HasChildQueryBuilder) {
+            throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query");
+        } else if (queryBuilder instanceof HasParentQueryBuilder) {
+            throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query");
         } else if (queryBuilder instanceof BoolQueryBuilder) {
             BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
             List<QueryBuilder> clauses = new ArrayList<>();
@@ -377,15 +386,15 @@ public class PercolatorFieldMapper extends FieldMapper {
             clauses.addAll(boolQueryBuilder.mustNot());
             clauses.addAll(boolQueryBuilder.should());
             for (QueryBuilder clause : clauses) {
-                verifyRangeQueries(clause);
+                verifyQuery(clause);
             }
         } else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
-            verifyRangeQueries(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
+            verifyQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
         } else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
-            verifyRangeQueries(((FunctionScoreQueryBuilder) queryBuilder).query());
+            verifyQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
         } else if (queryBuilder instanceof BoostingQueryBuilder) {
-            verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).negativeQuery());
-            verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).positiveQuery());
+            verifyQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
+            verifyQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
         }
     }
 

+ 22 - 11
modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

@@ -32,6 +32,7 @@ import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.search.TermRangeQuery;
+import org.apache.lucene.search.join.ScoreMode;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -49,6 +50,8 @@ import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.BoostingQueryBuilder;
 import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
+import org.elasticsearch.index.query.HasChildQueryBuilder;
+import org.elasticsearch.index.query.HasParentQueryBuilder;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
@@ -435,23 +438,31 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
         assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
     }
 
-    public void testVerifyRangeQueries() {
+    public void testUnsupportedQueries() {
         RangeQueryBuilder rangeQuery1 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("2017-01-01||/D");
         RangeQueryBuilder rangeQuery2 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("now");
-        PercolatorFieldMapper.verifyRangeQueries(rangeQuery1);
-        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyRangeQueries(rangeQuery2));
-        PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery1));
+        PercolatorFieldMapper.verifyQuery(rangeQuery1);
+        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(rangeQuery2));
+        PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery1));
         expectThrows(IllegalArgumentException.class, () ->
-                PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery2)));
-        PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder((rangeQuery1)));
+                PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery2)));
+        PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder((rangeQuery1)));
         expectThrows(IllegalArgumentException.class, () ->
-                PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder(rangeQuery2)));
-        PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder()));
+                PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder(rangeQuery2)));
+        PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder()));
         expectThrows(IllegalArgumentException.class, () ->
-                PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder())));
-        PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder()));
+                PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder())));
+        PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder()));
         expectThrows(IllegalArgumentException.class, () ->
-                PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder())));
+                PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder())));
+
+        HasChildQueryBuilder hasChildQuery = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None);
+        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasChildQuery));
+        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasChildQuery)));
+
+        HasParentQueryBuilder hasParentQuery = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false);
+        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasParentQuery));
+        expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasParentQuery)));
     }
 
     private void assertQueryBuilder(BytesRef actual, QueryBuilder expected) throws IOException {

+ 5 - 6
modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java

@@ -1777,16 +1777,15 @@ public class PercolatorIT extends ESIntegTestCase {
         assertThat(response1.getMatches()[0].getId().string(), equalTo("1"));
     }
 
-    public void testParentChild() throws Exception {
-        // We don't fail p/c queries, but those queries are unusable because only a single document can be provided in
-        // the percolate api
-
+    public void testFailParentChild() throws Exception {
         assertAcked(prepareCreate(INDEX_NAME)
                 .addMapping(TYPE_NAME, "query", "type=percolator")
                 .addMapping("child", "_parent", "type=parent").addMapping("parent"));
-        client().prepareIndex(INDEX_NAME, TYPE_NAME, "1")
+        Exception e = expectThrows(MapperParsingException.class, () -> client().prepareIndex(INDEX_NAME, TYPE_NAME, "1")
                 .setSource(jsonBuilder().startObject().field("query", hasChildQuery("child", matchAllQuery(), ScoreMode.None)).endObject())
-                .execute().actionGet();
+                .get());
+        assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getCause().getMessage(), equalTo("the [has_child] query is unsupported inside a percolator query"));
     }
 
     public void testPercolateDocumentWithParentField() throws Exception {