kaivalnp commented on PR #12590: URL: https://github.com/apache/lucene/pull/12590#issuecomment-1734432239
> You can easily get around all this without any expensive locking. > The collector has a "topDocs" method that could call some higher level collector. Nice idea! So basically we pass a subclass of `TopKnnCollector` [here](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/KnnFloatVectorQuery.java#L82), where the "topDocs" function is overloaded to pass the final results of each segment to a high-level collector when called? I wonder how this would be any different from passing the final `TopDocs` at the end of [`#approximateSearch`](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/KnnFloatVectorQuery.java#L79) directly to the high-level collector? In any case, even if we pass the `TopDocs` from each segment to a high-level collector, it would have to combine the individual `TopDocs` results across all segments to figure out the index-level `topK` independently of `AbstractKnnVectorQuery` right? (duplicating [this piece of work](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java#L88) in both places) > you might as well just override the "rewrite" method Overriding `rewrite` (first calling `super.rewrite` and looking at the rewritten `Query`) may not work because the [`DocAndScoreQuery`](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java#L297) class is `package-private` so we cannot read the internal [`docs`](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java#L299) without reflection? Moreover, they are sorted in ascending order of `docid` already (and the ordering along scores is lost - so this may not be as useful) We can always copy the [entire `rewrite` function](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java#L64-L93) and the [`DocAndScoreQuery`](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java#L297-L452) class into all implementers of `AbstractKnnVectorQuery` to get access to the `topK`, but this seems a bit overkill? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
