lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Muir <rcm...@gmail.com>
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
Date Sun, 14 Sep 2014 11:58:14 GMT
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 <uwe@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: uwe@thetaphi.de
>
>
>> -----Original Message-----
>> From: Michael McCandless [mailto:lucene@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 <ryan@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-unsubscribe@lucene.apache.org For additional
>> commands, e-mail: dev-help@lucene.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message