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

Reply via email to