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