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

IndexReader.listCommits should return a List and not an abstract Collection

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Activity

      Hide
      gsingers Grant Ingersoll added a comment -

      Bulk close for 3.1

      Show
      gsingers Grant Ingersoll added a comment - Bulk close for 3.1
      Hide
      thetaphi Uwe Schindler added a comment -

      We cannot declare both methods But backwards does not fail now

      Show
      thetaphi Uwe Schindler added a comment - We cannot declare both methods But backwards does not fail now
      Hide
      thetaphi Uwe Schindler added a comment -

      Java 5 allows covariant return types. Could we not declare both methods in 3.x and deprecate the old one? In trunk we can remove it and only provide List.

      Show
      thetaphi Uwe Schindler added a comment - Java 5 allows covariant return types. Could we not declare both methods in 3.x and deprecate the old one? In trunk we can remove it and only provide List.
      Hide
      shaie Shai Erera added a comment -

      Committed revision 1034080 + 1034144 (3x). Due to backwards tests failure, I kept the method signature as returning Collection, and only documented the new behavior.
      Committed revision 1034140 (trunk).

      Show
      shaie Shai Erera added a comment - Committed revision 1034080 + 1034144 (3x). Due to backwards tests failure, I kept the method signature as returning Collection, and only documented the new behavior. Committed revision 1034140 (trunk).
      Hide
      simonw Simon Willnauer added a comment -

      Shai, looks good to me!

      +1 to commit

      Show
      simonw Simon Willnauer added a comment - Shai, looks good to me! +1 to commit
      Hide
      shaie Shai Erera added a comment -

      Patch against 3x:

      • Changes listCommits() signature to return a List<IndexCommit>
      • DirReader.listCommits() sorts the list in the end.
      • Added a test case to TestIndexReader.
      • IndexCommit implements Comparable. Removed impl from CommitPoint (which also removed a redundant duplicate 'gen' member).

      I did not implement ReaderCommit to support deletes. Obtaining the lock for this purpose does not seem the right way to me ... IndexWriter has a deleteUnusedFiles which the application can use. If the app only does IR.listCommits, then being able to delete is an advantage, but otherwise it will need to mess with LockObtainFail exceptions. Not sure it's worth the efforts.

      I believe it is ready to commit. I'll wait a day or two until I commit it. Your comments are welcome.

      Show
      shaie Shai Erera added a comment - Patch against 3x: Changes listCommits() signature to return a List<IndexCommit> DirReader.listCommits() sorts the list in the end. Added a test case to TestIndexReader. IndexCommit implements Comparable. Removed impl from CommitPoint (which also removed a redundant duplicate 'gen' member). I did not implement ReaderCommit to support deletes. Obtaining the lock for this purpose does not seem the right way to me ... IndexWriter has a deleteUnusedFiles which the application can use. If the app only does IR.listCommits, then being able to delete is an advantage, but otherwise it will need to mess with LockObtainFail exceptions. Not sure it's worth the efforts. I believe it is ready to commit. I'll wait a day or two until I commit it. Your comments are welcome.

        People

        • Assignee:
          shaie Shai Erera
          Reporter:
          shaie Shai Erera
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development