Lucene - Core
  1. Lucene - Core
  2. LUCENE-4306

Make IR.getSequentialSubReaders() protected

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 4.0-BETA
    • Fix Version/s: 4.0, 5.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
          Uwe Schindler added a comment -

          Closed after release.

          Show
          Uwe Schindler added a comment - Closed after release.
          Hide
          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 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
          Uwe Schindler added a comment -

          Committed branch_4x revision: 1372871

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

          Committed trunk revision: 1372866
          Backporting...

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

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

          Show
          Uwe Schindler added a comment - OK, I changed the cocommit to a TODO and will commit this soon! Thanks for review.
          Hide
          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
          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
          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
          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
          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
          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:
              Uwe Schindler
              Reporter:
              Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development