Lucene - Core
  1. Lucene - Core
  2. LUCENE-4924

Make DocIdSetIterator.docID() return -1 when not positioned

    Details

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

      Description

      Today DocIdSetIterator.docID() can either return -1 or NO_MORE_DOCS when the enum is not positioned. I would like to only allow it to return -1 so that we can have better assertions.

      (This proposal is for trunk only.)

      1. LUCENE-4924.patch
        19 kB
        Adrien Grand
      2. LUCENE-4924.patch
        18 kB
        Robert Muir
      3. LUCENE-4924.patch
        9 kB
        Adrien Grand

        Activity

        Hide
        Yonik Seeley added a comment -

        Undefined can sometimes lead to more efficient implementations in certain circumstances. IIRC this was actually true of some iterators at some point in the past, but I don't know if it's still the case.

        Show
        Yonik Seeley added a comment - Undefined can sometimes lead to more efficient implementations in certain circumstances. IIRC this was actually true of some iterators at some point in the past, but I don't know if it's still the case.
        Hide
        Robert Muir added a comment -

        undefined is no option at all: things rely upon this behavior (e.g. disjunctionsumscorer, i was just looking at that one last night).

        Show
        Robert Muir added a comment - undefined is no option at all: things rely upon this behavior (e.g. disjunctionsumscorer, i was just looking at that one last night).
        Hide
        Michael McCandless added a comment -

        I would like to only allow it to return -1 so that we can have better assertions.

        +1

        Show
        Michael McCandless added a comment - I would like to only allow it to return -1 so that we can have better assertions. +1
        Hide
        Adrien Grand added a comment -

        Patch.

        Show
        Adrien Grand added a comment - Patch.
        Hide
        Robert Muir added a comment -

        Adrien, i did a quick pass thru the tests/test-framework and tried to fixup places that were still lenient about this here. I might have missed some, and i didnt exhaustively test yet.

        Show
        Robert Muir added a comment - Adrien, i did a quick pass thru the tests/test-framework and tried to fixup places that were still lenient about this here. I might have missed some, and i didnt exhaustively test yet.
        Hide
        Adrien Grand added a comment -

        Thanks Robert, I ran lucene tests and they all passed. I updated the patch to make the CHANGES entry clearer.

        Show
        Adrien Grand added a comment - Thanks Robert, I ran lucene tests and they all passed. I updated the patch to make the CHANGES entry clearer.
        Hide
        Adrien Grand added a comment -

        I plan to commit soon and backport everything to 4.x but the changes entry and the DocIdSetIterator.docID() javadoc change.

        Show
        Adrien Grand added a comment - I plan to commit soon and backport everything to 4.x but the changes entry and the DocIdSetIterator.docID() javadoc change.
        Hide
        Yonik Seeley added a comment -

        +1, looks good!

        Show
        Yonik Seeley added a comment - +1, looks good!
        Hide
        Adrien Grand added a comment -

        Thank you Robert and Yonik!

        Show
        Adrien Grand added a comment - Thank you Robert and Yonik!
        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:
            Adrien Grand
            Reporter:
            Adrien Grand
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development