mikemccand commented on code in PR #13398:
URL: https://github.com/apache/lucene/pull/13398#discussion_r1610001429
##########
lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java:
##########
@@ -202,6 +183,89 @@ public boolean keepFullyDeletedSegment(
dir.close();
}
+ public void testDeleteByQueries() throws IOException {
+ Directory dir = newDirectory();
+ IndexWriter writer =
+ new IndexWriter(
+ dir,
+ new IndexWriterConfig(new MockAnalyzer(random()))
+ .setMergePolicy(NoMergePolicy.INSTANCE));
+
+ Document doc;
+ for (int i = 0; i < 10; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+
+ for (int i = 10; i < 20; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+
+ for (int i = 20; i < 30; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+ writer.deleteDocuments(
Review Comment:
Thank you for the added test cases! But, is this code really executing the
new early break? Shouldn't you also delete by a redundant query matching all
docs in the 2nd segment to tickle that new code?
##########
lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java:
##########
@@ -202,6 +183,89 @@ public boolean keepFullyDeletedSegment(
dir.close();
}
+ public void testDeleteByQueries() throws IOException {
+ Directory dir = newDirectory();
+ IndexWriter writer =
+ new IndexWriter(
+ dir,
+ new IndexWriterConfig(new MockAnalyzer(random()))
+ .setMergePolicy(NoMergePolicy.INSTANCE));
+
+ Document doc;
+ for (int i = 0; i < 10; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+
+ for (int i = 10; i < 20; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+
+ for (int i = 20; i < 30; i++) {
+ doc = new Document();
+ doc.add(new LongField("content", i, Field.Store.NO));
+ writer.addDocument(doc);
+ }
+ writer.flush();
+ writer.deleteDocuments(
+ LongPoint.newRangeQuery("content", 10, 19),
LongPoint.newRangeQuery("content", 12, 15));
+
+ DirectoryReader reader = DirectoryReader.open(writer);
+ IndexSearcher searcher = newSearcher(reader);
+ assertEquals(
+ 0, searcher.search(LongPoint.newRangeQuery("content", 10, 19),
100).totalHits.value);
+ assertEquals(
+ 20, searcher.search(LongPoint.newRangeQuery("content", 0, 30),
100).totalHits.value);
+
+ reader.close();
+ writer.close();
+ dir.close();
+ }
+
+ public void testDeleteByTerms() throws IOException {
+ Directory dir = newDirectory();
+ IndexWriter writer =
+ new IndexWriter(
+ dir,
+ new IndexWriterConfig(new MockAnalyzer(random()))
+ .setMergePolicy(NoMergePolicy.INSTANCE));
+
+ Document doc;
+ doc = new Document();
+ doc.add(new TextField("f", "foo bar tea", Field.Store.NO));
+ writer.addDocument(doc);
+
Review Comment:
Maybe flush after each added document so you get three segments and then the
delTerms will tickle the new early-break code?
##########
lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java:
##########
@@ -58,28 +58,9 @@
import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.codecs.simpletext.SimpleTextCodec;
-import org.apache.lucene.document.BinaryDocValuesField;
-import org.apache.lucene.document.Document;
-import org.apache.lucene.document.Field;
-import org.apache.lucene.document.FieldType;
-import org.apache.lucene.document.LongPoint;
-import org.apache.lucene.document.NumericDocValuesField;
-import org.apache.lucene.document.SortedDocValuesField;
-import org.apache.lucene.document.SortedNumericDocValuesField;
-import org.apache.lucene.document.SortedSetDocValuesField;
-import org.apache.lucene.document.StoredField;
-import org.apache.lucene.document.StringField;
-import org.apache.lucene.document.TextField;
+import org.apache.lucene.document.*;
Review Comment:
We try to avoid wildcard imports -- I'm surprised our static checker isn't
angry about this -- could you put back all the direct imports? THanks.
--
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
