From ff0a45f8820480f49277f83fc5556837fe0e8c25 Mon Sep 17 00:00:00 2001 From: markharwood Date: Thu, 16 Jan 2020 10:41:43 +0000 Subject: [PATCH] Fix NPE in PinnedQuery call to DisjunctionMaxScorer. (#51047) (#51064) Fix NPE in PinnedQuery call to DisjunctionMaxScorer. (#51047) Added test and fix that tests for score type. Closes #51034 --- .../lucene/search/CappedScoreQuery.java | 13 ++++++---- .../PinnedQueryBuilderIT.java | 26 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java b/x-pack/plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java index 140e26c5e97..9a8f4fbe38b 100644 --- a/x-pack/plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java +++ b/x-pack/plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java @@ -123,11 +123,14 @@ public final class CappedScoreQuery extends Query { @Override public Scorer get(long leadCost) throws IOException { final Scorer innerScorer = innerScorerSupplier.get(leadCost); - // short-circuit if scores will not need capping - innerScorer.advanceShallow(0); - if (innerScorer.getMaxScore(DocIdSetIterator.NO_MORE_DOCS) <= maxScore) { - return innerScorer; - } + // test scoreMode to avoid NPE - see https://github.com/elastic/elasticsearch/issues/51034 + if (scoreMode == ScoreMode.TOP_SCORES) { + // short-circuit if scores will not need capping + innerScorer.advanceShallow(0); + if (innerScorer.getMaxScore(DocIdSetIterator.NO_MORE_DOCS) <= maxScore) { + return innerScorer; + } + } return new CappedScorer(innerWeight, innerScorer, maxScore); } diff --git a/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java b/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java index f17ff7e2580..2a7d799db52 100644 --- a/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java +++ b/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java @@ -14,6 +14,7 @@ import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.QueryStringQueryBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.SearchHit; import org.elasticsearch.test.ESIntegTestCase; @@ -142,6 +143,31 @@ public class PinnedQueryBuilderIT extends ESIntegTestCase { } + /** + * Test scoring the entire set of documents, which uses a slightly different logic when creating scorers. + */ + public void testExhaustiveScoring() throws Exception { + assertAcked(prepareCreate("test") + .addMapping("_doc", jsonBuilder().startObject().startObject("_doc").startObject("properties") + .startObject("field1").field("analyzer", "whitespace").field("type", "text").endObject() + .startObject("field2").field("analyzer", "whitespace").field("type", "text").endObject() + .endObject().endObject().endObject()) + .setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1))); + + client().prepareIndex("test", "_doc").setId("1").setSource("field1", "foo").get(); + client().prepareIndex("test", "_doc").setId("2").setSource("field1", "foo", "field2", "foo").get(); + + refresh(); + + QueryStringQueryBuilder organicQuery = QueryBuilders.queryStringQuery("foo"); + PinnedQueryBuilder pqb = new PinnedQueryBuilder(organicQuery, "2"); + SearchResponse searchResponse = client().prepareSearch().setQuery(pqb).setTrackTotalHits(true) + .setSearchType(DFS_QUERY_THEN_FETCH).get(); + + long numHits = searchResponse.getHits().getTotalHits().value; + assertThat(numHits, equalTo(2L)); + } + public void testExplain() throws Exception { assertAcked(prepareCreate("test").addMapping("type1", jsonBuilder().startObject().startObject("type1").startObject("properties").startObject("field1")