I'll open an issue for this; I think it should block 4.10.1. Mike McCandless
http://blog.mikemccandless.com On Sun, Sep 14, 2014 at 8:21 AM, Robert Muir <rcm...@gmail.com> wrote: > Also if there are going to be checks against this, if its really going > to become the primary entry point for deciding the version of a > segment and what exception to throw and so on, then I strongly > disagree with string encoding. > > In my opinion we already have such a scheme figured out (CodecUtil > methods) with exception handling and everything... > > On Sun, Sep 14, 2014 at 7:58 AM, Robert Muir <rcm...@gmail.com> wrote: >> 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 <u...@thetaphi.de> 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: u...@thetaphi.de >>> >>> >>>> -----Original Message----- >>>> From: Michael McCandless [mailto:luc...@mikemccandless.com] >>>> 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 <r...@iernst.net> 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: dev-unsubscr...@lucene.apache.org For additional >>>> commands, e-mail: dev-h...@lucene.apache.org >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org >>> For additional commands, e-mail: dev-h...@lucene.apache.org >>> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
