Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.5, 6.0
    • Component/s: modules/facet
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      Aggregator was replaced by FacetsAggregator. FacetRequest has createAggregator() which by default throws an UOE. It was left there until we migrate the aggregators to FacetsAggregator – now all of our requests support FacetsAggregator.

      Aggregator is used only by StandardFacetsAccumulator, which too needs to vanish, at some point. But it currently it's the only one which handles sampling, complements aggregation and partitions.

      What I'd like to do is remove FacetRequest.createAggregator and in StandardFacetsAccumulator support only CountFacetRequest and SumScoreFacetRequest, which are the only ones that make sense for sampling and partitions. SumScore does not even support complements (which only work for counting).

      I'll also rename StandardFA to OldStandardFA. The plan is to eventually implement a SamplingAccumulator, PartitionsAccumulator/Aggregator and ComplementsAggregator, removing that class entirely. Until then ...

        Activity

        Hide
        jpountz Adrien Grand added a comment -

        4.5 release -> bulk close

        Show
        jpountz Adrien Grand added a comment - 4.5 release -> bulk close
        Hide
        shaie Shai Erera added a comment -

        Thanks Hoss and Rob. Sorry for letting this nocommit slip through. I removed the TODO as the intention was to remove that piece of code.

        Show
        shaie Shai Erera added a comment - Thanks Hoss and Rob. Sorry for letting this nocommit slip through. I removed the TODO as the intention was to remove that piece of code.
        Hide
        rcmuir Robert Muir added a comment -

        Thanks Hoss, I almost forgot!

        I changed the nocommit to a TODO temporarily just to unbreak jenkins.

        Show
        rcmuir Robert Muir added a comment - Thanks Hoss, I almost forgot! I changed the nocommit to a TODO temporarily just to unbreak jenkins.
        Hide
        hossman Hoss Man added a comment -

        Shai: your commited changes for this issue included a "nocommit" comment. rmuir changed it to a TODO in these commits...

        http://svn.apache.org/r1508137
        http://svn.apache.org/r1508139

        ...if this is an appropriate change and your goal was to address this on a more long term basis, then just re-resolve, but i wanted t omake sure it was on your radar in case this is a genuine "this code should not have been committed as is" situation.

        Show
        hossman Hoss Man added a comment - Shai: your commited changes for this issue included a "nocommit" comment. rmuir changed it to a TODO in these commits... http://svn.apache.org/r1508137 http://svn.apache.org/r1508139 ...if this is an appropriate change and your goal was to address this on a more long term basis, then just re-resolve, but i wanted t omake sure it was on your radar in case this is a genuine "this code should not have been committed as is" situation.
        Hide
        shaie Shai Erera added a comment -

        Committed to trunk and 4x.

        Show
        shaie Shai Erera added a comment - Committed to trunk and 4x.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1508087 from Shai Erera in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1508087 ]

        LUCENE-5144: remove FacetRequest.createAggregator, rename StandardFacetsAccumulator to OldFA and move it and associated classes under o.a.l.facet.old

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1508087 from Shai Erera in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1508087 ] LUCENE-5144 : remove FacetRequest.createAggregator, rename StandardFacetsAccumulator to OldFA and move it and associated classes under o.a.l.facet.old
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1508085 from Shai Erera in branch 'dev/trunk'
        [ https://svn.apache.org/r1508085 ]

        LUCENE-5144: remove FacetRequest.createAggregator, rename StandardFacetsAccumulator to OldFA and move it and associated classes under o.a.l.facet.old

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1508085 from Shai Erera in branch 'dev/trunk' [ https://svn.apache.org/r1508085 ] LUCENE-5144 : remove FacetRequest.createAggregator, rename StandardFacetsAccumulator to OldFA and move it and associated classes under o.a.l.facet.old
        Hide
        shaie Shai Erera added a comment -

        Tests pass, if there are no objections, I intend to commit this shortly.

        Show
        shaie Shai Erera added a comment - Tests pass, if there are no objections, I intend to commit this shortly.
        Hide
        shaie Shai Erera added a comment -

        Patch removes FacetRequest.createAggregator (NOTE: not createFacetsAggregator) and replaces it by StandardFacetsAccumulator.createAggregator(FacetRequest).

        I also renamed SFA to OldFacetsAccumulator and moved it and all associated classes under o.a.l.facet.old, in the intention of removing them one day.

        Show
        shaie Shai Erera added a comment - Patch removes FacetRequest.createAggregator (NOTE: not createFacetsAggregator) and replaces it by StandardFacetsAccumulator.createAggregator(FacetRequest). I also renamed SFA to OldFacetsAccumulator and moved it and all associated classes under o.a.l.facet.old, in the intention of removing them one day.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development