Lucene - Core
  1. Lucene - Core
  2. LUCENE-2785

TopFieldCollector throws AIOOBE if numHits is 0

    Details

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

      Description

      See solr-user thread "ArrayIndexOutOfBoundsException for query with rows=0 and sort param".

      I think we should just create a null collector (only tallies up totalHits) if numHits is 0?

      1. LUCENE-2785.patch
        7 kB
        Michael McCandless

        Activity

        Hide
        Uwe Schindler added a comment -

        +1, I needed the same. Its often a use case to simply get number of results. And a PQ with one entry and all the logic does not make sense here. We should just count hits.

        Show
        Uwe Schindler added a comment - +1, I needed the same. Its often a use case to simply get number of results. And a PQ with one entry and all the logic does not make sense here. We should just count hits.
        Hide
        Shai Erera added a comment -

        It would be good if we could have a TotalHitsCollector which just counts total hits (as a standalone Collector). Problem is, we need to return a TDC from the static create() of TFC and TSDC, and a TotalHitsCollector shouldn't have topDocs() ...

        Show
        Shai Erera added a comment - It would be good if we could have a TotalHitsCollector which just counts total hits (as a standalone Collector). Problem is, we need to return a TDC from the static create() of TFC and TSDC, and a TotalHitsCollector shouldn't have topDocs() ...
        Hide
        Michael McCandless added a comment -

        So... IndexSearcher.search (the one that sorts by score) throws an EXC if numHits is 0...

        Maybe... we should do the same for the sort-by-field case?

        And, also make this new Collector that only counts hits? The exception message can then point users to this new Collector?

        I'm all for not making the app handle corner cases... but, then, I don't like that you silently get bad performance. It's better to steer the apps to use the right Collector for the job?

        Show
        Michael McCandless added a comment - So... IndexSearcher.search (the one that sorts by score) throws an EXC if numHits is 0... Maybe... we should do the same for the sort-by-field case? And, also make this new Collector that only counts hits? The exception message can then point users to this new Collector? I'm all for not making the app handle corner cases... but, then, I don't like that you silently get bad performance. It's better to steer the apps to use the right Collector for the job?
        Hide
        Uwe Schindler added a comment -

        So... IndexSearcher.search (the one that sorts by score) throws an EXC if numHits is 0... Maybe... we should do the same for the sort-by-field case?

        I think IndexSearcher should not only throw the exception, in my opinion the create() method in TSDC and TFDC should also check this and throw the exception? A lot of people, also solr are instantiating the collectors themselves (although not recommeneded, because they dont know the correct booleans passed in).

        +1 for TSDC and TFDC throw exception if hitcount <1

        And, also make this new Collector that only counts hits? The exception message can then point users to this new Collector?

        +1 to the new collector that simply counts. I have such a collector also sometimes in my code. Misusing TFDC or TSDC is wrong and uses too much resources.

        Show
        Uwe Schindler added a comment - So... IndexSearcher.search (the one that sorts by score) throws an EXC if numHits is 0... Maybe... we should do the same for the sort-by-field case? I think IndexSearcher should not only throw the exception, in my opinion the create() method in TSDC and TFDC should also check this and throw the exception? A lot of people, also solr are instantiating the collectors themselves (although not recommeneded, because they dont know the correct booleans passed in). +1 for TSDC and TFDC throw exception if hitcount <1 And, also make this new Collector that only counts hits? The exception message can then point users to this new Collector? +1 to the new collector that simply counts. I have such a collector also sometimes in my code. Misusing TFDC or TSDC is wrong and uses too much resources.
        Hide
        Earwin Burrfoot added a comment -

        "A Collector, that counts" - priceless.
        And, yes, I have one of these too.

        Show
        Earwin Burrfoot added a comment - "A Collector, that counts" - priceless. And, yes, I have one of these too.
        Hide
        Michael McCandless added a comment -

        Patch.

        I moved the numHits > 0 check down into TFC.create an TSDC.create, out of IndexSearcher.

        And I added a new TotalHitCountCollector.

        Show
        Michael McCandless added a comment - Patch. I moved the numHits > 0 check down into TFC.create an TSDC.create, out of IndexSearcher. And I added a new TotalHitCountCollector.
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1

          People

          • Assignee:
            Unassigned
            Reporter:
            Michael McCandless
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development