Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major 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
        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
        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.
        Hide
        Shai Erera added a comment -

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

        Show
        Shai Erera added a comment - Tests pass, if there are no objections, I intend to commit this shortly.
        Hide
        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
        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
        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
        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
        Shai Erera added a comment -

        Committed to trunk and 4x.

        Show
        Shai Erera added a comment - Committed to trunk and 4x.
        Hide
        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
        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
        Robert Muir added a comment -

        Thanks Hoss, I almost forgot!

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

        Show
        Robert Muir added a comment - Thanks Hoss, I almost forgot! I changed the nocommit to a TODO temporarily just to unbreak jenkins.
        Hide
        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
        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
        Adrien Grand added a comment -

        4.5 release -> bulk close

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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development