Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 7.2
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      At the moment, Scorer.freq() does different things depending on the Scorer implementation:

      • TermScorer and the phrase scorers return the frequency of that term or phrase in the current document. TermScorer.freq() is not actually called anywhere (apart from in a couple of tests), and XPhraseScorer.freq() is only called in PhraseWeight.explain()
      • The various Boolean scorers return the number of matching subscorers, and are used for coord calculations.

      I think this is confusing. I propose that we instead add a new coord() method to Scorer that by default returns 1, and that is overridden by the boolean scorers; and that we just remove freq() entirely. PhraseWeight.explain() can call a package-private method on XPhraseScorer.

      1. LUCENE-6278.patch
        49 kB
        Alan Woodward
      2. LUCENE-6278.patch
        57 kB
        Alan Woodward
      3. LUCENE-6278.patch
        56 kB
        Alan Woodward

        Activity

        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 183571c085201a2f1e126852647910d1930d50a6 in lucene-solr's branch refs/heads/master from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=183571c ]

        LUCENE-6278: Remove Scorer.freq()

        Show
        jira-bot ASF subversion and git services added a comment - Commit 183571c085201a2f1e126852647910d1930d50a6 in lucene-solr's branch refs/heads/master from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=183571c ] LUCENE-6278 : Remove Scorer.freq()
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 2fd70bfdc9591e6ea348181cbb094b4c59fa49e3 in lucene-solr's branch refs/heads/branch_7x from Alan Woodward
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2fd70bf ]

        LUCENE-6278: Remove Scorer.freq()

        Show
        jira-bot ASF subversion and git services added a comment - Commit 2fd70bfdc9591e6ea348181cbb094b4c59fa49e3 in lucene-solr's branch refs/heads/branch_7x from Alan Woodward [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2fd70bf ] LUCENE-6278 : Remove Scorer.freq()
        Hide
        dsmiley David Smiley added a comment -

        It's nice to see the patch removes lots of little implementations all over the place, thus simplifying a lot. I was going to suggest a ScorerWithFreq subclass but in the end it wouldn't solve any of the unfortunate needs to cast, so nevermind.

        Show
        dsmiley David Smiley added a comment - It's nice to see the patch removes lots of little implementations all over the place, thus simplifying a lot. I was going to suggest a ScorerWithFreq subclass but in the end it wouldn't solve any of the unfortunate needs to cast, so nevermind.
        Hide
        romseygeek Alan Woodward added a comment -

        Boolean coord() is long gone now, so Scorer.freq() is now used only in explanations. Here's a patch removing it entirely from the Scorer API, and just keeping it as a package-private method on those scorers that use it in explain().

        Show
        romseygeek Alan Woodward added a comment - Boolean coord() is long gone now, so Scorer.freq() is now used only in explanations. Here's a patch removing it entirely from the Scorer API, and just keeping it as a package-private method on those scorers that use it in explain().
        Hide
        romseygeek Alan Woodward added a comment -

        Here's another try. MinShouldMatchScorer, ReqOptSumScorer and DisjunctionScorer all extend package-private CoordScorer, with the #coord() method.

        Show
        romseygeek Alan Woodward added a comment - Here's another try. MinShouldMatchScorer, ReqOptSumScorer and DisjunctionScorer all extend package-private CoordScorer, with the #coord() method.
        Hide
        rcmuir Robert Muir added a comment -

        The special cases in BooleanTopLevelScorers just have to deal with MinShouldMatchSumScorer and DisjunctionScorer. Unfortunately these don't share a same base class, but they could, that exposes a package-private freq()

        Show
        rcmuir Robert Muir added a comment - The special cases in BooleanTopLevelScorers just have to deal with MinShouldMatchSumScorer and DisjunctionScorer. Unfortunately these don't share a same base class, but they could, that exposes a package-private freq()
        Hide
        rcmuir Robert Muir added a comment -

        Sorry, I think we should not add this coord() method. This is like a 1970s horror only for the default similarity and it should not be in all scorer apis, just booleanscorer. it can handle these things another way...

        This would be good, since we could just remove freq().

        Show
        rcmuir Robert Muir added a comment - Sorry, I think we should not add this coord() method. This is like a 1970s horror only for the default similarity and it should not be in all scorer apis, just booleanscorer. it can handle these things another way... This would be good, since we could just remove freq().
        Hide
        romseygeek Alan Woodward added a comment -

        Patch.

        If we wanted to support back-compat we could add freq() back in as a deprecated method that delegates to either coord() (for the boolean scorers) or the preserved freq() methods on TermScorer and *PhraseScorer.

        Show
        romseygeek Alan Woodward added a comment - Patch. If we wanted to support back-compat we could add freq() back in as a deprecated method that delegates to either coord() (for the boolean scorers) or the preserved freq() methods on TermScorer and *PhraseScorer.

          People

          • Assignee:
            romseygeek Alan Woodward
            Reporter:
            romseygeek Alan Woodward
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development