msokolov commented on code in PR #14607:
URL: https://github.com/apache/lucene/pull/14607#discussion_r2461040067


##########
lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java:
##########
@@ -410,6 +398,85 @@ private static void parseSegmentInfos(
       SegmentInfo info =
           codec.segmentInfoFormat().read(directory, segName, segmentID, 
IOContext.READONCE);
       info.setCodec(codec);
+      Version segMinVersion = info.getMinVersion();

Review Comment:
   is it a functional change to move this version checking early in the 
function? It makes reviewing harder since the diffs look bigger and one has to 
carefully read and scroll around to compare by eyeball, so if it's not needed, 
let's leave it where it was.



##########
lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java:
##########
@@ -410,6 +398,85 @@ private static void parseSegmentInfos(
       SegmentInfo info =
           codec.segmentInfoFormat().read(directory, segName, segmentID, 
IOContext.READONCE);
       info.setCodec(codec);
+      Version segMinVersion = info.getMinVersion();
+      Version segmentVersion = info.getVersion();
+
+      if (!segmentVersion.onOrAfter(infos.minSegmentLuceneVersion)) {

Review Comment:
   we prefer `== false` to `!` for clarity - it helps to avoid mistakes



##########
lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java:
##########
@@ -410,6 +398,85 @@ private static void parseSegmentInfos(
       SegmentInfo info =
           codec.segmentInfoFormat().read(directory, segName, segmentID, 
IOContext.READONCE);
       info.setCodec(codec);
+      Version segMinVersion = info.getMinVersion();
+      Version segmentVersion = info.getVersion();
+
+      if (!segmentVersion.onOrAfter(infos.minSegmentLuceneVersion)) {
+        throw new CorruptIndexException(
+            "segments file recorded minSegmentLuceneVersion="
+                + infos.minSegmentLuceneVersion
+                + " but segment="
+                + info
+                + " has older version="
+                + segmentVersion,
+            input);
+      }
+
+      if (infos.indexCreatedVersionMajor >= 7
+          && segmentVersion.major < infos.indexCreatedVersionMajor) {
+        throw new CorruptIndexException(
+            "segments file recorded indexCreatedVersionMajor="
+                + infos.indexCreatedVersionMajor
+                + " but segment="
+                + info
+                + " has older version="
+                + segmentVersion,
+            input);
+      }
+
+      if (infos.indexCreatedVersionMajor >= 7 && segMinVersion == null) {
+        throw new CorruptIndexException(
+            "segments infos must record minVersion with 
indexCreatedVersionMajor="
+                + infos.indexCreatedVersionMajor,
+            input);
+      }
+
+      // if trying to open some random old index (< Lucene 7)
+      if (segMinVersion == null) {
+        if (infos.indexCreatedVersionMajor < minSupportedMajorVersion) {
+          throw new IndexFormatTooOldException(
+              input,
+              "Index created with Lucene "
+                  + infos.indexCreatedVersionMajor
+                  + ".x is not supported by Lucene "
+                  + Version.LATEST
+                  + ". This Lucene version only supports indexes created with 
major version "
+                  + minSupportedMajorVersion
+                  + " or later (found: "
+                  + infos.indexCreatedVersionMajor
+                  + ", minimum: "
+                  + minSupportedMajorVersion
+                  + "). To resolve this issue: (1) Re-index your data using 
Lucene "
+                  + Version.LATEST.major
+                  + ".x, or (2) Use an older Lucene version that supports your 
index format.");
+        } else {
+          throw new CorruptIndexException(
+              "segments infos must record minVersion with 
indexCreatedVersionMajor="
+                  + infos.indexCreatedVersionMajor,
+              input);
+        }
+      }
+
+      if (segMinVersion.major < minSupportedMajorVersion) {
+        throw new IndexFormatTooOldException(
+            input,
+            "Index has segment traces from Lucene version "
+                + segMinVersion.major
+                + ".x and is not supported by Lucene "
+                + Version.LATEST
+                + ". This Lucene version only supports indexes with major 
version "
+                + minSupportedMajorVersion
+                + " or later (found: "
+                + segMinVersion.major
+                + ", minimum supported: "
+                + minSupportedMajorVersion
+                + "). To resolve this issue: (1) Re-index your data using 
Lucene "
+                + minSupportedMajorVersion
+                + ".x or later (preferably "

Review Comment:
   Let's strike "preferably" and say explicitly what is allowed



##########
lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java:
##########
@@ -410,6 +398,85 @@ private static void parseSegmentInfos(
       SegmentInfo info =
           codec.segmentInfoFormat().read(directory, segName, segmentID, 
IOContext.READONCE);
       info.setCodec(codec);
+      Version segMinVersion = info.getMinVersion();
+      Version segmentVersion = info.getVersion();
+
+      if (!segmentVersion.onOrAfter(infos.minSegmentLuceneVersion)) {
+        throw new CorruptIndexException(
+            "segments file recorded minSegmentLuceneVersion="
+                + infos.minSegmentLuceneVersion
+                + " but segment="
+                + info
+                + " has older version="
+                + segmentVersion,
+            input);
+      }
+
+      if (infos.indexCreatedVersionMajor >= 7
+          && segmentVersion.major < infos.indexCreatedVersionMajor) {
+        throw new CorruptIndexException(
+            "segments file recorded indexCreatedVersionMajor="
+                + infos.indexCreatedVersionMajor
+                + " but segment="
+                + info
+                + " has older version="
+                + segmentVersion,
+            input);
+      }
+
+      if (infos.indexCreatedVersionMajor >= 7 && segMinVersion == null) {
+        throw new CorruptIndexException(
+            "segments infos must record minVersion with 
indexCreatedVersionMajor="
+                + infos.indexCreatedVersionMajor,
+            input);
+      }
+
+      // if trying to open some random old index (< Lucene 7)
+      if (segMinVersion == null) {
+        if (infos.indexCreatedVersionMajor < minSupportedMajorVersion) {
+          throw new IndexFormatTooOldException(
+              input,
+              "Index created with Lucene "
+                  + infos.indexCreatedVersionMajor
+                  + ".x is not supported by Lucene "
+                  + Version.LATEST
+                  + ". This Lucene version only supports indexes created with 
major version "
+                  + minSupportedMajorVersion
+                  + " or later (found: "
+                  + infos.indexCreatedVersionMajor
+                  + ", minimum: "
+                  + minSupportedMajorVersion
+                  + "). To resolve this issue: (1) Re-index your data using 
Lucene "
+                  + Version.LATEST.major
+                  + ".x, or (2) Use an older Lucene version that supports your 
index format.");
+        } else {
+          throw new CorruptIndexException(
+              "segments infos must record minVersion with 
indexCreatedVersionMajor="
+                  + infos.indexCreatedVersionMajor,
+              input);
+        }
+      }
+
+      if (segMinVersion.major < minSupportedMajorVersion) {
+        throw new IndexFormatTooOldException(
+            input,
+            "Index has segment traces from Lucene version "

Review Comment:
   not sure what you mean by "traces" here? Maybe like "traces of an older 
version"? I think it will be clearer to just say "Index segment derived from 
version "? 



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to