Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      A FilterDirectoryReader would allow you to easily wrap all subreaders of a DirectoryReader with FilterAtomicReaders.

      1. LUCENE-4902.patch
        8 kB
        Alan Woodward
      2. LUCENE-4902.patch
        8 kB
        Alan Woodward

        Activity

        Hide
        Alan Woodward added a comment -

        Patch adds FilterDirectoryReader, and refactors AssertingDirectorReader to use it.

        Show
        Alan Woodward added a comment - Patch adds FilterDirectoryReader, and refactors AssertingDirectorReader to use it.
        Hide
        Alan Woodward added a comment -

        If everybody's happy with this, I'll commit it this afternoon.

        Show
        Alan Woodward added a comment - If everybody's happy with this, I'll commit it this afternoon.
        Hide
        Adrien Grand added a comment -

        Maybe getCoreCacheKey and getCombinedCoreAndDeletesKey should not be forwarded? I'm afraid that some atomic filters might change the content and/or live docs of the leaves, which might make FieldCache return incorrect results. (But AssertingDirectoryReader could specialize FilterDirectoryReader to forward those method calls since it only adds assertions.)

        Show
        Adrien Grand added a comment - Maybe getCoreCacheKey and getCombinedCoreAndDeletesKey should not be forwarded? I'm afraid that some atomic filters might change the content and/or live docs of the leaves, which might make FieldCache return incorrect results. (But AssertingDirectoryReader could specialize FilterDirectoryReader to forward those method calls since it only adds assertions.)
        Hide
        Alan Woodward added a comment -

        Good point. Here's a new patch with those methods removed, and re-implemented on AssertingDR.

        Thanks for the review, Adrien! Will commit shortly.

        Show
        Alan Woodward added a comment - Good point. Here's a new patch with those methods removed, and re-implemented on AssertingDR. Thanks for the review, Adrien! Will commit shortly.
        Hide
        Adrien Grand added a comment -

        +1

        Show
        Adrien Grand added a comment - +1
        Hide
        Uwe Schindler added a comment -

        +1

        Show
        Uwe Schindler added a comment - +1
        Hide
        Uwe Schindler added a comment -

        Hi, one thing to change (maybe):
        In my opinion, the methods implemented by simply forwarding and calling doWrapDirectoryReader(in.getReader) should be final, because the whole class depends on this functionality. If you override these methods in a subclass instead of correctly implementing the "SubReaderWrapper#wrap()" inner class and "doWrapDirectoryReader()", you break the whole reopen logic. You should do the work only in the abstract doWrapDirectoryReader() method is combination with the SubReaderWrapper. So the methods for doOpenIfChanged(...) should be made final to prevent mis-use.

        Show
        Uwe Schindler added a comment - Hi, one thing to change (maybe): In my opinion, the methods implemented by simply forwarding and calling doWrapDirectoryReader(in.getReader) should be final, because the whole class depends on this functionality. If you override these methods in a subclass instead of correctly implementing the "SubReaderWrapper#wrap()" inner class and "doWrapDirectoryReader()", you break the whole reopen logic. You should do the work only in the abstract doWrapDirectoryReader() method is combination with the SubReaderWrapper. So the methods for doOpenIfChanged(...) should be made final to prevent mis-use.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] romseygeek
        http://svn.apache.org/viewvc?view=revision&revision=1465072

        LUCENE-4902: Add FilterDirectoryReader

        Show
        Commit Tag Bot added a comment - [trunk commit] romseygeek http://svn.apache.org/viewvc?view=revision&revision=1465072 LUCENE-4902 : Add FilterDirectoryReader
        Hide
        Alan Woodward added a comment -

        Committed. I made the doOpenIfChanged methods and wrapDirectoryReader() final as well. Thanks Uwe!

        Show
        Alan Woodward added a comment - Committed. I made the doOpenIfChanged methods and wrapDirectoryReader() final as well. Thanks Uwe!
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Alan Woodward
            Reporter:
            Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development