Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9687

Values not assigned to all valid Interval Facet intervals in some cases

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.3.1
    • Fix Version/s: 6.3, 7.0
    • Component/s: faceting
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      Using the interval facet definitions:

      • {!key=Positive}(0,*]
      • {!key=Zero}[0,0]
      • {!key=Negative}(*,0)

      A document with the value "0" in the numeric field the intervals are being applied to is not counted in the Zero interval. If I change the order of the definitions to , Negative, Zero, Positive the "0" value is correctly counted in the Zero interval.

      Tracing into the 5.3.1 code the problem is in the org.apache.solr.request.IntervalFacets class. When the getSortedIntervals() method sorts the interval definitions for a field by their starting value is doesn't take into account the startOpen property. When two intervals have equal start values it needs to sort intervals where startOpen == false before intervals where startOpen == true.

      In the accumIntervalWithValue() method it checks which intervals each document value should be considered a match for. It iterates through the sorted intervals and stops checking subsequent intervals when LOWER_THAN_START result is returned. If the Positive interval is sorted before the Zero interval it never checks a zero value against the Zero interval.

      I compared the 5.3.1 version of the IntervalFacets class against the 6.2.1 code, and it looks like the same issue will occur in 6.2.1.

      1. SOLR-9687.patch
        2 kB
        Tomás Fernández Löbbe

        Activity

        Hide
        andycsolr Andy Chillrud added a comment -

        Couldn't figure out how to create a patch file, but I was able to resolve the issue in Solr 5.3.1 by modifying the getSortedIntervals() method. Replaced the last line of the method

                 return o1.start.compareTo(o2.start);
        

        with

                int startComparison = o1.start.compareTo(o2.start);
                if (startComparison == 0) {
                  if (o1.startOpen != o2.startOpen) {
                    if (!o1.startOpen) {
                      return -1;
                    }
                    else {
                      return 1;
                    }
                  }
                }
                return startComparison;
        
        Show
        andycsolr Andy Chillrud added a comment - Couldn't figure out how to create a patch file, but I was able to resolve the issue in Solr 5.3.1 by modifying the getSortedIntervals() method. Replaced the last line of the method return o1.start.compareTo(o2.start); with int startComparison = o1.start.compareTo(o2.start); if (startComparison == 0) { if (o1.startOpen != o2.startOpen) { if (!o1.startOpen) { return -1; } else { return 1; } } } return startComparison;
        Hide
        tomasflobbe Tomás Fernández Löbbe added a comment -

        Thanks Andy Chillrud, I'll take a look later today. If you can, this is how you can generate a patch: https://wiki.apache.org/solr/HowToContribute#Contributing_Code_.28Features.2C_Bug_Fixes.2C_Tests.2C_etc....29 You can also do a pull request if you are familiar with github, https://wiki.apache.org/solr/HowToContribute#Working_with_GitHub

        Show
        tomasflobbe Tomás Fernández Löbbe added a comment - Thanks Andy Chillrud , I'll take a look later today. If you can, this is how you can generate a patch: https://wiki.apache.org/solr/HowToContribute#Contributing_Code_.28Features.2C_Bug_Fixes.2C_Tests.2C_etc....29 You can also do a pull request if you are familiar with github, https://wiki.apache.org/solr/HowToContribute#Working_with_GitHub
        Hide
        tomasflobbe Tomás Fernández Löbbe added a comment -

        Here is a patch with the proposed fix and tests

        Show
        tomasflobbe Tomás Fernández Löbbe added a comment - Here is a patch with the proposed fix and tests
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 96e847a10c532663e39ad2de184ed8582e5eb0e2 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=96e847a ]

        SOLR-9687: Fixed Interval Facet count issue in cases of open/close intervals on the same values

        Show
        jira-bot ASF subversion and git services added a comment - Commit 96e847a10c532663e39ad2de184ed8582e5eb0e2 in lucene-solr's branch refs/heads/branch_6x from Tomas Fernandez Lobbe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=96e847a ] SOLR-9687 : Fixed Interval Facet count issue in cases of open/close intervals on the same values
        Hide
        tomasflobbe Tomás Fernández Löbbe added a comment - - edited

        This is the commit to master: https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ce57e8a8f4274db9ad1a78f06d37a7c9e02b3fb8
        I forgot to include the Jira code in the commit comment.

        Show
        tomasflobbe Tomás Fernández Löbbe added a comment - - edited This is the commit to master: https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ce57e8a8f4274db9ad1a78f06d37a7c9e02b3fb8 I forgot to include the Jira code in the commit comment.
        Hide
        tomasflobbe Tomás Fernández Löbbe added a comment -

        Resolving. Thanks Andy! The patch should apply cleanly to 5.3 if you need that

        Show
        tomasflobbe Tomás Fernández Löbbe added a comment - Resolving. Thanks Andy! The patch should apply cleanly to 5.3 if you need that
        Hide
        andycsolr Andy Chillrud added a comment -

        Thanks Tomás. You guys are quick!

        Show
        andycsolr Andy Chillrud added a comment - Thanks Tomás. You guys are quick!
        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        Closing after 6.3.0 release.

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

          People

          • Assignee:
            tomasflobbe Tomás Fernández Löbbe
            Reporter:
            andycsolr Andy Chillrud
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development