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

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`).
This commit is contained in:
Luca Cavanna 2018-08-22 17:23:54 +02:00 committed by GitHub
parent abb4c183f1
commit 393eec1482
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 163 additions and 25 deletions

View File

@ -256,7 +256,7 @@ public class SearchIT extends ESRestHighLevelClientTestCase {
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 class SearchIT extends ESRestHighLevelClientTestCase {
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 class SearchIT extends ESRestHighLevelClientTestCase {
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 class SearchIT extends ESRestHighLevelClientTestCase {
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 class SearchIT extends ESRestHighLevelClientTestCase {
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 class SearchIT extends ESRestHighLevelClientTestCase {
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());

View File

@ -144,7 +144,7 @@ Possible response:
},
"hits": {
"total": 3,
"max_score": 0.0,
"max_score": null,
"hits": []
},
"aggregations": {

View File

@ -1141,7 +1141,7 @@ And the response (partially shown):
},
"hits" : {
"total" : 1000,
"max_score" : 0.0,
"max_score" : null,
"hits" : [ ]
},
"aggregations" : {

View File

@ -151,7 +151,7 @@ returns
},
"hits": {
"total": 3,
"max_score": 0.0,
"max_score": null,
"hits": []
},
"aggregations": {

View File

@ -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.

View File

@ -161,7 +161,7 @@ be set to `true` in the response.
},
"hits": {
"total": 1,
"max_score": 0.0,
"max_score": null,
"hits": []
}
}

View File

@ -258,7 +258,7 @@ Which should look like:
},
"hits": {
"total" : 0,
"max_score" : 0.0,
"max_score" : null,
"hits" : []
},
"suggest": {

View File

@ -131,7 +131,7 @@ class ParentChildInnerHitContextBuilder extends InnerHitContextBuilder {
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;

View File

@ -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 }

View File

@ -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":

View File

@ -128,7 +128,6 @@ setup:
- match: { hits.total: 2 }
- match: { aggregations.some_agg.doc_count: 3 }
- do:
search:
pre_filter_shard_size: 1

View File

@ -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}
---

View File

@ -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 }

View File

@ -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) {

View File

@ -398,7 +398,7 @@ public class NestedQueryBuilder extends AbstractQueryBuilder<NestedQueryBuilder>
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;

View File

@ -95,7 +95,7 @@ public class QueryPhase implements SearchPhase {
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;
}

View File

@ -120,7 +120,7 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext {
@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);
}
}

View File

@ -321,7 +321,7 @@ public class TopHitsIT extends ESIntegTestCase {
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 class TopHitsIT extends ESIntegTestCase {
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"));

View File

@ -67,6 +67,7 @@ import java.util.List;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.instanceOf;
@ -103,6 +104,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
final boolean rescore = QueryPhase.execute(context, searcher, checkCancelled -> {});
assertFalse(rescore);
assertEquals(searcher.count(query), context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
}
private void countTestCase(boolean withDeletions) throws Exception {
@ -172,11 +174,14 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(1, context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(0));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
contextSearcher = new IndexSearcher(reader);
context.parsedPostFilter(new ParsedQuery(new MatchNoDocsQuery()));
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(0, context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
reader.close();
dir.close();
}
@ -205,13 +210,13 @@ public class QueryPhaseTests extends IndexShardTestCase {
context.parsedPostFilter(new ParsedQuery(new TermQuery(new Term("foo", Integer.toString(i)))));
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(1, context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(1));
}
reader.close();
dir.close();
}
public void testMinScoreDisablesCountOptimization() throws Exception {
Directory dir = newDirectory();
final Sort sort = new Sort(new SortField("rank", SortField.Type.INT));
@ -230,11 +235,13 @@ public class QueryPhaseTests extends IndexShardTestCase {
context.setTask(new SearchTask(123L, "", "", "", null, Collections.emptyMap()));
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(1, context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
contextSearcher = new IndexSearcher(reader);
context.minimumScore(100);
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(0, context.queryResult().topDocs().totalHits);
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
reader.close();
dir.close();
}
@ -289,6 +296,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertThat(context.queryResult().topDocs().totalHits, equalTo((long) numDocs));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertNull(context.queryResult().terminatedEarly());
assertThat(context.terminateAfter(), equalTo(0));
assertThat(context.queryResult().getTotalHits(), equalTo((long) numDocs));
@ -296,9 +304,11 @@ public class QueryPhaseTests extends IndexShardTestCase {
contextSearcher = getAssertingEarlyTerminationSearcher(reader, size);
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertThat(context.queryResult().topDocs().totalHits, equalTo((long) numDocs));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.terminateAfter(), equalTo(size));
assertThat(context.queryResult().getTotalHits(), equalTo((long) numDocs));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertThat(context.queryResult().topDocs().scoreDocs[0].doc, greaterThanOrEqualTo(size));
reader.close();
dir.close();
@ -334,12 +344,14 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(1));
context.setSize(0);
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(0));
}
@ -348,6 +360,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(1F));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(1));
}
{
@ -360,6 +373,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), greaterThan(0f));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(1));
context.setSize(0);
@ -367,6 +381,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(0));
}
{
@ -376,6 +391,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), greaterThan(0f));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(1));
assertThat(collector.getTotalHits(), equalTo(1));
context.queryCollectors().clear();
@ -387,6 +403,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertTrue(context.queryResult().terminatedEarly());
assertThat(context.queryResult().topDocs().totalHits, equalTo(1L));
assertThat(context.queryResult().topDocs().getMaxScore(), equalTo(Float.NaN));
assertThat(context.queryResult().topDocs().scoreDocs.length, equalTo(0));
assertThat(collector.getTotalHits(), equalTo(1));
}
@ -539,19 +556,19 @@ public class QueryPhaseTests extends IndexShardTestCase {
dir.close();
}
static IndexSearcher getAssertingEarlyTerminationSearcher(IndexReader reader, int size) {
private static IndexSearcher getAssertingEarlyTerminationSearcher(IndexReader reader, int size) {
return new IndexSearcher(reader) {
protected void search(List<LeafReaderContext> leaves, Weight weight, Collector collector) throws IOException {
final Collector in = new AssertingEalyTerminationFilterCollector(collector, size);
final Collector in = new AssertingEarlyTerminationFilterCollector(collector, size);
super.search(leaves, weight, in);
}
};
}
private static class AssertingEalyTerminationFilterCollector extends FilterCollector {
private static class AssertingEarlyTerminationFilterCollector extends FilterCollector {
private final int size;
AssertingEalyTerminationFilterCollector(Collector in, int size) {
AssertingEarlyTerminationFilterCollector(Collector in, int size) {
super(in);
this.size = size;
}

View File

@ -32,6 +32,7 @@ import static org.elasticsearch.test.hamcrest.RegexMatcher.matches;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
/**
@ -70,8 +71,13 @@ public class MatchAssertion extends Assertion {
}
}
assertNotNull("field [" + getField() + "] is null", actualValue);
logger.trace("assert that [{}] matches [{}] (field [{}])", actualValue, expectedValue, getField());
if (expectedValue == null) {
assertNull("field [" + getField() + "] should be null but was [" + actualValue + "]", actualValue);
return;
}
assertNotNull("field [" + getField() + "] is null", actualValue);
if (actualValue.getClass().equals(safeClass(expectedValue)) == false) {
if (actualValue instanceof Number && expectedValue instanceof Number) {
//Double 1.0 is equal to Integer 1

View File

@ -0,0 +1,42 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.test.rest.yaml.section;
import org.elasticsearch.common.xcontent.XContentLocation;
import org.elasticsearch.test.ESTestCase;
public class MatchAssertionTests extends ESTestCase {
public void testNull() {
XContentLocation xContentLocation = new XContentLocation(0, 0);
{
MatchAssertion matchAssertion = new MatchAssertion(xContentLocation, "field", null);
matchAssertion.doAssert(null, null);
expectThrows(AssertionError.class, () -> matchAssertion.doAssert("non-null", null));
}
{
MatchAssertion matchAssertion = new MatchAssertion(xContentLocation, "field", "non-null");
expectThrows(AssertionError.class, () -> matchAssertion.doAssert(null, "non-null"));
}
{
MatchAssertion matchAssertion = new MatchAssertion(xContentLocation, "field", "/exp/");
expectThrows(AssertionError.class, () -> matchAssertion.doAssert(null, "/exp/"));
}
}
}