mayya-sharipova commented on a change in pull request #2186: URL: https://github.com/apache/lucene-solr/pull/2186#discussion_r595568938
########## File path: lucene/core/src/java/org/apache/lucene/index/FieldInfos.java ########## @@ -481,205 +433,102 @@ synchronized int addOrGet( + fieldName + "] as non-soft-deletes already"); } - return fieldNumber.intValue(); } - synchronized void verifyConsistent(Integer number, String name, IndexOptions indexOptions) { - if (name.equals(numberToName.get(number)) == false) { - throw new IllegalArgumentException( - "field number " - + number - + " is already mapped to field name \"" - + numberToName.get(number) - + "\", not \"" - + name - + "\""); - } - if (number.equals(nameToNumber.get(name)) == false) { - throw new IllegalArgumentException( - "field name \"" - + name - + "\" is already mapped to field number \"" - + nameToNumber.get(name) - + "\", not \"" - + number - + "\""); - } - IndexOptions currentIndexOptions = this.indexOptions.get(name); - if (indexOptions != IndexOptions.NONE - && currentIndexOptions != null - && currentIndexOptions != IndexOptions.NONE - && indexOptions != currentIndexOptions) { - throw new IllegalArgumentException( - "cannot change field \"" - + name - + "\" from index options=" - + currentIndexOptions - + " to inconsistent index options=" - + indexOptions); - } - } - - synchronized void verifyConsistent(Integer number, String name, DocValuesType dvType) { - if (name.equals(numberToName.get(number)) == false) { - throw new IllegalArgumentException( - "field number " - + number - + " is already mapped to field name \"" - + numberToName.get(number) - + "\", not \"" - + name - + "\""); - } - if (number.equals(nameToNumber.get(name)) == false) { - throw new IllegalArgumentException( - "field name \"" - + name - + "\" is already mapped to field number \"" - + nameToNumber.get(name) - + "\", not \"" - + number - + "\""); - } - DocValuesType currentDVType = docValuesType.get(name); - if (dvType != DocValuesType.NONE - && currentDVType != null - && currentDVType != DocValuesType.NONE - && dvType != currentDVType) { - throw new IllegalArgumentException( - "cannot change DocValues type from " - + currentDVType - + " to " - + dvType - + " for field \"" - + name - + "\""); - } - } - - synchronized void verifyConsistentDimensions( - Integer number, - String name, - int dataDimensionCount, + private void verifySameSchema( + String fieldName, + IndexOptions indexOptions, + boolean storeTermVector, + boolean omitNorms, + DocValuesType dvType, + int dimensionCount, int indexDimensionCount, - int dimensionNumBytes) { - if (name.equals(numberToName.get(number)) == false) { - throw new IllegalArgumentException( - "field number " - + number - + " is already mapped to field name \"" - + numberToName.get(number) - + "\", not \"" - + name - + "\""); - } - if (number.equals(nameToNumber.get(name)) == false) { - throw new IllegalArgumentException( - "field name \"" - + name - + "\" is already mapped to field number \"" - + nameToNumber.get(name) - + "\", not \"" - + number - + "\""); - } - FieldDimensions dim = dimensions.get(name); - if (dim != null) { - if (dim.dimensionCount != dataDimensionCount) { - throw new IllegalArgumentException( - "cannot change point dimension count from " - + dim.dimensionCount - + " to " - + dataDimensionCount - + " for field=\"" - + name - + "\""); - } - if (dim.indexDimensionCount != indexDimensionCount) { - throw new IllegalArgumentException( - "cannot change point index dimension count from " - + dim.indexDimensionCount - + " to " - + indexDimensionCount - + " for field=\"" - + name - + "\""); - } - if (dim.dimensionNumBytes != dimensionNumBytes) { - throw new IllegalArgumentException( - "cannot change point numBytes from " - + dim.dimensionNumBytes - + " to " - + dimensionNumBytes - + " for field=\"" - + name - + "\""); - } - } + int dimensionNumBytes, + int vectorDimension, + VectorValues.SearchStrategy searchStrategy) { + + IndexOptions currentOpts = this.indexOptions.get(fieldName); + verifySameIndexOptions(fieldName, currentOpts, indexOptions); + if (currentOpts != IndexOptions.NONE) { + boolean curStoreTermVector = this.storeTermVectors.get(fieldName); + verifySameStoreTermVectors(fieldName, curStoreTermVector, storeTermVector); + boolean curOmitNorms = this.omitNorms.get(fieldName); + verifySameOmitNorms(fieldName, curOmitNorms, omitNorms); + } + + DocValuesType currentDVType = docValuesType.get(fieldName); + verifySameDocValuesType(fieldName, currentDVType, dvType); + + FieldDimensions dims = dimensions.get(fieldName); + verifySamePointsOptions( + fieldName, + dims.dimensionCount, + dims.indexDimensionCount, + dims.dimensionNumBytes, + dimensionCount, + indexDimensionCount, + dimensionNumBytes); + + FieldVectorProperties props = vectorProps.get(fieldName); + verifySameVectorOptions( + fieldName, props.numDimensions, props.searchStrategy, vectorDimension, searchStrategy); } - synchronized void verifyConsistentVectorProperties( - Integer number, - String name, - int numDimensions, - VectorValues.SearchStrategy searchStrategy) { - if (name.equals(numberToName.get(number)) == false) { - throw new IllegalArgumentException( - "field number " - + number - + " is already mapped to field name \"" - + numberToName.get(number) - + "\", not \"" - + name - + "\""); - } - if (number.equals(nameToNumber.get(name)) == false) { - throw new IllegalArgumentException( - "field name \"" - + name - + "\" is already mapped to field number \"" - + nameToNumber.get(name) - + "\", not \"" - + number - + "\""); - } - FieldVectorProperties props = vectorProps.get(name); - if (props != null) { - if (props.numDimensions != numDimensions) { - throw new IllegalArgumentException( - "cannot change vector dimension from " - + props.numDimensions - + " to " - + numDimensions - + " for field=\"" - + name - + "\""); - } - if (props.searchStrategy != searchStrategy) { - throw new IllegalArgumentException( - "cannot change vector search strategy from " - + props.searchStrategy - + " to " - + searchStrategy - + " for field=\"" - + name - + "\""); - } - } + /** + * Returns true if the {@code fieldName} exists in the map and is of the same {@code dvType}, + * and it is docValues only field. + */ + synchronized boolean containsDvOnlyField(String fieldName, DocValuesType dvType) { + if (nameToNumber.containsKey(fieldName) == false) return false; + if (dvType != docValuesType.get(fieldName)) return false; + FieldDimensions fdimensions = dimensions.get(fieldName); + if (fdimensions != null && fdimensions.dimensionCount != 0) return false; + IndexOptions ioptions = indexOptions.get(fieldName); + if (ioptions != null && ioptions != IndexOptions.NONE) return false; + FieldVectorProperties fvp = vectorProps.get(fieldName); + if (fvp != null && fvp.numDimensions != 0) return false; + return true; } /** - * Returns true if the {@code fieldName} exists in the map and is of the same {@code dvType}. + * Construct a new FieldInfo based on the options in global field numbers This method is not + * synchronized as all the options it uses are not modifiable. + * + * @param fieldName name of the field + * @param dvType doc values type + * @param newFieldNumber a new field number + * @return {@code null} if {@code fieldName} doesn't exist in the map or is not of the same + * {@code dvType} returns a new FieldInfo based based on the options in global field numbers */ - synchronized boolean contains(String fieldName, DocValuesType dvType) { - // used by IndexWriter.updateNumericDocValue - if (!nameToNumber.containsKey(fieldName)) { - return false; - } else { - // only return true if the field has the same dvType as the requested one - return dvType == docValuesType.get(fieldName); - } + FieldInfo constructFieldInfo(String fieldName, DocValuesType dvType, int newFieldNumber) { + Integer fieldNumber; + synchronized (this) { + fieldNumber = nameToNumber.get(fieldName); + } + if (fieldNumber == null) return null; + DocValuesType dvType0 = docValuesType.get(fieldName); + if (dvType != dvType0) return null; + + boolean isSoftDeletesField = fieldName.equals(softDeletesFieldName); + FieldDimensions dims = dimensions.get(fieldName); + FieldVectorProperties vectors = vectorProps.get(fieldName); Review comment: Addressed [here](https://github.com/apache/lucene/pull/11/commits/0fe3493110ac2a5f750ad41f732436daff6c69f5#diff-b81932cf39b70347481b9659dd55b9b373427c2992cdacdfbc10ff8f4ea9eec0L504) ---------------------------------------------------------------- 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