[GitHub] [lucene] mayya-sharipova commented on a change in pull request #11: LUCENE-9334 Consistency of field data structures

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

mayya-sharipova commented on a change in pull request #11:
URL: https://github.com/apache/lucene/pull/11#discussion_r596299038



##########
File path: lucene/core/src/java/org/apache/lucene/index/FieldInfos.java
##########
@@ -796,167 +565,97 @@ public void add(FieldInfos other) {
     /** Create a new field, or return existing one. */
     public FieldInfo getOrAdd(String name) {
       FieldInfo fi = fieldInfo(name);
-      if (fi == null) {
-        assert assertNotFinished();
-        // This field wasn't yet added to this in-RAM
-        // segment's FieldInfo, so now we get a global
-        // number for this field.  If the field was seen
-        // before then we'll get the same name and number,
-        // else we'll allocate a new one:
-        final boolean isSoftDeletesField = name.equals(globalFieldNumbers.softDeletesFieldName);
-        final int fieldNumber =
-            globalFieldNumbers.addOrGet(
-                name,
-                -1,
-                IndexOptions.NONE,
-                DocValuesType.NONE,
-                0,
-                0,
-                0,
-                0,
-                VectorValues.SearchStrategy.NONE,
-                isSoftDeletesField);
-        fi =
-            new FieldInfo(
-                name,
-                fieldNumber,
-                false,
-                false,
-                false,
-                IndexOptions.NONE,
-                DocValuesType.NONE,
-                -1,
-                new HashMap<>(),
-                0,
-                0,
-                0,
-                0,
-                VectorValues.SearchStrategy.NONE,
-                isSoftDeletesField);
-        assert !byName.containsKey(fi.name);
-        globalFieldNumbers.verifyConsistent(
-            Integer.valueOf(fi.number), fi.name, DocValuesType.NONE);
-        byName.put(fi.name, fi);
+      if (fi != null) {
+        return fi;
+      } else {
+        return addField(
+            name,
+            -1,
+            false,
+            false,
+            false,
+            IndexOptions.NONE,
+            DocValuesType.NONE,
+            -1,
+            new HashMap<>(),
+            0,
+            0,
+            0,
+            0,
+            VectorValues.SearchStrategy.NONE,
+            name.equals(globalFieldNumbers.softDeletesFieldName));
       }
+    }
 
-      return fi;
+    public FieldInfo add(FieldInfo fi) {
+      return add(fi, -1);
+    }
+
+    public FieldInfo add(FieldInfo fi, long dvGen) {
+      // IMPORTANT - reuse the field number if possible for consistent field numbers across segments
+      if (fi.getDocValuesType() == null) {
+        throw new NullPointerException("DocValuesType must not be null");
+      }
+      final FieldInfo curFi = fieldInfo(fi.name);
+      if (curFi == null) {
+        // original attributes is UnmodifiableMap
+        Map<String, String> attributes =
+            fi.attributes() == null ? null : new HashMap<>(fi.attributes());
+        return addField(
+            fi.name,
+            fi.number,
+            fi.hasVectors(),
+            fi.omitsNorms(),
+            fi.hasPayloads(),
+            fi.getIndexOptions(),
+            fi.getDocValuesType(),
+            dvGen,
+            attributes,
+            fi.getPointDimensionCount(),
+            fi.getPointIndexDimensionCount(),
+            fi.getPointNumBytes(),
+            fi.getVectorDimension(),
+            fi.getVectorSearchStrategy(),
+            fi.isSoftDeletesField());
+      } else {
+        curFi.verifySameSchema(fi, dvGen);
+        if (fi.attributes() != null) {
+          fi.attributes().forEach((k, v) -> curFi.putAttribute(k, v));
+        }
+        return curFi;
+      }
     }
 
-    private FieldInfo addOrUpdateInternal(
+    public FieldInfo add(

Review comment:
       Unfortunately I can't do anything with formatting. This is how `./gradlew :lucene:core:spotlessApply` reformats the code. 
   If I do the modifications as you suggested, `:lucene:core:spotlessJavaCheck` which is a part of `precommit` will not pass. 




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