jpountz commented on a change in pull request #2212:
URL: https://github.com/apache/lucene-solr/pull/2212#discussion_r559039707
##########
File path: lucene/core/src/java/org/apache/lucene/index/DirectoryReader.java
##########
@@ -104,6 +105,17 @@ public static DirectoryReader open(final IndexCommit
commit) throws IOException
return StandardDirectoryReader.open(commit.getDirectory(), commit);
}
+ /**
+ * Expert: returns an IndexReader reading the index in the given {@link
IndexCommit}.
+ *
+ * @param commit the commit point to open
+ * @param minIndexVersionCreated the minimum index version created to check
before opening the index
Review comment:
I like how `listCommits` calls it a "major" version which avoids some
ambiguity, maybe do the same here?
Also could you improve javadocs to better describe how this version
parameter affects the behavior of this method?
##########
File path: lucene/core/src/java/org/apache/lucene/index/LeafMetaData.java
##########
@@ -31,15 +31,19 @@
private final Sort sort;
/** Expert: Sole constructor. Public for use by custom {@link LeafReader}
impls. */
- public LeafMetaData(int createdVersionMajor, Version minVersion, Sort sort) {
+ public LeafMetaData(int createdVersionMajor, Version minVersion, int
minVersionSupported, Sort sort) {
this.createdVersionMajor = createdVersionMajor;
+ if (minVersionSupported > Version.LATEST.major || minVersionSupported < 0)
{
+ throw new IllegalArgumentException("minVersionSupported must be positive
and <= "
+ + Version.LATEST.major + " but was: " + minVersionSupported);
+ }
if (createdVersionMajor > Version.LATEST.major) {
throw new IllegalArgumentException(
"createdVersionMajor is in the future: " + createdVersionMajor);
}
- if (createdVersionMajor < 6) {
+ if (createdVersionMajor < minVersionSupported) {
throw new IllegalArgumentException(
- "createdVersionMajor must be >= 6, got: " + createdVersionMajor);
+ "createdVersionMajor must be >= " + minVersionSupported + ", got: "
+ createdVersionMajor);
Review comment:
These checks were introduced in the first place because 7.0 was the
version that started recording the major version that created the index, and
any older index would have this constant set to 6, rather than because 6 was
the minimum supported version. So I think we can keep the logic how it is today
without adding a new parameter to the ctor?
##########
File path:
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java
##########
@@ -1986,4 +1989,36 @@ static BytesRef toBytes(long value) {
bytes.bytes[bytes.length++] = (byte) value;
return bytes;
}
+
+ public void testFailOpenOldIndex() throws IOException {
+ for (String name : oldNames) {
+ Directory directory = oldIndexDirs.get(name);
+ IndexCommit commit = DirectoryReader.listCommits(directory).get(0);
+ IndexFormatTooOldException ex =
expectThrows(IndexFormatTooOldException.class, () ->
+ StandardDirectoryReader.open(commit, Version.LATEST.major));
+ ex.getMessage().contains("only supports reading from version " +
Version.LATEST.major + " upwards." );
Review comment:
this should be within an `assertTrue`?
##########
File path:
lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -40,42 +41,49 @@
final SegmentInfos segmentInfos;
private final boolean applyAllDeletes;
private final boolean writeAllDeletes;
+ private final int minVersionCreated;
/** called only from static open() methods */
StandardDirectoryReader(
- Directory directory,
- LeafReader[] readers,
- IndexWriter writer,
- SegmentInfos sis,
- boolean applyAllDeletes,
- boolean writeAllDeletes)
+ Directory directory,
+ LeafReader[] readers,
+ IndexWriter writer,
+ SegmentInfos sis,
+ boolean applyAllDeletes,
+ boolean writeAllDeletes, int minVersionCreated)
Review comment:
put on a new line like other parameters?
----------------------------------------------------------------
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
