Hi,
I am not sure, if the hard checks in the ctor are future-proof: Currently we detect versions < 3.0 early when reading segments, because they have some signatures that identify them as 3.x or earlier, so we can “correctly” throw IndexFormatTooOldException (because we never try to parse a version). If we later (e.g. in version 5 or 6) drop support for those older versions, how can we correctly throw “IndexFormatTooOldException”, if the parsing of a version written to segment fails with IAE during parsing? In addition, we can never throw IndexFormatTooNewException, if we get (some time in the future) with the current 4.10 version - as released last month - such an value in a future index file. The user would get an exception, but the wrong one. Therefore, I would not restrict versions soo much in the ctor. Ideally we would throw the IndexFormatTooOld/New in ctor already (but it is an IOException, so it does fit the use-case to be thrown while parsing). Uwe ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen <http://www.thetaphi.de/> http://www.thetaphi.de eMail: [email protected] From: Ryan Ernst [mailto:[email protected]] Sent: Sunday, September 14, 2014 4:24 AM To: [email protected] Subject: Re: svn commit: r1624773 - in /lucene/dev/branches/branch_4x: ./ lucene/ lucene/core/ lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java How would the Version be constructed with an invalid major version, given this exact check in the constructor (and the fact that the only way to construct is through Version.parse)? On Sat, Sep 13, 2014 at 11:48 AM, <[email protected]> wrote: Author: mikemccand Date: Sat Sep 13 18:48:41 2014 New Revision: 1624773 URL: http://svn.apache.org/r1624773 Log: detect invalid major version when writing .si Modified: lucene/dev/branches/branch_4x/ (props changed) lucene/dev/branches/branch_4x/lucene/ (props changed) lucene/dev/branches/branch_4x/lucene/core/ (props changed) lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java?rev=1624773 <http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java?rev=1624773&r1=1624772&r2=1624773&view=diff> &r1=1624772&r2=1624773&view=diff ============================================================================== --- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java (original) +++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46SegmentInfoWriter.java Sat Sep 13 18:48:41 2014 @@ -28,6 +28,7 @@ import org.apache.lucene.store.Directory import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexOutput; import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.Version; /** * Lucene 4.0 implementation of {@link SegmentInfoWriter}. @@ -52,8 +53,12 @@ public class Lucene46SegmentInfoWriter e boolean success = false; try { CodecUtil.writeHeader(output, Lucene46SegmentInfoFormat.CODEC_NAME, Lucene46SegmentInfoFormat.VERSION_CURRENT); + Version version = si.getVersion(); + if (version.major < 3 || version.major > 4) { + throw new IllegalArgumentException("invalid major version: should be 3 or 4 but got: " + version.major + " segment=" + si); + } // Write the Lucene version that created this segment, since 3.1 - output.writeString(si.getVersion().toString()); + output.writeString(version.toString()); output.writeInt(si.getDocCount()); output.writeByte((byte) (si.getUseCompoundFile() ? SegmentInfo.YES : SegmentInfo.NO));
