Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      IndexFormatTooOldException and IndexFormatTooNewException both extend from CorruptIndexException. But this is not a corruption, it is simply an unsupported version of an index. They should just extend IOException.

      1. LUCENE-5972.patch
        5 kB
        Ryan Ernst
      2. LUCENE-5972.patch
        5 kB
        Ryan Ernst
      3. LUCENE-5972.patch
        7 kB
        Ryan Ernst

        Activity

        Hide
        Ryan Ernst added a comment -

        Patch.

        Show
        Ryan Ernst added a comment - Patch.
        Hide
        Robert Muir added a comment -

        Why are the constructors taking datainput removed? This forces calls to toString(), which will cause NPE in some cases (whereas before, it would just concatenate "null").

        If things go corrupt, who knows what could go wrong, i think its best to avoid this.

        Show
        Robert Muir added a comment - Why are the constructors taking datainput removed? This forces calls to toString(), which will cause NPE in some cases (whereas before, it would just concatenate "null"). If things go corrupt, who knows what could go wrong, i think its best to avoid this.
        Hide
        Robert Muir added a comment -

        Also the existing logic (calling in.toString) within these Exceptions themselves is bogus today, as is the bogus assert != null.

        This stuff needs to be removed. The ctor taking datainput should just call Objects.toString() instead (see corruptindexexception), so that null will not corrupt this exception with some bogus NPE or assertionerror.

        Show
        Robert Muir added a comment - Also the existing logic (calling in.toString) within these Exceptions themselves is bogus today, as is the bogus assert != null. This stuff needs to be removed. The ctor taking datainput should just call Objects.toString() instead (see corruptindexexception), so that null will not corrupt this exception with some bogus NPE or assertionerror.
        Hide
        Ryan Ernst added a comment -

        Ok, new patch, using Objects.toString, and adding back ctors taking DataInput

        Show
        Ryan Ernst added a comment - Ok, new patch, using Objects.toString , and adding back ctors taking DataInput
        Hide
        Robert Muir added a comment -

        Looks good, can we remove this assert in both classes?

        assert resourceDesc != null;
        

        Such things do not belong in exceptions. Maybe while we are here, we can fix CorruptIndexException, to not throw NPE if its 'message' is null.

        Currently it has:

            super(message + " (resource=" + resourceDescription + ")", cause);
        

        This can just be super(Objects.toString(message) + ...

        Show
        Robert Muir added a comment - Looks good, can we remove this assert in both classes? assert resourceDesc != null ; Such things do not belong in exceptions. Maybe while we are here, we can fix CorruptIndexException, to not throw NPE if its 'message' is null. Currently it has: super (message + " (resource=" + resourceDescription + ")" , cause); This can just be super(Objects.toString(message) + ...
        Hide
        Ryan Ernst added a comment -

        Good ideas. This patch adds removing the asserts, and Objects.toString called on message in CorruptIndexException.

        Show
        Ryan Ernst added a comment - Good ideas. This patch adds removing the asserts, and Objects.toString called on message in CorruptIndexException.
        Hide
        Robert Muir added a comment -

        Looks great, thank you!

        Show
        Robert Muir added a comment - Looks great, thank you!
        Hide
        ASF subversion and git services added a comment -

        Commit 1626914 from Ryan Ernst in branch 'dev/trunk'
        [ https://svn.apache.org/r1626914 ]

        LUCENE-5972: IndexFormatTooOldException and IndexFormatTooNewException now extend from IOException

        Show
        ASF subversion and git services added a comment - Commit 1626914 from Ryan Ernst in branch 'dev/trunk' [ https://svn.apache.org/r1626914 ] LUCENE-5972 : IndexFormatTooOldException and IndexFormatTooNewException now extend from IOException
        Hide
        ASF subversion and git services added a comment -

        Commit 1626920 from Ryan Ernst in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1626920 ]

        LUCENE-5972: IndexFormatTooOldException and IndexFormatTooNewException now extend from IOException (merged 1626914)

        Show
        ASF subversion and git services added a comment - Commit 1626920 from Ryan Ernst in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1626920 ] LUCENE-5972 : IndexFormatTooOldException and IndexFormatTooNewException now extend from IOException (merged 1626914)
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

          • Assignee:
            Ryan Ernst
            Reporter:
            Ryan Ernst
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development