dsmiley commented on a change in pull request #797: LUCENE-8921: IndexSearcher.termStatistics requires docFreq totalTermFreq instead of TermStates URL: https://github.com/apache/lucene-solr/pull/797#discussion_r307019512
########## File path: lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java ########## @@ -861,17 +861,17 @@ public String toString() { /** * Returns {@link TermStatistics} for a term, or {@code null} if - * the term does not exist. + * the term does not exist (if docFreq == 0). * * This can be overridden for example, to return a term's statistics * across a distributed collection. * @lucene.experimental */ - public TermStatistics termStatistics(Term term, TermStates context) throws IOException { - if (context.docFreq() == 0) { + public TermStatistics termStatistics(Term term, int docFreq, long totalTermFreq) throws IOException { + if (docFreq == 0) { Review comment: I bring this up because I don't think we should simply add this extra work for _all_ callers (as I see you did in the last commit Bruno); it's only unusual cases where this might happen. You might first remove them all and tell us what tests reveal which queries need this, then put the zero check only for them. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org