[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2088: LUCENE-9617: Reset lowestUnassignedFieldNumber in FieldNumbers.clear()

[GitBox <gi...@apache.org>]

dsmiley commented on a change in pull request #2088:
URL: https://github.com/apache/lucene-solr/pull/2088#discussion_r526893867



##########
File path: lucene/CHANGES.txt
##########
@@ -184,6 +184,9 @@ Bug fixes
 * LUCENE-9365: FuzzyQuery was missing matches when prefix length was equal to the term length
   (Mark Harwood, Mike Drob)
 
+* LUCENE-9617: Reset lowestUnassignedFieldNumber on FieldNumbers.clear(), to avoid leaking

Review comment:
       Can you rewrite in terms of what a user might understand?  e.g.
   `IndexWriter.deleteAll now resets internal field numbers; prevents ever-increasing numbers in unusual use-cases`
   The latter part of what you wrote isn't bad but the first part is technical mumbo-jumbo that only Lucene deep divers would even recognize.

##########
File path: lucene/core/src/test/org/apache/lucene/index/TestFieldInfos.java
##########
@@ -187,4 +187,23 @@ public void testMergedFieldInfos_singleLeaf() throws IOException {
     writer.close();
     dir.close();
   }
+
+  public void testFieldNumbersAutoIncrement() {
+    FieldInfos.FieldNumbers fieldNumbers = new FieldInfos.FieldNumbers("softDeletes");
+    for (int i = 0; i < 10; i++) {
+      fieldNumbers.addOrGet("field" + i, -1, IndexOptions.NONE, DocValuesType.NONE,
+          0, 0, 0, 0,
+          VectorValues.SearchStrategy.NONE, false);
+    }
+    int idx = fieldNumbers.addOrGet("EleventhField", -1, IndexOptions.NONE, DocValuesType.NONE,
+        0, 0, 0, 0,
+        VectorValues.SearchStrategy.NONE, false);
+    assertEquals("Field numbers 0 through 9 were allocated", 10, idx);
+
+    fieldNumbers.clear();

Review comment:
       My only problem with unit tests like this is that it doesn't test what we _really_ want to know -- that when IW.deleteAll() is called (a user level thing, fieldNumbers.clear() is not), that the field numbers get reset.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org