I agree, can we remove all the checks here? They are bogus in a few ways: 1. Version.parse claims its forwards compatible, but the ctor throws these exceptions. 2. These exceptions are thrown without indicating what the actual value was. 3. These exceptions are thrown with no context (the toString of the input file itself).
So, I don't think Version should do these checks, at least, if it wants to do them, it needs to step up to the plate and do them correctly. On Sun, Sep 14, 2014 at 7:19 AM, Uwe Schindler <[email protected]> wrote: > Especially, as I said in my previous email: ist in my opinion bad to throw > IllegalArgumentException on version's parse if it is a vlaid version. > > I tried it locally and created an Index with version 6.0 written to disk (I > hacked it). If I try top open it with IndexReader of Lucene 4, so it should > throw IndexTooNewException! But in fact it did not, because Version.parse() > failed earlier! So we are inconsistent with Exceptions! It should in fact > really throw IndexFormatTooNewException! > > In my opinion, the bounds checks in the Version ctor should be "optional", so > when parsing versions from index files we have a chance to throw the "correct > exception". > > Uwe > > ----- > Uwe Schindler > H.-H.-Meier-Allee 63, D-28213 Bremen > http://www.thetaphi.de > eMail: [email protected] > > >> -----Original Message----- >> From: Michael McCandless [mailto:[email protected]] >> Sent: Sunday, September 14, 2014 12:46 PM >> To: Lucene/Solr dev >> Subject: Re: svn commit: r1624773 - in /lucene/dev/branches/branch_4x: ./ >> lucene/ lucene/core/ >> lucene/core/src/java/org/apache/lucene/codecs/lucene46/Lucene46Segme >> ntInfoWriter.java >> >> On Sat, Sep 13, 2014 at 10:23 PM, Ryan Ernst <[email protected]> wrote: >> > 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)? >> >> I think it makes sense to be defensive here and have the check in two places. >> >> This also guards against any future changes to Version that somehow allow >> this ... it sure look like Version can't ever be created in an "out of >> bounds" >> state today, but who knows tomorrow ... >> >> Mike McCandless >> >> http://blog.mikemccandless.com >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] For additional >> commands, e-mail: [email protected] > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
