Details

    • Type: New Feature New Feature
    • 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, Patch Available

      Description

      ChainingCollector allows chaining a bunch of Collectors, w/o them needing to know or care about each other, and be passed into Lucene's search API, since it is a Collector on its own. It is a convenient, yet useful, class. Will post a patch w/ it shortly.

      1. LUCENE-2636.patch
        13 kB
        Shai Erera
      2. LUCENE-2636.patch
        8 kB
        Shai Erera

        Activity

        Hide
        Shai Erera added a comment -

        Patch w/ ChainingCollector, a matching test and CHANGES entry.

        Show
        Shai Erera added a comment - Patch w/ ChainingCollector, a matching test and CHANGES entry.
        Hide
        Shai Erera added a comment -

        Committed revision 995373 (3x).
        Committed revision 995375 (trunk).

        Show
        Shai Erera added a comment - Committed revision 995373 (3x). Committed revision 995375 (trunk).
        Hide
        Shai Erera added a comment -

        I've received some offline comments about the name - MultiCollector (the original name in our code) seems to be more aligned w/ other Lucene classes, like MultiReader/Searcher. I personally think ChainingCollector is fine as well (and describes better what this class is about), however for consistency I think I'll rename it to MultiCollector. Any objections?

        Show
        Shai Erera added a comment - I've received some offline comments about the name - MultiCollector (the original name in our code) seems to be more aligned w/ other Lucene classes, like MultiReader/Searcher. I personally think ChainingCollector is fine as well (and describes better what this class is about), however for consistency I think I'll rename it to MultiCollector. Any objections?
        Hide
        Yonik Seeley added a comment -

        Heh - Solr added a MultiCollector a few weeks ago (and it does exactly the same thing).
        As far as naming, I agree this isn't a chaining collector, but a branching collector (and MultiCollector is a better name).

        Solr's version also has this, which simplifies building a list of collectors and then creating a single collector:

          public static Collector wrap(List<? extends Collector> collectors) {
            return collectors.size() == 1 ? collectors.get(0) : new MultiCollector(collectors);  
          }
        

        We should consolidate these two versions of course.

        Show
        Yonik Seeley added a comment - Heh - Solr added a MultiCollector a few weeks ago (and it does exactly the same thing). As far as naming, I agree this isn't a chaining collector, but a branching collector (and MultiCollector is a better name). Solr's version also has this, which simplifies building a list of collectors and then creating a single collector: public static Collector wrap(List<? extends Collector> collectors) { return collectors.size() == 1 ? collectors.get(0) : new MultiCollector(collectors); } We should consolidate these two versions of course.
        Hide
        Shai Erera added a comment -

        Heh - Solr added a MultiCollector a few weeks ago

        Nice .

        ... MultiCollector is a better name

        I will rename.

        Solr's version also has this

        That makes sense. If you eventually run w/ a single Collector, no need to have MultiCollector in the middle. I'll change MC then to have a private ctor and a static create method. I would like though for the method to receive a Collector... rather than List, because I think it simplifies matters on the application's end. And create will filter out all the null ones ...

        I'll post a patch soon.

        Show
        Shai Erera added a comment - Heh - Solr added a MultiCollector a few weeks ago Nice . ... MultiCollector is a better name I will rename. Solr's version also has this That makes sense. If you eventually run w/ a single Collector, no need to have MultiCollector in the middle. I'll change MC then to have a private ctor and a static create method. I would like though for the method to receive a Collector... rather than List, because I think it simplifies matters on the application's end. And create will filter out all the null ones ... I'll post a patch soon.
        Hide
        Shai Erera added a comment -

        Might need 1-2 iterations, so reopening.

        Show
        Shai Erera added a comment - Might need 1-2 iterations, so reopening.
        Hide
        Shai Erera added a comment -

        Name changes + static wrap.

        Show
        Shai Erera added a comment - Name changes + static wrap.
        Hide
        Yonik Seeley added a comment -

        +1 looks good!

        Show
        Yonik Seeley added a comment - +1 looks good!
        Hide
        Shai Erera added a comment -

        Committed revision 996058 (3x).
        Committed revision 996060 (trunk).

        Show
        Shai Erera added a comment - Committed revision 996058 (3x). Committed revision 996060 (trunk).
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development