kaivalnp commented on code in PR #932:
URL: https://github.com/apache/lucene/pull/932#discussion_r887637829
##########
lucene/core/src/java/org/apache/lucene/search/KnnVectorQuery.java:
##########
@@ -225,6 +225,11 @@ public BitSetIterator getIterator(int contextOrd) {
return new BitSetIterator(bitSets[contextOrd], cost[contextOrd]);
}
+ public void setBitSet(BitSet bitSet, int cost) {
+ bitSets[ord] = bitSet;
Review Comment:
> So .. this relies on the fact that `BitSetCollector.doSetNextReader` will
have been called prior to this, setting the appropriate `ord`. This is very
clever, but it relies heavily on knowledge of internal implementation details
that aren't really part of the published API, and this will tie us to this
BitSetCollector implementation. I wonder if we could create a new
KnnVectorQuery constructor that would accept a Function<Integer,
BitSetIterator>, rather than a Query, that would be responsible for returning a
BitSetIterator for a given ord?
This is interesting and would solve many problems of wrapping the BitSet in
a Query (and associated BulkScorer). Given this function, we could add a new
constructor to BitSetCollector (to instantiate the internal BitSets from these
BitSetIterators instead), and no indexSearcher.search would be required
thereafter
My concern is that it creates different search paths and constructors for
this function, which will only be used from our test and might complicate the
class (Though this addition would be helpful if we want to delegate the task of
populating the filtered docs into BitSets outside of KnnVectorQuery)
--
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]
