Lucene 9 #4867
Lucene 9 #4867
Conversation
|
see comments in #4570 |
| public BitIntsHolder reduce(Collection<SuggestResultCollector> collectors) { | ||
| BitIntsHolder reduced = documentIds; | ||
| for (SuggestResultCollector collector : collectors) { | ||
| documentIds.or(collector.documentIds); //TODO fix as per https://github.com/apache/lucene/pull/766/files |
There was a problem hiding this comment.
Can this be addressed for this PR ? I don't like adding TODOs in the code.
idodeclare
left a comment
There was a problem hiding this comment.
Hi, Lubos. A couple of comments and questions, mostly concerning paging handling
| collector = TopScoreDocCollector.create(totalHits, Short.MAX_VALUE); | ||
| searcher.search(query, collector); | ||
| collectorManager = new TopScoreDocCollectorManager(totalHits, Short.MAX_VALUE); | ||
| hits = searcher.search(query, collectorManager).scoreDocs; |
There was a problem hiding this comment.
Since the new version is compelling this slightly intrusive revision in order to use CollectorManager instead of Collector, may we rework at this time to call search() only once whether paging is true or false? That is, avoid the double search() on line 210 and 217 when paging is false (and go back to only setting hits once)?
Also that way we could make sure search() only happens prior to the Statistics.report() on line 212; in the current code, the Statistics are not reflecting the search() happening on line 217.
| searcher.search(query, collector); | ||
| totalHits = collector.getTotalHits(); | ||
| hits = searcher.search(query, collectorManager).scoreDocs; | ||
| totalHits = searcher.count(query); |
There was a problem hiding this comment.
Formerly totalhits would have had its accuracy bounded by the Short.MAX_VALUE value of totalHitsThreshold (on the TopScoreDocCollector). For very large indexes, is there any possible performance penalty getting the entire, "fully accurate" int value of totalHits?
No description provided.