Solr
  1. Solr
  2. SOLR-5020

Add finish() method to DelegatingCollector

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0
    • Fix Version/s: 4.5, 5.0
    • Component/s: search
    • Labels:
      None

      Description

      This issue adds a finish() method to the DelegatingCollector class so that it can be notified when collection is complete.

      The current collect() method assumes that the delegating collector will either forward on the document or not with each call. The finish() method will allow DelegatingCollectors to have more sophisticated behavior.

      For example a Field Collapsing delegating collector could collapse the documents as the collect() method is being called. Then when the finish() method is called it could pass the collapsed documents to the delegate collectors.

      This would allow grouping to be implemented within the PostFilter framework.

      1. SOLR-5020.patch
        2 kB
        Joel Bernstein

        Issue Links

          Activity

          Hide
          Joel Bernstein added a comment -

          Patch for review.

          Show
          Joel Bernstein added a comment - Patch for review.
          Hide
          Mikhail Khludnev added a comment -

          I second this feature, however I suppose it should be done on Lucene level ie. base Collector should has such method for notification and IndexSearcher should call it.

          Show
          Mikhail Khludnev added a comment - I second this feature, however I suppose it should be done on Lucene level ie. base Collector should has such method for notification and IndexSearcher should call it.
          Hide
          Robert Muir added a comment -

          We discussed this on another issue (I think I may have opened it, not sure), the thing is that if someone calls search(Query, Filter, Collector), they know when its done, when this very method returns!

          I also tried to look at what it would take (even though it seems stupid for lucene), thinking it might make things easier somehow for people: and tried to test that all collectors were well-behaved, and its really complicated.

          So after review I think it doesnt make a lot of sense there.

          Show
          Robert Muir added a comment - We discussed this on another issue (I think I may have opened it, not sure), the thing is that if someone calls search(Query, Filter, Collector), they know when its done, when this very method returns! I also tried to look at what it would take (even though it seems stupid for lucene), thinking it might make things easier somehow for people: and tried to test that all collectors were well-behaved, and its really complicated. So after review I think it doesnt make a lot of sense there.
          Hide
          ASF subversion and git services added a comment -

          Commit 1501376 from Yonik Seeley
          [ https://svn.apache.org/r1501376 ]

          SOLR-5020: add DelegatingCollector.final()

          Show
          ASF subversion and git services added a comment - Commit 1501376 from Yonik Seeley [ https://svn.apache.org/r1501376 ] SOLR-5020 : add DelegatingCollector.final()
          Hide
          ASF subversion and git services added a comment -

          Commit 1501383 from Yonik Seeley
          [ https://svn.apache.org/r1501383 ]

          SOLR-5020: add DelegatingCollector.final()

          Show
          ASF subversion and git services added a comment - Commit 1501383 from Yonik Seeley [ https://svn.apache.org/r1501383 ] SOLR-5020 : add DelegatingCollector.final()
          Hide
          Yonik Seeley added a comment -

          Although I'm still questioning "configurable collectors", this issue certainly makes sense on it's own.

          Committed 4x & trunk.

          Show
          Yonik Seeley added a comment - Although I'm still questioning "configurable collectors", this issue certainly makes sense on it's own. Committed 4x & trunk.
          Hide
          Tomás Fernández Löbbe added a comment -

          I think the issue Robert refers to is LUCENE-4370. I had a similar requirement once I implemented a Collector.

          Show
          Tomás Fernández Löbbe added a comment - I think the issue Robert refers to is LUCENE-4370 . I had a similar requirement once I implemented a Collector.
          Hide
          Simon Endele added a comment -

          It looks like this isn't working in combination with grouping. Is that possible?

          I applied the attached patch to my Solr 4.4.0 workspace containing an AclQParserPlugin as described here:
          http://searchhub.org/2012/02/22/custom-security-filtering-in-solr/

          It works without grouping, but if grouping is activated, the collect() method is still called, but finish() is not.

          Show
          Simon Endele added a comment - It looks like this isn't working in combination with grouping. Is that possible? I applied the attached patch to my Solr 4.4.0 workspace containing an AclQParserPlugin as described here: http://searchhub.org/2012/02/22/custom-security-filtering-in-solr/ It works without grouping, but if grouping is activated, the collect() method is still called, but finish() is not.
          Hide
          Joel Bernstein added a comment -

          Good catch. I'll open a new ticket for this and get a patch up this afternoon.

          Show
          Joel Bernstein added a comment - Good catch. I'll open a new ticket for this and get a patch up this afternoon.
          Hide
          Joel Bernstein added a comment -

          Simon,

          I uploaded a patch to SOLR-5230. I'll test it tomorrow. In the meantime if you have a chance to test it let me know how it goes.

          Show
          Joel Bernstein added a comment - Simon, I uploaded a patch to SOLR-5230 . I'll test it tomorrow. In the meantime if you have a chance to test it let me know how it goes.
          Hide
          Adrien Grand added a comment -

          4.5 release -> bulk close

          Show
          Adrien Grand added a comment - 4.5 release -> bulk close

            People

            • Assignee:
              Unassigned
              Reporter:
              Joel Bernstein
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development