benwtrent commented on code in PR #13162:
URL: https://github.com/apache/lucene/pull/13162#discussion_r1516111922
##########
lucene/core/src/java/org/apache/lucene/search/FloatVectorSimilarityValuesSource.java:
##########
@@ -40,6 +40,9 @@ public FloatVectorSimilarityValuesSource(float[] vector,
String fieldName) {
@Override
public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores)
throws IOException {
final FloatVectorValues vectorValues =
ctx.reader().getFloatVectorValues(fieldName);
+ if (vectorValues == null) {
+ return DoubleValues.EMPTY;
+ }
Review Comment:
Why do you throw in the byte similarity source, but not here? We need to be
consistent. I think throwing here is acceptable as well (via
`FloatVectorValues.check`).
##########
lucene/core/src/java/org/apache/lucene/index/LeafReader.java:
##########
@@ -246,11 +246,12 @@ public final PostingsEnum postings(Term term) throws
IOException {
public final TopDocs searchNearestVectors(
String field, float[] target, int k, Bits acceptDocs, int visitedLimit)
throws IOException {
FieldInfo fi = getFieldInfos().fieldInfo(field);
- if (fi == null || fi.getVectorDimension() == 0) {
- // The field does not exist or does not index vectors
+ FloatVectorValues floatVectorValues = getFloatVectorValues(fi.name);
+ if (floatVectorValues == null) {
+ FloatVectorValues.checkField(this, field);
Review Comment:
The leaf reader here shouldn't throw. Especially since the companion method
that accepts a KnnCollector doesn't.
##########
lucene/core/src/java/org/apache/lucene/search/FieldExistsQuery.java:
##########
@@ -128,12 +128,12 @@ public Query rewrite(IndexSearcher indexSearcher) throws
IOException {
break;
}
} else if (fieldInfo.getVectorDimension() != 0) { // the field indexes
vectors
- int numVectors =
+ DocIdSetIterator vectorValues =
switch (fieldInfo.getVectorEncoding()) {
- case FLOAT32 -> leaf.getFloatVectorValues(field).size();
- case BYTE -> leaf.getByteVectorValues(field).size();
+ case FLOAT32 -> leaf.getFloatVectorValues(field);
+ case BYTE -> leaf.getByteVectorValues(field);
};
- if (numVectors != leaf.maxDoc()) {
+ if (vectorValues != null && vectorValues.cost() != leaf.maxDoc()) {
Review Comment:
```suggestion
assert vectorValues != null : "unexpected null vector values";
if (vectorValues != null && vectorValues.cost() != leaf.maxDoc()) {
```
I think being safe here is fine. But, this should never be null. An
assertion should be added to ensure that it isn't (keeping the null check in
the if statement is ok).
##########
lucene/core/src/java/org/apache/lucene/index/LeafReader.java:
##########
@@ -287,11 +288,12 @@ public final TopDocs searchNearestVectors(
public final TopDocs searchNearestVectors(
String field, byte[] target, int k, Bits acceptDocs, int visitedLimit)
throws IOException {
FieldInfo fi = getFieldInfos().fieldInfo(field);
- if (fi == null || fi.getVectorDimension() == 0) {
- // The field does not exist or does not index vectors
+ ByteVectorValues byteVectorValues = getByteVectorValues(fi.name);
+ if (byteVectorValues == null) {
+ ByteVectorValues.checkField(this, field);
Review Comment:
The leaf reader here shouldn't throw. Especially since the companion method
that accepts a KnnCollector doesn't.
--
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]
