mayya-sharipova commented on a change in pull request #2186:
URL: https://github.com/apache/lucene-solr/pull/2186#discussion_r595568561
##########
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.
Review comment:
From the point of view `FieldInfos.FieldNumbers`, doc values type is
not modifiable and set only once on adding this field to global field numbers.
This method `constructFieldInfo` also checks the the provided in arguments
`dvType` matches the one stored in the global field numbers for this field. So
I am not sure how it can be modifiable.
Please let me know if I am missing something.
----------------------------------------------------------------
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
