From 280a2f55d64b64663058e216273f8047dd01e425 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 7 Jun 2018 13:37:29 +0200 Subject: [PATCH] Fix random failure on SearchQueryIT#testTermExpansionExceptionOnSpanFailure This change moves an integration test that relies on setting the value of a static variable (boolean max clause count) to an unit test where we are sure that the same jvm is used to access the static variable. --- .../query/SpanMultiTermQueryBuilder.java | 71 +++++++++---------- .../query/SpanMultiTermQueryBuilderTests.java | 39 ++++++++-- .../search/query/SearchQueryIT.java | 27 ------- 3 files changed, 70 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java index 0576a68a85b..9aca0c0fc0d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java @@ -137,47 +137,42 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder> delegate = new ScoringRewrite>() { - - @Override - protected List getTopLevelBuilder() { - return new ArrayList(); - } - - @Override - protected Query build(List builder) { - return new SpanOrQuery((SpanQuery[]) builder.toArray(new SpanQuery[builder.size()])); - } - - @Override - protected void checkMaxClauseCount(int count) { - if (count > maxExpansions) { - throw new ElasticsearchException("[" + multiTermQuery.toString() + " ] " + - "exceeds maxClauseCount [ Boolean maxClauseCount is set to " + BooleanQuery.getMaxClauseCount() + "]"); + final MultiTermQuery.RewriteMethod delegate = new ScoringRewrite>() { + @Override + protected List getTopLevelBuilder() { + return new ArrayList(); } - } - @Override - protected void addClause(List topLevel, Term term, int docCount, float boost, TermContext states) { - SpanTermQuery q = new SpanTermQuery(term, states); - topLevel.add(q); - } - }; + @Override + protected Query build(List builder) { + return new SpanOrQuery((SpanQuery[]) builder.toArray(new SpanQuery[builder.size()])); + } + + @Override + protected void checkMaxClauseCount(int count) { + if (count > maxExpansions) { + throw new RuntimeException("[" + query.toString() + " ] " + + "exceeds maxClauseCount [ Boolean maxClauseCount is set to " + BooleanQuery.getMaxClauseCount() + "]"); + } + } + + @Override + protected void addClause(List topLevel, Term term, int docCount, float boost, TermContext states) { + SpanTermQuery q = new SpanTermQuery(term, states); + topLevel.add(q); + } + }; + return (SpanQuery) delegate.rewrite(reader, query); + } } @Override @@ -222,6 +217,7 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder(prefixQuery); } else { String origFieldName = ((PrefixQueryBuilder) multiTermQueryBuilder).fieldName(); @@ -240,9 +236,12 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder((MultiTermQuery) subQuery); - if (((MultiTermQuery) subQuery).getRewriteMethod() instanceof TopTermsRewrite == false) { - ((SpanMultiTermQueryWrapper) spanQuery).setRewriteMethod(new - TopTermSpanBooleanQueryRewriteWithMaxClause(BooleanQuery.getMaxClauseCount())); + } + if (subQuery instanceof MultiTermQuery) { + MultiTermQuery multiTermQuery = (MultiTermQuery) subQuery; + SpanMultiTermQueryWrapper wrapper = (SpanMultiTermQueryWrapper) spanQuery; + if (multiTermQuery.getRewriteMethod() instanceof TopTermsRewrite == false) { + wrapper.setRewriteMethod(new TopTermSpanBooleanQueryRewriteWithMaxClause()); } } if (boost != AbstractQueryBuilder.DEFAULT_BOOST) { diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java index fae66dd68bc..c93df5b7519 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTests.java @@ -19,7 +19,13 @@ package org.elasticsearch.index.query; +import org.apache.lucene.analysis.core.WhitespaceAnalyzer; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.TextField; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.RandomIndexWriter; import org.apache.lucene.index.Term; +import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.PrefixQuery; @@ -30,6 +36,7 @@ import org.apache.lucene.search.spans.SpanBoostQuery; import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper; import org.apache.lucene.search.spans.SpanQuery; import org.apache.lucene.search.spans.SpanTermQuery; +import org.apache.lucene.store.Directory; import org.elasticsearch.Version; import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; @@ -41,6 +48,7 @@ import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; +import static java.util.Collections.singleton; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; @@ -251,14 +259,37 @@ public class SpanMultiTermQueryBuilderTests extends AbstractQueryTestCase query.rewrite(reader)); + assertThat(exc.getMessage(), containsString("maxClauseCount")); + + } finally { + BooleanQuery.setMaxClauseCount(origBoolMaxClauseCount); + } + } + } + } } public void testTopNMultiTermsRewriteInsideSpan() throws Exception { - - Query query = QueryBuilders.spanMultiTermQueryBuilder(QueryBuilders.prefixQuery("foo", "b").rewrite - ("top_terms_boost_2000")). - toQuery(createShardContext()); + Query query = QueryBuilders.spanMultiTermQueryBuilder( + QueryBuilders.prefixQuery("foo", "b").rewrite("top_terms_boost_2000") + ).toQuery(createShardContext()); if (query instanceof SpanBoostQuery) { query = ((SpanBoostQuery)query).getQuery(); diff --git a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java index 5dd669119e5..c3df8d778a7 100644 --- a/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -1825,31 +1825,4 @@ public class SearchQueryIT extends ESIntegTestCase { SearchResponse searchResponse = client().prepareSearch("test").setQuery(range).get(); assertHitCount(searchResponse, 1); } - - public void testTermExpansionExceptionOnSpanFailure() throws ExecutionException, InterruptedException { - Settings.Builder builder = Settings.builder(); - builder.put(SETTING_NUMBER_OF_SHARDS, 1).build(); - - createIndex("test", builder.build()); - ArrayList reqs = new ArrayList<>(); - int origBoolMaxClauseCount = BooleanQuery.getMaxClauseCount(); - try { - BooleanQuery.setMaxClauseCount(2); - for (int i = 0; i < BooleanQuery.getMaxClauseCount() + 1; i++) { - reqs.add(client().prepareIndex("test", "_doc", Integer.toString(i)).setSource("body", "foo" + - Integer.toString(i) + " bar baz")); - } - indexRandom(true, false, reqs); - - QueryBuilder queryBuilder = new SpanNearQueryBuilder(new SpanMultiTermQueryBuilder(QueryBuilders.wildcardQuery - ("body", "f*")), 0).addClause(new SpanTermQueryBuilder("body", "bar")); - - expectThrows(ElasticsearchException.class, () -> - client().prepareSearch().setIndices("test").setQuery(queryBuilder).get()); - } finally { - BooleanQuery.setMaxClauseCount(origBoolMaxClauseCount); - } - - } - }