benwtrent commented on code in PR #13469:
URL: https://github.com/apache/lucene/pull/13469#discussion_r1638749258
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -217,6 +219,18 @@ public ByteVectorValues getByteVectorValues(String field)
throws IOException {
vectorData);
}
+ @Override
+ public void search(String field, float[] target, KnnCollector knnCollector,
Bits acceptDocs)
+ throws IOException {
+ // don't scan stored field data. If we didn't index it, produce no search
results
+ }
Review Comment:
Since this is the case for all `FlatVectorsReader` objects, think we should
put this override there?
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsFormat.java:
##########
@@ -79,7 +79,8 @@ public final class Lucene99FlatVectorsFormat extends
FlatVectorsFormat {
private final FlatVectorsScorer vectorsScorer;
/** Constructs a format */
- public Lucene99FlatVectorsFormat(FlatVectorsScorer vectorsScorer) {
+ public Lucene99FlatVectorsFormat(String name, FlatVectorsScorer
vectorsScorer) {
+ super(name);
Review Comment:
I wonder why for `Lucene99FlatVectorsFormat` we allow an external name to be
provided. Is this because it isn't really ever loaded via the SPI?
Seems like `Lucene99FlatVectorsFormat` shouldn't accept a name as a
parameter and simply provide `super` with the correct name.
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99ScalarQuantizedVectorsReader.java:
##########
@@ -189,6 +191,18 @@ public ByteVectorValues getByteVectorValues(String field)
throws IOException {
return rawVectorsReader.getByteVectorValues(field);
}
+ @Override
+ public void search(String field, float[] target, KnnCollector knnCollector,
Bits acceptDocs)
+ throws IOException {
+ // don't scan stored field data. If we didn't index it, produce no search
results
+ }
+
+ @Override
+ public void search(String field, byte[] target, KnnCollector knnCollector,
Bits acceptDocs)
+ throws IOException {
+ // don't scan stored field data. If we didn't index it, produce no search
results
+ }
Review Comment:
Maybe this should be in `FlatVectorsReader`
##########
lucene/codecs/src/java/org/apache/lucene/codecs/bitvectors/HnswBitVectorsFormat.java:
##########
@@ -128,7 +129,7 @@ public HnswBitVectorsFormat(
} else {
this.mergeExec = null;
}
- this.flatVectorsFormat = new Lucene99FlatVectorsFormat(new
FlatBitVectorsScorer());
+ this.flatVectorsFormat = new Lucene99FlatVectorsFormat(NAME_FLAT, new
FlatBitVectorsScorer());
Review Comment:
I wonder why a unique name here is required.
The top level format name is `HnswBitVectorsFormat`, and it doesn't really
do anything to the inner format, it simply overrides the scoring mechanism.
--
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
