Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-4306

Make IR.getSequentialSubReaders() protected

    Details

    • Type: Task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 4.0-BETA
    • Fix Version/s: 4.0, 6.0
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Followup from LUCENE-4152: The method is/should now only be used from CompositeReader's reader context to initialize itsself. No need to call it from anywhere else.

      1. LUCENE-4306.patch
        41 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          thetaphi Uwe Schindler added a comment -

          Closed after release.

          Show
          thetaphi Uwe Schindler added a comment - Closed after release.
          Hide
          commit-tag-bot Commit Tag Bot added a comment -

          [branch_4x commit] Robert Muir
          http://svn.apache.org/viewvc?view=revision&revision=1384002

          LUCENE-4306: dont upgrade this method to public in BaseCompositeReader

          Show
          commit-tag-bot Commit Tag Bot added a comment - [branch_4x commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1384002 LUCENE-4306 : dont upgrade this method to public in BaseCompositeReader
          Hide
          thetaphi Uwe Schindler added a comment -

          Committed branch_4x revision: 1372871

          Show
          thetaphi Uwe Schindler added a comment - Committed branch_4x revision: 1372871
          Hide
          thetaphi Uwe Schindler added a comment -

          Committed trunk revision: 1372866
          Backporting...

          Show
          thetaphi Uwe Schindler added a comment - Committed trunk revision: 1372866 Backporting...
          Hide
          thetaphi Uwe Schindler added a comment -

          OK, I changed the cocommit to a TODO and will commit this soon! Thanks for review.

          Show
          thetaphi Uwe Schindler added a comment - OK, I changed the cocommit to a TODO and will commit this soon! Thanks for review.
          Hide
          rcmuir Robert Muir added a comment -

          I dont think renaming it accomplishes anything. I would just make it protected here while the patch still applies

          Show
          rcmuir Robert Muir added a comment - I dont think renaming it accomplishes anything. I would just make it protected here while the patch still applies
          Hide
          thetaphi Uwe Schindler added a comment -

          Should we rename getSequentialSubReaders? It's protected and not for general use, only subclasses of CompositeReader need to implement it. getSubReaders?

          Show
          thetaphi Uwe Schindler added a comment - Should we rename getSequentialSubReaders? It's protected and not for general use, only subclasses of CompositeReader need to implement it. getSubReaders?
          Hide
          thetaphi Uwe Schindler added a comment -

          Patch for trunk:

          • Makes getSequentialSubReaders() protected
          • Converts all tests to use leaves() when dealing with DirectoryReaders - stupid casting is gone!
          • Facet code used getSequentialSubreaders very often including summing up docBases. This is not needed, the code now uses AtomicReaderContext and its docBase -> much cleaner code, easier to read

          Problems:

          • FieldCacheSanityChecker previously looked into subreaders using getSequentialSubReaders, which did not throw AlreadyClosedException. But getTopReaderContext() throws this exception (which is important to prevent readers from being used by searchers when closed), so the sanity checker can no longer visit closed readers - which it should not do in my opinion.
          Show
          thetaphi Uwe Schindler added a comment - Patch for trunk: Makes getSequentialSubReaders() protected Converts all tests to use leaves() when dealing with DirectoryReaders - stupid casting is gone! Facet code used getSequentialSubreaders very often including summing up docBases. This is not needed, the code now uses AtomicReaderContext and its docBase -> much cleaner code, easier to read Problems: FieldCacheSanityChecker previously looked into subreaders using getSequentialSubReaders, which did not throw AlreadyClosedException. But getTopReaderContext() throws this exception (which is important to prevent readers from being used by searchers when closed), so the sanity checker can no longer visit closed readers - which it should not do in my opinion.

            People

            • Assignee:
              thetaphi Uwe Schindler
              Reporter:
              thetaphi Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development