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