gsmiller commented on a change in pull request #217:
URL: https://github.com/apache/lucene/pull/217#discussion_r676879074
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java
##########
@@ -195,11 +195,8 @@ private void doQueryFirstScoring(Bits acceptDocs,
LeafCollector collector, DocsA
collectDocID = docID;
Review comment:
Thanks, this looks great!
I think the caching could be useful down in `collectHit()` in the case that
a `sidewaysLeafCollector` decides to call back into `score()` (e.g., if
`FacetsCollector` has `keepScores` set to `true` and calls back to get the
score
[here](https://github.com/apache/lucene/blob/main/lucene/facet/src/java/org/apache/lucene/facet/FacetsCollector.java#L125)).
Without using something like `ScoreCachingWrappingScorer`, the underlying
score would need to be recomputed for the same docid if I'm not mistaken. Does
that sound right or am I overlooking something?
Thanks again for taking this up! Excited to get this change merged once we
figure out whether-or-not we need the caching layer in place.
--
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-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
