Thanks Adrien & Mike! I hadn't seen the Solr email thread, but that's a good reference (as is the AssertingLeafCollector implementation). I've opened two follow-up PRs as a result:
1. Fix a buried drill-sideways bug where #finish can be called more than once: https://github.com/apache/lucene/pull/12642 2. Add a small note to LeafCollector#finish javadoc: https://github.com/apache/lucene/pull/12643 Cheers, -Greg On Mon, Oct 9, 2023 at 1:45 PM Mike Drob <md...@mdrob.com> wrote: > Not sure if you saw this, Greg, but Alex ran into a similar question > recently from Solr. > https://lists.apache.org/thread/1gs3nsv1mcns1czdtdnqyz84f31tqm2x > > On Mon, Oct 9, 2023 at 10:47 AM Adrien Grand <jpou...@gmail.com> wrote: > >> Hi Greg, >> >> I agree that LeafCollector implementations should be able to assume that >> finish() only gets called once. The test framework already makes this >> assumption: >> https://github.com/apache/lucene/blob/dfff1e635805ffc61dd6029a8060e2635bfcbdb9/lucene/test-framework/src/java/org/apache/lucene/tests/search/AssertingLeafCollector.java#L95-L100 >> . >> >> On Mon, Oct 9, 2023 at 5:38 PM Greg Miller <gsmil...@gmail.com> wrote: >> >>> Hey folks- >>> >>> I'm curious if anyone has thoughts around idempotency concerns related >>> to the LeafCollector#finish API added in GH#12380 >>> <https://github.com/apache/lucene/pull/12380>. My expectation would be >>> that LeafCollector implementations should be able to assume #finish will >>> only get called once. In fact, it looks like FacetsCollector is already >>> making that assumption. >>> >>> Is this inline with other folks' expectations? If so, I'm going to, 1) >>> address a small bug related to drill-sideways that results #finish being >>> called multiple times on one of the collectors, and 2) propose some >>> additional javadoc on LeafCollector#finish clarifying this. >>> >>> Make sense? >>> >>> Cheers, >>> -Greg >>> >> >> >> -- >> Adrien >> >
