From 3fcb95b8140e576b604fb9f4fae5a1733016a7c5 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 30 Aug 2016 11:29:55 +0200 Subject: [PATCH] percolator: Fail indexing percolator queries containing either a has_child or has_parent query. Closes #2960 --- .../percolator/PercolatorFieldMapper.java | 29 ++++++++++------ .../PercolatorFieldMapperTests.java | 33 ++++++++++++------- .../percolator/PercolatorIT.java | 11 +++---- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 32633775167..82e1d42ba23 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/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 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()); } } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index 364b90ef70a..df1e6ea6f8c 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/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 { diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java index e4a10ce04a0..7d10b831bc8 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java +++ b/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 {