Set maxScore for empty TopDocs to Nan rather than 0 (#32938)

javanna · web-flow · commit 393eec148217 · 2018-08-22T17:23:54.000+02:00
We used to set `maxScore` to `0` within `TopDocs` in situations where there is really no score as the size was set to `0` and scores were not even tracked. In such scenarios, `Float.Nan` is more appropriate, which gets converted to `max_score: null` on the REST layer. That's also more consistent with lucene which set `maxScore` to `Float.Nan` when merging empty `TopDocs` (see `TopDocs#merge`).
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SearchIT.java
@@ -256,7 +256,7 @@ public void testSearchWithTermsAgg() throws IOException {
         assertNull(searchResponse.getSuggest());
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(0, searchResponse.getHits().getHits().length);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         Terms termsAgg = searchResponse.getAggregations().get("agg1");
         assertEquals("agg1", termsAgg.getName());
         assertEquals(2, termsAgg.getBuckets().size());
@@ -293,7 +293,7 @@ public void testSearchWithRangeAgg() throws IOException {
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(5, searchResponse.getHits().totalHits);
         assertEquals(0, searchResponse.getHits().getHits().length);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         Range rangeAgg = searchResponse.getAggregations().get("agg1");
         assertEquals("agg1", rangeAgg.getName());
         assertEquals(2, rangeAgg.getBuckets().size());
@@ -323,7 +323,7 @@ public void testSearchWithTermsAndRangeAgg() throws IOException {
         assertNull(searchResponse.getSuggest());
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(0, searchResponse.getHits().getHits().length);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         Terms termsAgg = searchResponse.getAggregations().get("agg1");
         assertEquals("agg1", termsAgg.getName());
         assertEquals(2, termsAgg.getBuckets().size());
@@ -375,7 +375,7 @@ public void testSearchWithMatrixStats() throws IOException {
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(5, searchResponse.getHits().totalHits);
         assertEquals(0, searchResponse.getHits().getHits().length);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         assertEquals(1, searchResponse.getAggregations().asList().size());
         MatrixStats matrixStats = searchResponse.getAggregations().get("agg1");
         assertEquals(5, matrixStats.getFieldCount("num"));
@@ -474,7 +474,7 @@ public void testSearchWithParentJoin() throws IOException {
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(3, searchResponse.getHits().totalHits);
         assertEquals(0, searchResponse.getHits().getHits().length);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         assertEquals(1, searchResponse.getAggregations().asList().size());
         Terms terms = searchResponse.getAggregations().get("top-tags");
         assertEquals(0, terms.getDocCountError());
@@ -513,7 +513,7 @@ public void testSearchWithSuggest() throws IOException {
         assertNull(searchResponse.getAggregations());
         assertEquals(Collections.emptyMap(), searchResponse.getProfileResults());
         assertEquals(0, searchResponse.getHits().totalHits);
-        assertEquals(0f, searchResponse.getHits().getMaxScore(), 0f);
+        assertEquals(Float.NaN, searchResponse.getHits().getMaxScore(), 0f);
         assertEquals(0, searchResponse.getHits().getHits().length);
         assertEquals(1, searchResponse.getSuggest().size());
 
diff --git a/docs/reference/aggregations/bucket/children-aggregation.asciidoc b/docs/reference/aggregations/bucket/children-aggregation.asciidoc
@@ -144,7 +144,7 @@ Possible response:
   },
   "hits": {
     "total": 3,
-    "max_score": 0.0,
+    "max_score": null,
     "hits": []
   },
   "aggregations": {
diff --git a/docs/reference/getting-started.asciidoc b/docs/reference/getting-started.asciidoc
@@ -1141,7 +1141,7 @@ And the response (partially shown):
   },
   "hits" : {
     "total" : 1000,
-    "max_score" : 0.0,
+    "max_score" : null,
     "hits" : [ ]
   },
   "aggregations" : {
diff --git a/docs/reference/mapping/params/normalizer.asciidoc b/docs/reference/mapping/params/normalizer.asciidoc
@@ -151,7 +151,7 @@ returns
   },
   "hits": {
     "total": 3,
-    "max_score": 0.0,
+    "max_score": null,
     "hits": []
   },
   "aggregations": {
diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc
@@ -100,3 +100,8 @@ and the context is only accepted if `path` points to a field with `geo_point` ty
 `max_concurrent_shard_requests` used to limit the total number of concurrent shard
 requests a single high level search request can execute. In 7.0 this changed to be the
 max number of concurrent shard requests per node. The default is now `5`.
+
+==== `max_score` set to `null` when scores are not tracked
+
+`max_score` used to be set to `0` whenever scores are not tracked. `null` is now used
+instead which is a more appropriate value for a scenario where scores are not available.
diff --git a/docs/reference/search/request-body.asciidoc b/docs/reference/search/request-body.asciidoc
@@ -161,7 +161,7 @@ be set to `true` in the response.
   },
   "hits": {
     "total": 1,
-    "max_score": 0.0,
+    "max_score": null,
     "hits": []
   }
 }
diff --git a/docs/reference/search/suggesters/completion-suggest.asciidoc b/docs/reference/search/suggesters/completion-suggest.asciidoc
@@ -258,7 +258,7 @@ Which should look like:
     },
     "hits": {
         "total" : 0,
-        "max_score" : 0.0,
+        "max_score" : null,
         "hits" : []
     },
     "suggest": {
diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/query/ParentChildInnerHitContextBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/query/ParentChildInnerHitContextBuilder.java
@@ -131,7 +131,7 @@ public TopDocs[] topDocs(SearchHit[] hits) throws IOException {
                     for (LeafReaderContext ctx : context.searcher().getIndexReader().leaves()) {
                         intersect(weight, innerHitQueryWeight, totalHitCountCollector, ctx);
                     }
-                    result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, 0);
+                    result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, Float.NaN);
                 } else {
                     int topN = Math.min(from() + size(), context.searcher().getIndexReader().maxDoc());
                     TopDocsCollector<?> topDocsCollector;
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yml
@@ -233,3 +233,51 @@
           query:
             match_all: {}
           size: 0
+
+---
+"Scroll max_score is null":
+  - skip:
+      version: " - 6.99.99"
+      reason:  max_score was set to 0 rather than null before 7.0
+
+  - do:
+      indices.create:
+        index: test_scroll
+  - do:
+      index:
+        index:  test_scroll
+        type:   test
+        id:     42
+        body:   { foo: 1 }
+
+  - do:
+      index:
+        index:  test_scroll
+        type:   test
+        id:     43
+        body:   { foo: 2 }
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      search:
+        index: test_scroll
+        size: 1
+        scroll: 1m
+        sort: foo
+        body:
+          query:
+            match_all: {}
+
+  - set: {_scroll_id: scroll_id}
+  - length: {hits.hits:      1    }
+  - match: { hits.max_score: null }
+
+  - do:
+      scroll:
+        scroll_id: $scroll_id
+        scroll: 1m
+
+  - length: {hits.hits:      1    }
+  - match: { hits.max_score: null }
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml
@@ -244,6 +244,23 @@ setup:
   - match: { hits.total: 6 }
   - length: { hits.hits: 0 }
 
+---
+"no hits and inner_hits max_score null":
+
+  - skip:
+      version: " - 6.99.99"
+      reason:  max_score was set to 0 rather than null before 7.0
+
+  - do:
+      search:
+        index: test
+        body:
+          size: 0
+          collapse: { field: numeric_group, inner_hits: { name: sub_hits, size: 1} }
+          sort: [{ sort: desc }]
+
+  - match: { hits.max_score: null }
+
 ---
 "field collapsing and multiple inner_hits":
 
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/140_pre_filter_search_shards.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/140_pre_filter_search_shards.yml
@@ -128,7 +128,6 @@ setup:
   - match: { hits.total: 2 }
   - match: { aggregations.some_agg.doc_count: 3 }
 
-
   - do:
       search:
         pre_filter_shard_size: 1
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml
@@ -39,6 +39,7 @@ setup:
         df: text
 
   - match: {hits.total: 1}
+  - match: {hits.max_score: 1}
   - match: {hits.hits.0._score: 1}
 
   - do:
@@ -52,6 +53,7 @@ setup:
               boost: 2
 
   - match: {hits.total: 1}
+  - match: {hits.max_score: 2}
   - match: {hits.hits.0._score: 2}
 
   - do:
@@ -61,6 +63,7 @@ setup:
         df: text
 
   - match: {hits.total: 1}
+  - match: {hits.max_score: 1}
   - match: {hits.hits.0._score: 1}
 
 ---
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/210_rescore_explain.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/210_rescore_explain.yml
@@ -29,6 +29,7 @@
               query_weight: 5
               rescore_query_weight: 10
 
+  - match: {hits.max_score: 15}
   - match: { hits.hits.0._score: 15 }
   - match: { hits.hits.0._explanation.value: 15 }
 
diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java
@@ -101,7 +101,7 @@ public class Lucene {
 
     public static final ScoreDoc[] EMPTY_SCORE_DOCS = new ScoreDoc[0];
 
-    public static final TopDocs EMPTY_TOP_DOCS = new TopDocs(0, EMPTY_SCORE_DOCS, 0.0f);
+    public static final TopDocs EMPTY_TOP_DOCS = new TopDocs(0, EMPTY_SCORE_DOCS, Float.NaN);
 
     public static Version parseVersion(@Nullable String version, Version defaultVersion, Logger logger) {
         if (version == null) {
diff --git a/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java
@@ -398,7 +398,7 @@ public TopDocs[] topDocs(SearchHit[] hits) throws IOException {
                 if (size() == 0) {
                     TotalHitCountCollector totalHitCountCollector = new TotalHitCountCollector();
                     intersect(weight, innerHitQueryWeight, totalHitCountCollector, ctx);
-                    result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, 0);
+                    result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, Float.NaN);
                 } else {
                     int topN = Math.min(from() + size(), context.searcher().getIndexReader().maxDoc());
                     TopDocsCollector<?> topDocsCollector;
diff --git a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java
@@ -95,7 +95,7 @@ public void execute(SearchContext searchContext) throws QueryPhaseExecutionExcep
             suggestPhase.execute(searchContext);
             // TODO: fix this once we can fetch docs for suggestions
             searchContext.queryResult().topDocs(
-                    new TopDocs(0, Lucene.EMPTY_SCORE_DOCS, 0),
+                    new TopDocs(0, Lucene.EMPTY_SCORE_DOCS, Float.NaN),
                     new DocValueFormat[0]);
             return;
         }
diff --git a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java
@@ -120,7 +120,7 @@ Collector create(Collector in) {
         @Override
         void postProcess(QuerySearchResult result) {
             final int totalHitCount = hitCountSupplier.getAsInt();
-            result.topDocs(new TopDocs(totalHitCount, Lucene.EMPTY_SCORE_DOCS, 0), null);
+            result.topDocs(new TopDocs(totalHitCount, Lucene.EMPTY_SCORE_DOCS, Float.NaN), null);
         }
     }
 
diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java
@@ -321,7 +321,7 @@ public void testIssue11119() throws Exception {
 
         assertThat(response.getHits().getTotalHits(), equalTo(8L));
         assertThat(response.getHits().getHits().length, equalTo(0));
-        assertThat(response.getHits().getMaxScore(), equalTo(0f));
+        assertThat(response.getHits().getMaxScore(), equalTo(Float.NaN));
         Terms terms = response.getAggregations().get("terms");
         assertThat(terms, notNullValue());
         assertThat(terms.getName(), equalTo("terms"));
@@ -356,7 +356,7 @@ public void testIssue11119() throws Exception {
 
         assertThat(response.getHits().getTotalHits(), equalTo(8L));
         assertThat(response.getHits().getHits().length, equalTo(0));
-        assertThat(response.getHits().getMaxScore(), equalTo(0f));
+        assertThat(response.getHits().getMaxScore(), equalTo(Float.NaN));
         terms = response.getAggregations().get("terms");
         assertThat(terms, notNullValue());
         assertThat(terms.getName(), equalTo("terms"));
diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java
diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/MatchAssertion.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/MatchAssertion.java
diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/MatchAssertionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/MatchAssertionTests.java

Original file line number	Diff line number	Diff line change
@@ -161,7 +161,7 @@ be set to `true` in the response.
`161`	`161`	`},`
`162`	`162`	`"hits": {`
`163`	`163`	`"total": 1,`
`164`		`- "max_score": 0.0,`
	`164`	`+ "max_score": null,`
`165`	`165`	`"hits": []`
`166`	`166`	`}`
`167`	`167`	`}`
Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ public TopDocs[] topDocs(SearchHit[] hits) throws IOException {`
`131`	`131`	`for (LeafReaderContext ctx : context.searcher().getIndexReader().leaves()) {`
`132`	`132`	`intersect(weight, innerHitQueryWeight, totalHitCountCollector, ctx);`
`133`	`133`	`}`
`134`		`- result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, 0);`
	`134`	`+ result[i] = new TopDocs(totalHitCountCollector.getTotalHits(), Lucene.EMPTY_SCORE_DOCS, Float.NaN);`
`135`	`135`	`} else {`
`136`	`136`	`int topN = Math.min(from() + size(), context.searcher().getIndexReader().maxDoc());`
`137`	`137`	`TopDocsCollector<?> topDocsCollector;`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ public void execute(SearchContext searchContext) throws QueryPhaseExecutionExcep`
`95`	`95`	`suggestPhase.execute(searchContext);`
`96`	`96`	`// TODO: fix this once we can fetch docs for suggestions`
`97`	`97`	`searchContext.queryResult().topDocs(`
`98`		`- new TopDocs(0, Lucene.EMPTY_SCORE_DOCS, 0),`
	`98`	`+ new TopDocs(0, Lucene.EMPTY_SCORE_DOCS, Float.NaN),`
`99`	`99`	`new DocValueFormat[0]);`
`100`	`100`	`return;`
`101`	`101`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ Collector create(Collector in) {`
`120`	`120`	`@Override`
`121`	`121`	`void postProcess(QuerySearchResult result) {`
`122`	`122`	`final int totalHitCount = hitCountSupplier.getAsInt();`
`123`		`- result.topDocs(new TopDocs(totalHitCount, Lucene.EMPTY_SCORE_DOCS, 0), null);`
	`123`	`+ result.topDocs(new TopDocs(totalHitCount, Lucene.EMPTY_SCORE_DOCS, Float.NaN), null);`
`124`	`124`	`}`
`125`	`125`	`}`
`126`	`126`