Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not too happy that all the failures I got came from integration tests and docs tests. I added some assertions to QueryPhaseTests but maybe that is not enough. Suggestions are welcome on where to add tests for this change.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Possible response:
},
"hits": {
"total": 3,
"max_score": 0.0,
"max_score": null,
"hits": []
},
"aggregations": {
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ And the response (partially shown):
},
"hits" : {
"total" : 1000,
"max_score" : 0.0,
"max_score" : null,
"hits" : [ ]
},
"aggregations" : {
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/mapping/params/normalizer.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ returns
},
"hits": {
"total": 3,
"max_score": 0.0,
"max_score": null,
"hits": []
},
"aggregations": {
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/migration/migrate_7_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion docs/reference/search/request-body.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ be set to `true` in the response.
},
"hits": {
"total": 1,
"max_score": 0.0,
"max_score": null,
"hits": []
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ Which should look like:
},
"hits": {
"total" : 0,
"max_score" : 0.0,
"max_score" : null,
"hits" : []
},
"suggest": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Original file line number Diff line number Diff line change
Expand Up @@ -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":

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ setup:
- match: { hits.total: 2 }
- match: { aggregations.some_agg.doc_count: 3 }


- do:
search:
pre_filter_shard_size: 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ setup:
df: text

- match: {hits.total: 1}
- match: {hits.max_score: 1}
- match: {hits.hits.0._score: 1}

- do:
Expand All @@ -52,6 +53,7 @@ setup:
boost: 2

- match: {hits.total: 1}
- match: {hits.max_score: 2}
- match: {hits.hits.0._score: 2}

- do:
Expand All @@ -61,6 +63,7 @@ setup:
df: text

- match: {hits.total: 1}
- match: {hits.max_score: 1}
- match: {hits.hits.0._score: 1}

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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"));
Expand Down
Loading