benwtrent commented on code in PR #13641:
URL: https://github.com/apache/lucene/pull/13641#discussion_r1754984085


##########
lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseKnnVectorsFormatTestCase.java:
##########
@@ -230,6 +236,120 @@ public void testIllegalDimChangeTwoWriters() throws 
Exception {
     }
   }
 
+  @SuppressWarnings("unused")
+  public void testMergingWithDifferentKnnFields() throws Exception {
+    try (var dir = newDirectory()) {
+      IndexWriterConfig iwc = new IndexWriterConfig();
+      Codec codec = getCodec();
+      if (codec.knnVectorsFormat() instanceof PerFieldKnnVectorsFormat 
perFieldKnnVectorsFormat) {
+        final KnnVectorsFormat format =
+            perFieldKnnVectorsFormat.getKnnVectorsFormatForField("field");
+        iwc.setCodec(
+            new FilterCodec(codec.getName(), codec) {
+              @Override
+              public KnnVectorsFormat knnVectorsFormat() {
+                return format;
+              }
+            });
+      }
+      TestMergeScheduler mergeScheduler = new TestMergeScheduler();
+      iwc.setMergeScheduler(mergeScheduler);
+      iwc.setMergePolicy(new ForceMergePolicy(iwc.getMergePolicy()));
+      try (var writer = new IndexWriter(dir, iwc)) {
+        for (int i = 0; i < 10; i++) {
+          var doc = new Document();
+          doc.add(new KnnFloatVectorField("field", new float[] {i, i + 1, i + 
2, i + 3}));
+          writer.addDocument(doc);
+        }
+        writer.commit();
+        for (int i = 0; i < 10; i++) {
+          var doc = new Document();
+          doc.add(new KnnFloatVectorField("otherVector", new float[] {i, i, i, 
i}));
+          writer.addDocument(doc);
+        }
+        writer.commit();
+        try {
+          writer.forceMerge(1);
+        } catch (IllegalStateException e) {
+          // do nothing, we expect this exception
+        }
+        assertNotNull(mergeScheduler.ex.get());
+        Exception ex = mergeScheduler.ex.get();
+        assertTrue(ex instanceof IllegalArgumentException);
+      }
+    }
+  }
+
+  @SuppressWarnings("unused")
+  public void testMergingWithDifferentByteKnnFields() throws Exception {
+    try (var dir = newDirectory()) {
+      IndexWriterConfig iwc = new IndexWriterConfig();
+      Codec codec = getCodec();
+      if (codec.knnVectorsFormat() instanceof PerFieldKnnVectorsFormat 
perFieldKnnVectorsFormat) {
+        final KnnVectorsFormat format =
+            perFieldKnnVectorsFormat.getKnnVectorsFormatForField("field");
+        iwc.setCodec(
+            new FilterCodec(codec.getName(), codec) {
+              @Override
+              public KnnVectorsFormat knnVectorsFormat() {
+                return format;
+              }
+            });

Review Comment:
   So, this test would have the merge throw on missing fields with this change. 
Now, we protect against this and this test no longer throws on missing fields.



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