You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/02/21 23:37:42 UTC

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

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



##########
File path: lucene/core/src/java/org/apache/lucene/index/FieldInfo.java
##########
@@ -130,127 +167,255 @@ 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,
+  void verifySameSchema(
+      IndexOptions indexOptions,
       boolean omitNorms,
       boolean storePayloads,
-      IndexOptions indexOptions,
-      Map<String, String> attributes,
+      boolean storeTermVector,
+      DocValuesType docValuesType,
+      long dvGen,
       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);
-      }
+      int dimensionNumBytes,
+      int vectorDimension,
+      VectorValues.SearchStrategy searchStrategy) {
+    verifySameIndexOptions(this.name, this.indexOptions, indexOptions);
+    if (this.indexOptions != IndexOptions.NONE) {
+      verifySamePostingsOptions(
+          this.name,
+          this.storeTermVector,
+          this.omitNorms,
+          this.storePayloads,
+          storeTermVector,
+          omitNorms,
+          storePayloads);
+    }
+    verifySameDocValuesType(this.name, this.docValuesType, docValuesType);
+    verifySameDVGen(this.name, this.dvGen, dvGen);
+    verifySamePointsOptions(
+        this.name,
+        this.pointDimensionCount,
+        this.pointIndexDimensionCount,
+        this.pointNumBytes,
+        dimensionCount,
+        indexDimensionCount,
+        dimensionNumBytes);
+    verifySameVectorOptions(
+        this.name,
+        this.vectorDimension,
+        this.vectorSearchStrategy,
+        vectorDimension,
+        searchStrategy);
+  }
+
+  /**
+   * Very that the provided index options are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameIndexOptions(String fieldName, IndexOptions io1, IndexOptions io2) {
+    if (io1 != io2) {
+      throw new IllegalArgumentException(
+          "cannot change field \""
+              + fieldName
+              + "\" from index options="
+              + io1
+              + " to inconsistent index options="
+              + io2);
     }
+  }
 
-    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)) {
+  /**
+   * Very that the provided docValues type are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameDocValuesType(
+      String fieldName, DocValuesType dv1, DocValuesType dv2) {
+    if (dv1 != dv2) {
       throw new IllegalArgumentException(
           "cannot change field \""
-              + name
+              + fieldName
+              + "\" from doc values type="
+              + dv1
+              + " to inconsistent doc values type="
+              + dv2);
+    }
+  }
+
+  /**
+   * Very that the provided doc values generations are the same
+   *
+   * @throws IllegalArgumentException if they are not the same
+   */
+  public static void verifySameDVGen(String fieldName, long dvg1, long dvg2) {
+    if (dvg1 != dvg2) {
+      throw new IllegalArgumentException(
+          "cannot change field \""
+              + fieldName
+              + "\" from doc values generation="
+              + dvg1
+              + " to inconsistent doc values generation="
+              + dvg2);
+    }
+  }
+
+  /**
+   * Very that the provided posting options are the same

Review comment:
       Addressed in 6e7540ebd0ef79536cffabcf0ddc7a592b792252




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