Ensure top docs optimization is fully disabled for queries with unbounded max scores. (#46105) (#46139)

When a query contains a mandatory clause that doesn't track the max score per
block, we disable the max score optimization. Previously, we were doing this by
wrapping the collector with a FilterCollector that always returned
ScoreMode.COMPLETE.

However we weren't adjusting totalHitsThreshold, so the collector could still
call Scorer#setMinCompetitiveScore. It is against the method contract to call
setMinCompetitiveScore when the score mode is COMPLETE, and some scorers like
ReqOptSumScorer throw an error in this case.

This commit tries to disable the optimization by always setting
totalHitsThreshold to max int, as opposed to wrapping the collector.
This commit is contained in:
Julie Tibshirani 2019-08-29 10:56:53 -07:00 committed by GitHub
parent c4aa8e0d90
commit b5d8b364bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 28 deletions

View File

@ -33,13 +33,11 @@ import org.apache.lucene.search.Collector;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
import org.apache.lucene.search.FieldDoc;
import org.apache.lucene.search.FilterCollector;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.MultiCollector;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.QueryVisitor;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TermQuery;
@ -238,7 +236,15 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext {
this.sortAndFormats = sortAndFormats;
final TopDocsCollector<?> topDocsCollector;
if (trackTotalHitsUpTo == SearchContext.TRACK_TOTAL_HITS_DISABLED) {
if ((sortAndFormats == null || SortField.FIELD_SCORE.equals(sortAndFormats.sort.getSort()[0]))
&& hasInfMaxScore(query)) {
// disable max score optimization since we have a mandatory clause
// that doesn't track the maximum score
topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, Integer.MAX_VALUE);
topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
totalHitsSupplier = () -> topDocsSupplier.get().totalHits;
} else if (trackTotalHitsUpTo == SearchContext.TRACK_TOTAL_HITS_DISABLED) {
// don't compute hit counts via the collector
topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, 1);
topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
@ -274,27 +280,7 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext {
maxScoreSupplier = () -> Float.NaN;
}
final Collector collector = MultiCollector.wrap(topDocsCollector, maxScoreCollector);
if (sortAndFormats == null ||
SortField.FIELD_SCORE.equals(sortAndFormats.sort.getSort()[0])) {
if (hasInfMaxScore(query)) {
// disable max score optimization since we have a mandatory clause
// that doesn't track the maximum score
this.collector = new FilterCollector(collector) {
@Override
public ScoreMode scoreMode() {
if (in.scoreMode() == ScoreMode.TOP_SCORES) {
return ScoreMode.COMPLETE;
}
return in.scoreMode();
}
};
} else {
this.collector = collector;
}
} else {
this.collector = collector;
}
this.collector = MultiCollector.wrap(topDocsCollector, maxScoreCollector);
}

View File

@ -606,15 +606,16 @@ public class QueryPhaseTests extends IndexShardTestCase {
.build();
context.parsedQuery(new ParsedQuery(q));
context.setSize(10);
context.setSize(3);
context.trackTotalHitsUpTo(3);
TopDocsCollectorContext topDocsContext =
TopDocsCollectorContext.createTopDocsCollectorContext(context, reader, false);
assertEquals(topDocsContext.create(null).scoreMode(), org.apache.lucene.search.ScoreMode.COMPLETE);
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(5, context.queryResult().topDocs().topDocs.totalHits.value);
assertEquals(context.queryResult().topDocs().topDocs.totalHits.relation, TotalHits.Relation.EQUAL_TO);
assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(5));
assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(3));
context.sort(new SortAndFormats(new Sort(new SortField("other", SortField.Type.INT)),
new DocValueFormat[] { DocValueFormat.RAW }));
@ -623,7 +624,7 @@ public class QueryPhaseTests extends IndexShardTestCase {
assertEquals(topDocsContext.create(null).scoreMode(), org.apache.lucene.search.ScoreMode.COMPLETE_NO_SCORES);
QueryPhase.execute(context, contextSearcher, checkCancelled -> {});
assertEquals(5, context.queryResult().topDocs().topDocs.totalHits.value);
assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(5));
assertThat(context.queryResult().topDocs().topDocs.scoreDocs.length, equalTo(3));
assertEquals(context.queryResult().topDocs().topDocs.totalHits.relation, TotalHits.Relation.EQUAL_TO);
reader.close();