[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery

[GitBox <gi...@apache.org>]

gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r940164398


##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
       }
 
       @Override
-      public Scorer scorer(LeafReaderContext context) throws IOException {
-        final WeightOrDocIdSet weightOrBitSet = rewrite(context);
-        if (weightOrBitSet == null) {
-          return null;
-        } else if (weightOrBitSet.weight != null) {
-          return weightOrBitSet.weight.scorer(context);
-        } else {
-          return scorer(weightOrBitSet.set);
+      public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+        // Cost estimation reasoning is:
+        //  1. Assume every query term matches at least one document (queryTermsCount).
+        //  2. Determine the total number of docs beyond the first one for each term.
+        //     That count provides a ceiling on the number of extra docs that could match beyond
+        //     that first one. (We omit the first since it's already been counted in #1).
+        // This approach still provides correct worst-case cost in general, but provides tighter
+        // estimates for primary-key-like fields. See: LUCENE-10207
+
+        // TODO: This cost estimation may grossly overestimate since we have no index statistics
+        // for the specific query terms. While it's nice to avoid the cost of intersecting the
+        // query terms with the index, it could be beneficial to do that work and get better
+        // cost estimates.
+        final long cost;
+        final long queryTermsCount = termData.size();
+        Terms indexTerms = context.reader().terms(field);
+        long potentialExtraCost = indexTerms.getSumDocFreq();
+        final long indexedTermCount = indexTerms.size();
+        if (indexedTermCount != -1) {
+          potentialExtraCost -= indexedTermCount;
         }
+        cost = queryTermsCount + potentialExtraCost;
+
+        final Weight weight = this;
+        return new ScorerSupplier() {
+          @Override
+          public Scorer get(long leadCost) throws IOException {
+            WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+            if (weightOrDocIdSet == null) {
+              return null;
+            }
+
+            final Scorer scorer;
+            if (weightOrDocIdSet.weight != null) {
+              scorer = weightOrDocIdSet.weight.scorer(context);
+            } else {
+              scorer = scorer(weightOrDocIdSet.set);
+            }
+
+            return Objects.requireNonNullElseGet(
+                scorer,
+                () ->
+                    new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+          }
+
+          @Override
+          public long cost() {
+            return cost;

Review Comment:
   I went this direction initially, but [got some feedback](https://issues.apache.org/jira/browse/LUCENE-10207?focusedCommentId=17436292&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17436292) that we expect `#cost()` to model the amount of work required by the scorer, which in the case of a disjunction like this, is more proportional to `sumDocFreq`. I suppose it's still not 100% clear to me if `#cost` should model the number of docs that could be produced or the amount of work. But if you dig into [the way we estimate cost for disjunctions in general](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java#L47), it doesn't put any cap on cost, so this is consistent. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org