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



##########
File path: lucene/core/src/java/org/apache/lucene/index/FieldInfo.java
##########
@@ -130,127 +167,252 @@ public boolean checkConsistency() {
       }
     }
 
-    if (pointDimensionCount < 0) {
+    if (docValuesType == null) {
+      throw new IllegalStateException("DocValuesType must not be null (field: 
'" + name + "')");
+    }
+    if (dvGen != -1 && docValuesType == DocValuesType.NONE) {
       throw new IllegalStateException(
-          "pointDimensionCount must be >= 0; got " + pointDimensionCount);
+          "field '"
+              + name
+              + "' cannot have a docvalues update generation without having 
docvalues");
     }
 
+    if (pointDimensionCount < 0) {
+      throw new IllegalStateException(
+          "pointDimensionCount must be >= 0; got "
+              + pointDimensionCount
+              + " (field: '"
+              + name
+              + "')");
+    }
     if (pointIndexDimensionCount < 0) {
       throw new IllegalStateException(
-          "pointIndexDimensionCount must be >= 0; got " + 
pointIndexDimensionCount);
+          "pointIndexDimensionCount must be >= 0; got "
+              + pointIndexDimensionCount
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointNumBytes < 0) {
-      throw new IllegalStateException("pointNumBytes must be >= 0; got " + 
pointNumBytes);
+      throw new IllegalStateException(
+          "pointNumBytes must be >= 0; got " + pointNumBytes + " (field: '" + 
name + "')");
     }
 
     if (pointDimensionCount != 0 && pointNumBytes == 0) {
       throw new IllegalStateException(
-          "pointNumBytes must be > 0 when pointDimensionCount=" + 
pointDimensionCount);
+          "pointNumBytes must be > 0 when pointDimensionCount="
+              + pointDimensionCount
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointIndexDimensionCount != 0 && pointDimensionCount == 0) {
       throw new IllegalStateException(
-          "pointIndexDimensionCount must be 0 when pointDimensionCount=0");
+          "pointIndexDimensionCount must be 0 when pointDimensionCount=0"
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointNumBytes != 0 && pointDimensionCount == 0) {
       throw new IllegalStateException(
-          "pointDimensionCount must be > 0 when pointNumBytes=" + 
pointNumBytes);
+          "pointDimensionCount must be > 0 when pointNumBytes="
+              + pointNumBytes
+              + " (field: '"
+              + name
+              + "')");
     }
 
-    if (dvGen != -1 && docValuesType == DocValuesType.NONE) {
+    if (vectorSearchStrategy == null) {
       throw new IllegalStateException(
-          "field '"
-              + name
-              + "' cannot have a docvalues update generation without having 
docvalues");
+          "Vector search strategy must not be null (field: '" + name + "')");
     }
-
     if (vectorDimension < 0) {
-      throw new IllegalStateException("vectorDimension must be >=0; got " + 
vectorDimension);
+      throw new IllegalStateException(
+          "vectorDimension must be >=0; got " + vectorDimension + " (field: '" 
+ name + "')");
     }
-
     if (vectorDimension == 0 && vectorSearchStrategy != 
VectorValues.SearchStrategy.NONE) {
       throw new IllegalStateException(
-          "vector search strategy must be NONE when dimension = 0; got " + 
vectorSearchStrategy);
+          "vector search strategy must be NONE when dimension = 0; got "
+              + vectorSearchStrategy
+              + " (field: '"
+              + name
+              + "')");
     }
-
     return true;
   }
 
-  // should only be called by FieldInfos#addOrUpdate
-  void update(
-      boolean storeTermVector,
-      boolean omitNorms,
-      boolean storePayloads,
-      IndexOptions indexOptions,
-      Map<String, String> attributes,
-      int dimensionCount,
-      int indexDimensionCount,
-      int dimensionNumBytes) {
-    if (indexOptions == null) {
-      throw new NullPointerException("IndexOptions must not be null (field: 
\"" + name + "\")");
-    }
-    // System.out.println("FI.update field=" + name + " indexed=" + indexed + 
" omitNorms=" +
-    // omitNorms + " this.omitNorms=" + this.omitNorms);
-    if (this.indexOptions != indexOptions) {
-      if (this.indexOptions == IndexOptions.NONE) {
-        this.indexOptions = indexOptions;
-      } else if (indexOptions != IndexOptions.NONE) {
-        throw new IllegalArgumentException(
-            "cannot change field \""
-                + name
-                + "\" from index options="
-                + this.indexOptions
-                + " to inconsistent index options="
-                + indexOptions);
-      }
+  void verifySameSchema(FieldInfo o, long dvGen) {
+    String fieldName = this.name;
+    verifySameIndexOptions(fieldName, this.indexOptions, o.getIndexOptions());
+    if (this.indexOptions != IndexOptions.NONE) {
+      verifySameOmitNorms(fieldName, this.omitNorms, o.omitNorms);
+      verifySameStoreTermVectors(fieldName, this.storeTermVector, 
o.storeTermVector);
+    }
+    verifySameDocValuesType(fieldName, this.docValuesType, o.docValuesType);
+    verifySameDVGen(fieldName, this.dvGen, dvGen);
+    verifySamePointsOptions(
+        fieldName,
+        this.pointDimensionCount,
+        this.pointIndexDimensionCount,
+        this.pointNumBytes,
+        o.pointDimensionCount,
+        o.pointIndexDimensionCount,
+        o.pointNumBytes);
+    verifySameVectorOptions(
+        fieldName,
+        this.vectorDimension,
+        this.vectorSearchStrategy,
+        o.vectorDimension,
+        o.vectorSearchStrategy);
+  }
+
+  /**
+   * Verify that the provided index options are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameIndexOptions(
+      String fieldName, IndexOptions indexOptions1, IndexOptions 
indexOptions2) {
+    if (indexOptions1 != indexOptions2) {
+      throw new IllegalArgumentException(
+          "cannot change field \""
+              + fieldName
+              + "\" from index options="
+              + indexOptions1
+              + " to inconsistent index options="
+              + indexOptions2);
     }
+  }
 
-    if (this.pointDimensionCount == 0 && dimensionCount != 0) {
-      this.pointDimensionCount = dimensionCount;
-      this.pointIndexDimensionCount = indexDimensionCount;
-      this.pointNumBytes = dimensionNumBytes;
-    } else if (dimensionCount != 0
-        && (this.pointDimensionCount != dimensionCount
-            || this.pointIndexDimensionCount != indexDimensionCount
-            || this.pointNumBytes != dimensionNumBytes)) {
+  /**
+   * Verify that the provided docValues type are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameDocValuesType(
+      String fieldName, DocValuesType docValuesType1, DocValuesType 
docValuesType2) {
+    if (docValuesType1 != docValuesType2) {
       throw new IllegalArgumentException(
           "cannot change field \""
-              + name
+              + fieldName
+              + "\" from doc values type="
+              + docValuesType1
+              + " to inconsistent doc values type="
+              + docValuesType2);
+    }
+  }
+
+  /**
+   * Verify that the provided doc values generations are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  // TODO: not sure if gen also must be the same
+  public static void verifySameDVGen(String fieldName, long docValuesGen1, 
long docValuesGen2) {
+    if (docValuesGen1 != docValuesGen2) {
+      throw new IllegalArgumentException(
+          "cannot change field \""
+              + fieldName
+              + "\" from doc values generation="
+              + docValuesGen1
+              + " to inconsistent doc values generation="
+              + docValuesGen2);
+    }
+  }
+
+  /**
+   * Verify that the provided store term vectors options are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameStoreTermVectors(

Review comment:
       Addressed in 0fe3493110ac2a5f750ad41f732436daff6c69f5

##########
File path: lucene/core/src/java/org/apache/lucene/index/FieldInfo.java
##########
@@ -130,127 +167,252 @@ public boolean checkConsistency() {
       }
     }
 
-    if (pointDimensionCount < 0) {
+    if (docValuesType == null) {
+      throw new IllegalStateException("DocValuesType must not be null (field: 
'" + name + "')");
+    }
+    if (dvGen != -1 && docValuesType == DocValuesType.NONE) {
       throw new IllegalStateException(
-          "pointDimensionCount must be >= 0; got " + pointDimensionCount);
+          "field '"
+              + name
+              + "' cannot have a docvalues update generation without having 
docvalues");
     }
 
+    if (pointDimensionCount < 0) {
+      throw new IllegalStateException(
+          "pointDimensionCount must be >= 0; got "
+              + pointDimensionCount
+              + " (field: '"
+              + name
+              + "')");
+    }
     if (pointIndexDimensionCount < 0) {
       throw new IllegalStateException(
-          "pointIndexDimensionCount must be >= 0; got " + 
pointIndexDimensionCount);
+          "pointIndexDimensionCount must be >= 0; got "
+              + pointIndexDimensionCount
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointNumBytes < 0) {
-      throw new IllegalStateException("pointNumBytes must be >= 0; got " + 
pointNumBytes);
+      throw new IllegalStateException(
+          "pointNumBytes must be >= 0; got " + pointNumBytes + " (field: '" + 
name + "')");
     }
 
     if (pointDimensionCount != 0 && pointNumBytes == 0) {
       throw new IllegalStateException(
-          "pointNumBytes must be > 0 when pointDimensionCount=" + 
pointDimensionCount);
+          "pointNumBytes must be > 0 when pointDimensionCount="
+              + pointDimensionCount
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointIndexDimensionCount != 0 && pointDimensionCount == 0) {
       throw new IllegalStateException(
-          "pointIndexDimensionCount must be 0 when pointDimensionCount=0");
+          "pointIndexDimensionCount must be 0 when pointDimensionCount=0"
+              + " (field: '"
+              + name
+              + "')");
     }
-
     if (pointNumBytes != 0 && pointDimensionCount == 0) {
       throw new IllegalStateException(
-          "pointDimensionCount must be > 0 when pointNumBytes=" + 
pointNumBytes);
+          "pointDimensionCount must be > 0 when pointNumBytes="
+              + pointNumBytes
+              + " (field: '"
+              + name
+              + "')");
     }
 
-    if (dvGen != -1 && docValuesType == DocValuesType.NONE) {
+    if (vectorSearchStrategy == null) {
       throw new IllegalStateException(
-          "field '"
-              + name
-              + "' cannot have a docvalues update generation without having 
docvalues");
+          "Vector search strategy must not be null (field: '" + name + "')");
     }
-
     if (vectorDimension < 0) {
-      throw new IllegalStateException("vectorDimension must be >=0; got " + 
vectorDimension);
+      throw new IllegalStateException(
+          "vectorDimension must be >=0; got " + vectorDimension + " (field: '" 
+ name + "')");
     }
-
     if (vectorDimension == 0 && vectorSearchStrategy != 
VectorValues.SearchStrategy.NONE) {
       throw new IllegalStateException(
-          "vector search strategy must be NONE when dimension = 0; got " + 
vectorSearchStrategy);
+          "vector search strategy must be NONE when dimension = 0; got "
+              + vectorSearchStrategy
+              + " (field: '"
+              + name
+              + "')");
     }
-
     return true;
   }
 
-  // should only be called by FieldInfos#addOrUpdate
-  void update(
-      boolean storeTermVector,
-      boolean omitNorms,
-      boolean storePayloads,
-      IndexOptions indexOptions,
-      Map<String, String> attributes,
-      int dimensionCount,
-      int indexDimensionCount,
-      int dimensionNumBytes) {
-    if (indexOptions == null) {
-      throw new NullPointerException("IndexOptions must not be null (field: 
\"" + name + "\")");
-    }
-    // System.out.println("FI.update field=" + name + " indexed=" + indexed + 
" omitNorms=" +
-    // omitNorms + " this.omitNorms=" + this.omitNorms);
-    if (this.indexOptions != indexOptions) {
-      if (this.indexOptions == IndexOptions.NONE) {
-        this.indexOptions = indexOptions;
-      } else if (indexOptions != IndexOptions.NONE) {
-        throw new IllegalArgumentException(
-            "cannot change field \""
-                + name
-                + "\" from index options="
-                + this.indexOptions
-                + " to inconsistent index options="
-                + indexOptions);
-      }
+  void verifySameSchema(FieldInfo o, long dvGen) {
+    String fieldName = this.name;
+    verifySameIndexOptions(fieldName, this.indexOptions, o.getIndexOptions());
+    if (this.indexOptions != IndexOptions.NONE) {
+      verifySameOmitNorms(fieldName, this.omitNorms, o.omitNorms);
+      verifySameStoreTermVectors(fieldName, this.storeTermVector, 
o.storeTermVector);
+    }
+    verifySameDocValuesType(fieldName, this.docValuesType, o.docValuesType);
+    verifySameDVGen(fieldName, this.dvGen, dvGen);
+    verifySamePointsOptions(
+        fieldName,
+        this.pointDimensionCount,
+        this.pointIndexDimensionCount,
+        this.pointNumBytes,
+        o.pointDimensionCount,
+        o.pointIndexDimensionCount,
+        o.pointNumBytes);
+    verifySameVectorOptions(
+        fieldName,
+        this.vectorDimension,
+        this.vectorSearchStrategy,
+        o.vectorDimension,
+        o.vectorSearchStrategy);
+  }
+
+  /**
+   * Verify that the provided index options are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameIndexOptions(
+      String fieldName, IndexOptions indexOptions1, IndexOptions 
indexOptions2) {
+    if (indexOptions1 != indexOptions2) {
+      throw new IllegalArgumentException(
+          "cannot change field \""
+              + fieldName
+              + "\" from index options="
+              + indexOptions1
+              + " to inconsistent index options="
+              + indexOptions2);
     }
+  }
 
-    if (this.pointDimensionCount == 0 && dimensionCount != 0) {
-      this.pointDimensionCount = dimensionCount;
-      this.pointIndexDimensionCount = indexDimensionCount;
-      this.pointNumBytes = dimensionNumBytes;
-    } else if (dimensionCount != 0
-        && (this.pointDimensionCount != dimensionCount
-            || this.pointIndexDimensionCount != indexDimensionCount
-            || this.pointNumBytes != dimensionNumBytes)) {
+  /**
+   * Verify that the provided docValues type are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameDocValuesType(
+      String fieldName, DocValuesType docValuesType1, DocValuesType 
docValuesType2) {
+    if (docValuesType1 != docValuesType2) {
       throw new IllegalArgumentException(
           "cannot change field \""
-              + name
+              + fieldName
+              + "\" from doc values type="
+              + docValuesType1
+              + " to inconsistent doc values type="
+              + docValuesType2);
+    }
+  }
+
+  /**
+   * Verify that the provided doc values generations are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  // TODO: not sure if gen also must be the same
+  public static void verifySameDVGen(String fieldName, long docValuesGen1, 
long docValuesGen2) {

Review comment:
       Addressed in 0fe3493110ac2a5f750ad41f732436daff6c69f5




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

Reply via email to