Fix NPE in PinnedQuery call to DisjunctionMaxScorer. (elastic#51047)

Fix NPE in PinnedQuery call to DisjunctionMaxScorer. Added test and fix that tests for score type. Closes elastic#51034
markharwood · Jan 16, 2020 · 13e64d8 · 13e64d8
1 parent 45d7bdc
commit 13e64d8
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 5 deletions.
diff --git a/...plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java b/...plugin/search-business-rules/src/main/java/org/apache/lucene/search/CappedScoreQuery.java
@@ -123,11 +123,14 @@ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOExcepti
                         @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/...rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java b/...rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/PinnedQueryBuilderIT.java
@@ -14,6 +14,7 @@
 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 void testPinnedPromotions() throws Exception {
 
     }
 
+    /**
+     * Test scoring the entire set of documents, which uses a slightly different logic when creating scorers.
+     */
+    public void testExhaustiveScoring() throws Exception {
+        assertAcked(prepareCreate("test")
+                .setMapping(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").setId("1").setSource("field1", "foo").get();
+        client().prepareIndex("test").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")