jimczi commented on code in PR #12962:
URL: https://github.com/apache/lucene/pull/12962#discussion_r1456027720


##########
lucene/core/src/java/org/apache/lucene/index/LeafReader.java:
##########
@@ -240,11 +241,19 @@ public final PostingsEnum postings(Term term) throws 
IOException {
    * @param acceptDocs {@link Bits} that represents the allowed documents to 
match, or {@code null}
    *     if they are all allowed to match.
    * @param visitedLimit the maximum number of nodes that the search is 
allowed to visit
+   * @param globalScoreQueue the global score queue used to track the top 
scores collected across
+   *     all leaves
    * @return the k nearest neighbor documents, along with their 
(searchStrategy-specific) scores.
    * @lucene.experimental
    */
   public final TopDocs searchNearestVectors(
-      String field, float[] target, int k, Bits acceptDocs, int visitedLimit) 
throws IOException {
+      String field,
+      float[] target,
+      int k,
+      Bits acceptDocs,
+      int visitedLimit,
+      BlockingFloatHeap globalScoreQueue)

Review Comment:
   In retrospect I don't think we need to overload this function. The global 
queue is an implementation detail of the `KnnCollector` and should be 
considered expert. The knn query builds its own knn collector so the existing 
`public abstract void searchNearestVectors(
         String field, byte[] target, KnnCollector knnCollector, Bits 
acceptDocs) throws IOException;` is enough for what we need. 



##########
lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java:
##########
@@ -79,24 +82,30 @@ public Query rewrite(IndexSearcher indexSearcher) throws 
IOException {
       filterWeight = null;
     }
 
+    BlockingFloatHeap globalScoreQueue = new BlockingFloatHeap(k);

Review Comment:
   We can check if the query will use an executor and pick a non-blocking heap 
in such case?



##########
lucene/core/src/java/org/apache/lucene/index/LeafReader.java:
##########
@@ -280,12 +289,20 @@ public final TopDocs searchNearestVectors(
    * @param k the number of docs to return
    * @param acceptDocs {@link Bits} that represents the allowed documents to 
match, or {@code null}
    *     if they are all allowed to match.
-   * @param visitedLimit the maximum number of nodes that the search is 
allowed to visit
+   * @param visitedLimit the maximum number of nodes that the search is 
allowed to visit *@param
+   *     globalScoreQueue the global score queue used to track the top scores 
collected across all
+   *     leaves
    * @return the k nearest neighbor documents, along with their 
(searchStrategy-specific) scores.
    * @lucene.experimental
    */
   public final TopDocs searchNearestVectors(
-      String field, byte[] target, int k, Bits acceptDocs, int visitedLimit) 
throws IOException {
+      String field,
+      byte[] target,
+      int k,
+      Bits acceptDocs,
+      int visitedLimit,
+      BlockingFloatHeap globalScoreQueue)
+      throws IOException {

Review Comment:
   See my previous comment, we don't need to overload any function in the leaf 
reader and only rely on the `KnnCollector` to do the right thing.



-- 
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

Reply via email to