mayya-sharipova commented on a change in pull request #11: URL: https://github.com/apache/lucene/pull/11#discussion_r596391496
########## 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); Review comment: We indeed can use `equals` to compare, but then we would be able to return only a general error message without the ability to specify which specific options are different. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org