Skip to content

Commit 3fcb95b

Browse files
martijnvgmartijnvg
committed
percolator: Fail indexing percolator queries containing either a has_child or has_parent query.
Closes #2960
1 parent a68083f commit 3fcb95b

File tree

3 files changed

+46
-27
lines changed

3 files changed

+46
-27
lines changed

‎modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
import org.elasticsearch.index.query.BoolQueryBuilder;
5454
import org.elasticsearch.index.query.BoostingQueryBuilder;
5555
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
56+
import org.elasticsearch.index.query.HasChildQueryBuilder;
57+
import org.elasticsearch.index.query.HasParentQueryBuilder;
5658
import org.elasticsearch.index.query.QueryBuilder;
5759
import org.elasticsearch.index.query.QueryParseContext;
5860
import org.elasticsearch.index.query.QueryShardContext;
@@ -269,7 +271,7 @@ public Mapper parse(ParseContext context) throws IOException {
269271

270272
XContentParser parser = context.parser();
271273
QueryBuilder queryBuilder = parseQueryBuilder(queryShardContext.newParseContext(parser), parser.getTokenLocation());
272-
verifyRangeQueries(queryBuilder);
274+
verifyQuery(queryBuilder);
273275
// Fetching of terms, shapes and indexed scripts happen during this rewrite:
274276
queryBuilder = queryBuilder.rewrite(queryShardContext);
275277

@@ -356,19 +358,26 @@ protected String contentType() {
356358
}
357359

358360
/**
359-
* Fails if a range query with a date range is found based on current time
361+
* Fails if a percolator contains an unsupported query. The following queries are not supported:
362+
* 1) a range query with a date range based on current time
363+
* 2) a has_child query
364+
* 3) a has_parent query
360365
*/
361-
static void verifyRangeQueries(QueryBuilder queryBuilder) {
366+
static void verifyQuery(QueryBuilder queryBuilder) {
362367
if (queryBuilder instanceof RangeQueryBuilder) {
363368
RangeQueryBuilder rangeQueryBuilder = (RangeQueryBuilder) queryBuilder;
364369
if (rangeQueryBuilder.from() instanceof String) {
365370
String from = (String) rangeQueryBuilder.from();
366371
String to = (String) rangeQueryBuilder.to();
367372
if (from.contains("now") || to.contains("now")) {
368-
throw new IllegalArgumentException("Percolator queries containing time range queries based on the " +
369-
"current time are forbidden");
373+
throw new IllegalArgumentException("percolator queries containing time range queries based on the " +
374+
"current time is unsupported");
370375
}
371376
}
377+
} else if (queryBuilder instanceof HasChildQueryBuilder) {
378+
throw new IllegalArgumentException("the [has_child] query is unsupported inside a percolator query");
379+
} else if (queryBuilder instanceof HasParentQueryBuilder) {
380+
throw new IllegalArgumentException("the [has_parent] query is unsupported inside a percolator query");
372381
} else if (queryBuilder instanceof BoolQueryBuilder) {
373382
BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) queryBuilder;
374383
List<QueryBuilder> clauses = new ArrayList<>();
@@ -377,15 +386,15 @@ static void verifyRangeQueries(QueryBuilder queryBuilder) {
377386
clauses.addAll(boolQueryBuilder.mustNot());
378387
clauses.addAll(boolQueryBuilder.should());
379388
for (QueryBuilder clause : clauses) {
380-
verifyRangeQueries(clause);
389+
verifyQuery(clause);
381390
}
382391
} else if (queryBuilder instanceof ConstantScoreQueryBuilder) {
383-
verifyRangeQueries(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
392+
verifyQuery(((ConstantScoreQueryBuilder) queryBuilder).innerQuery());
384393
} else if (queryBuilder instanceof FunctionScoreQueryBuilder) {
385-
verifyRangeQueries(((FunctionScoreQueryBuilder) queryBuilder).query());
394+
verifyQuery(((FunctionScoreQueryBuilder) queryBuilder).query());
386395
} else if (queryBuilder instanceof BoostingQueryBuilder) {
387-
verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).negativeQuery());
388-
verifyRangeQueries(((BoostingQueryBuilder) queryBuilder).positiveQuery());
396+
verifyQuery(((BoostingQueryBuilder) queryBuilder).negativeQuery());
397+
verifyQuery(((BoostingQueryBuilder) queryBuilder).positiveQuery());
389398
}
390399
}
391400

‎modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.lucene.search.PhraseQuery;
3333
import org.apache.lucene.search.TermQuery;
3434
import org.apache.lucene.search.TermRangeQuery;
35+
import org.apache.lucene.search.join.ScoreMode;
3536
import org.apache.lucene.util.BytesRef;
3637
import org.elasticsearch.Version;
3738
import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -49,6 +50,8 @@
4950
import org.elasticsearch.index.query.BoolQueryBuilder;
5051
import org.elasticsearch.index.query.BoostingQueryBuilder;
5152
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
53+
import org.elasticsearch.index.query.HasChildQueryBuilder;
54+
import org.elasticsearch.index.query.HasParentQueryBuilder;
5255
import org.elasticsearch.index.query.MatchAllQueryBuilder;
5356
import org.elasticsearch.index.query.QueryBuilder;
5457
import org.elasticsearch.index.query.QueryParseContext;
@@ -435,23 +438,31 @@ public void testRangeQueryWithNowRangeIsForbidden() throws Exception {
435438
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
436439
}
437440

438-
public void testVerifyRangeQueries() {
441+
public void testUnsupportedQueries() {
439442
RangeQueryBuilder rangeQuery1 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("2017-01-01||/D");
440443
RangeQueryBuilder rangeQuery2 = new RangeQueryBuilder("field").from("2016-01-01||/D").to("now");
441-
PercolatorFieldMapper.verifyRangeQueries(rangeQuery1);
442-
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyRangeQueries(rangeQuery2));
443-
PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery1));
444+
PercolatorFieldMapper.verifyQuery(rangeQuery1);
445+
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(rangeQuery2));
446+
PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery1));
444447
expectThrows(IllegalArgumentException.class, () ->
445-
PercolatorFieldMapper.verifyRangeQueries(new BoolQueryBuilder().must(rangeQuery2)));
446-
PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder((rangeQuery1)));
448+
PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(rangeQuery2)));
449+
PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder((rangeQuery1)));
447450
expectThrows(IllegalArgumentException.class, () ->
448-
PercolatorFieldMapper.verifyRangeQueries(new ConstantScoreQueryBuilder(rangeQuery2)));
449-
PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder()));
451+
PercolatorFieldMapper.verifyQuery(new ConstantScoreQueryBuilder(rangeQuery2)));
452+
PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery1, new MatchAllQueryBuilder()));
450453
expectThrows(IllegalArgumentException.class, () ->
451-
PercolatorFieldMapper.verifyRangeQueries(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder())));
452-
PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder()));
454+
PercolatorFieldMapper.verifyQuery(new BoostingQueryBuilder(rangeQuery2, new MatchAllQueryBuilder())));
455+
PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery1, new RandomScoreFunctionBuilder()));
453456
expectThrows(IllegalArgumentException.class, () ->
454-
PercolatorFieldMapper.verifyRangeQueries(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder())));
457+
PercolatorFieldMapper.verifyQuery(new FunctionScoreQueryBuilder(rangeQuery2, new RandomScoreFunctionBuilder())));
458+
459+
HasChildQueryBuilder hasChildQuery = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None);
460+
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasChildQuery));
461+
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasChildQuery)));
462+
463+
HasParentQueryBuilder hasParentQuery = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false);
464+
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasParentQuery));
465+
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasParentQuery)));
455466
}
456467

457468
private void assertQueryBuilder(BytesRef actual, QueryBuilder expected) throws IOException {

‎modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,16 +1777,15 @@ public void testGeoShapeWithMapUnmappedFieldAsString() throws Exception {
17771777
assertThat(response1.getMatches()[0].getId().string(), equalTo("1"));
17781778
}
17791779

1780-
public void testParentChild() throws Exception {
1781-
// We don't fail p/c queries, but those queries are unusable because only a single document can be provided in
1782-
// the percolate api
1783-
1780+
public void testFailParentChild() throws Exception {
17841781
assertAcked(prepareCreate(INDEX_NAME)
17851782
.addMapping(TYPE_NAME, "query", "type=percolator")
17861783
.addMapping("child", "_parent", "type=parent").addMapping("parent"));
1787-
client().prepareIndex(INDEX_NAME, TYPE_NAME, "1")
1784+
Exception e = expectThrows(MapperParsingException.class, () -> client().prepareIndex(INDEX_NAME, TYPE_NAME, "1")
17881785
.setSource(jsonBuilder().startObject().field("query", hasChildQuery("child", matchAllQuery(), ScoreMode.None)).endObject())
1789-
.execute().actionGet();
1786+
.get());
1787+
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
1788+
assertThat(e.getCause().getMessage(), equalTo("the [has_child] query is unsupported inside a percolator query"));
17901789
}
17911790

17921791
public void testPercolateDocumentWithParentField() throws Exception {

0 commit comments

Comments
 (0)