From d03b8b6f14dd1f3e6a118c1a997e66af82a165d2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 26 Oct 2017 12:20:55 +0200 Subject: [PATCH] Fix max score tracking with field collapsing This change makes sure that we track score when sort is set to relevancy only. In this case we always track max score like normal search does. Closes #23840 --- .../search/query/TopDocsCollectorContext.java | 3 +- .../CollapsingTopDocsCollectorTests.java | 28 +++++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java b/core/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java index 46b03c61ca0b9..0c14ef6a8157e 100644 --- a/core/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java +++ b/core/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java @@ -283,9 +283,10 @@ static TopDocsCollectorContext createTopDocsCollectorContext(SearchContext searc return new ScrollingTopDocsCollectorContext(searchContext.scrollContext(), searchContext.sort(), numDocs, searchContext.trackScores(), searchContext.numberOfShards()); } else if (searchContext.collapse() != null) { + boolean trackScores = searchContext.sort() == null ? true : searchContext.trackScores(); int numDocs = Math.min(searchContext.from() + searchContext.size(), totalNumDocs); return new CollapsingTopDocsCollectorContext(searchContext.collapse(), - searchContext.sort(), numDocs, searchContext.trackScores()); + searchContext.sort(), numDocs, trackScores); } else { int numDocs = Math.min(searchContext.from() + searchContext.size(), totalNumDocs); final boolean rescore = searchContext.rescore().isEmpty() == false; diff --git a/core/src/test/java/org/apache/lucene/grouping/CollapsingTopDocsCollectorTests.java b/core/src/test/java/org/apache/lucene/grouping/CollapsingTopDocsCollectorTests.java index aef354a04951f..4352f16c05f2b 100644 --- a/core/src/test/java/org/apache/lucene/grouping/CollapsingTopDocsCollectorTests.java +++ b/core/src/test/java/org/apache/lucene/grouping/CollapsingTopDocsCollectorTests.java @@ -54,6 +54,8 @@ import java.util.List; import java.util.Set; +import static org.hamcrest.core.IsEqual.equalTo; + public class CollapsingTopDocsCollectorTests extends ESTestCase { private static class SegmentSearcher extends IndexSearcher { private final List ctx; @@ -82,12 +84,15 @@ interface CollapsingDocValuesProducer { } void assertSearchCollapse(CollapsingDocValuesProducer dvProducers, boolean numeric) throws IOException { - assertSearchCollapse(dvProducers, numeric, true); - assertSearchCollapse(dvProducers, numeric, false); + assertSearchCollapse(dvProducers, numeric, true, true); + assertSearchCollapse(dvProducers, numeric, true, false); + assertSearchCollapse(dvProducers, numeric, false, true); + assertSearchCollapse(dvProducers, numeric, false, false); } private void assertSearchCollapse(CollapsingDocValuesProducer dvProducers, - boolean numeric, boolean multivalued) throws IOException { + boolean numeric, boolean multivalued, + boolean trackMaxScores) throws IOException { final int numDocs = randomIntBetween(1000, 2000); int maxGroup = randomIntBetween(2, 500); final Directory dir = newDirectory(); @@ -118,14 +123,14 @@ private void assertSearchCollapse(CollapsingDocValuesProd final CollapsingTopDocsCollector collapsingCollector; if (numeric) { collapsingCollector = - CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups, false); + CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups, trackMaxScores); } else { collapsingCollector = - CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups, false); + CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups, trackMaxScores); } TopFieldCollector topFieldCollector = - TopFieldCollector.create(sort, totalHits, true, false, false); + TopFieldCollector.create(sort, totalHits, true, trackMaxScores, trackMaxScores); searcher.search(new MatchAllDocsQuery(), collapsingCollector); searcher.search(new MatchAllDocsQuery(), topFieldCollector); @@ -136,6 +141,11 @@ private void assertSearchCollapse(CollapsingDocValuesProd assertEquals(totalHits, collapseTopFieldDocs.totalHits); assertEquals(totalHits, topDocs.scoreDocs.length); assertEquals(totalHits, topDocs.totalHits); + if (trackMaxScores) { + assertThat(collapseTopFieldDocs.getMaxScore(), equalTo(topDocs.getMaxScore())); + } else { + assertThat(collapseTopFieldDocs.getMaxScore(), equalTo(Float.NaN)); + } Set seen = new HashSet<>(); // collapse field is the last sort @@ -186,14 +196,14 @@ private void assertSearchCollapse(CollapsingDocValuesProd } final CollapseTopFieldDocs[] shardHits = new CollapseTopFieldDocs[subSearchers.length]; - final Weight weight = searcher.createNormalizedWeight(new MatchAllDocsQuery(), false); + final Weight weight = searcher.createNormalizedWeight(new MatchAllDocsQuery(), true); for (int shardIDX = 0; shardIDX < subSearchers.length; shardIDX++) { final SegmentSearcher subSearcher = subSearchers[shardIDX]; final CollapsingTopDocsCollector c; if (numeric) { - c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups, false); + c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups, trackMaxScores); } else { - c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups, false); + c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups, trackMaxScores); } subSearcher.search(weight, c); shardHits[shardIDX] = c.getTopDocs();