Solr
  1. Solr
  2. SOLR-3956

group.facet and facet.limit=-1 returns no facet counts

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.3, 6.0
    • Component/s: search
    • Labels:
      None

      Description

      Attempting to use group.facet=true and facet.limit=-1 to return all facets from a grouped result ends up with the counts not being returned. Adjusting the facet.limit to any number greater than 0 returns the facet counts as expected.

      This does not appear limited to a specific field type, as I have tried on (both multivalued and not) text, string, boolean, and double types.

      1. SOLR-3956.patch
        12 kB
        Hoss Man
      2. SOLR-3956.patch
        10 kB
        Hoss Man
      3. SOLR-3956.patch
        8 kB
        Hoss Man
      4. SOLR-3956.patch
        6 kB
        Hoss Man
      5. SOLR-3956.patch
        1 kB
        Chris van der Merwe

        Activity

        Hide
        Chris van der Merwe added a comment -

        Proposed patch attached.

        Show
        Chris van der Merwe added a comment - Proposed patch attached.
        Hide
        Hoss Man added a comment -

        Updated patch to include some test cases

        Show
        Hoss Man added a comment - Updated patch to include some test cases
        Hide
        Hoss Man added a comment -

        Patch updated to include testing directly from the grouping module as well.

        Show
        Hoss Man added a comment - Patch updated to include testing directly from the grouping module as well.
        Hide
        Hoss Man added a comment -

        Hmmm... looking at the context of the fix a bit more i realized it wasn't going to play nice with using a non-0 offset, so in this updated patch i added tests for that case and fixed the code.

        The lucene level test passes, but the solr ones don't suggesting that there is another aspect of this bug somewhere (either that i managed to screw up the solr tests) ... need to look closer later

        Show
        Hoss Man added a comment - Hmmm... looking at the context of the fix a bit more i realized it wasn't going to play nice with using a non-0 offset, so in this updated patch i added tests for that case and fixed the code. The lucene level test passes, but the solr ones don't suggesting that there is another aspect of this bug somewhere (either that i managed to screw up the solr tests) ... need to look closer later
        Hide
        Hoss Man added a comment -

        After tracing down the problem with using facet.offset combined with facet.limit<0 i started to rethink the wisdom of changing getFacetEntries to treat negative numbers as "unlimited" it seemed more prudent to revise the patch so that the the calling code in Solr passes Integer.MAX_VALUE when facet.limit < 0, and update getFacetEntries to not have int overflow if Integer.MAX_VALUE < offset+limit. (fixing other cases extreme cases beyond just facet.limit < 0)

        tests now all pass, but i'm running some more iterations in case i've tripped something in other random tests.

        Show
        Hoss Man added a comment - After tracing down the problem with using facet.offset combined with facet.limit<0 i started to rethink the wisdom of changing getFacetEntries to treat negative numbers as "unlimited" it seemed more prudent to revise the patch so that the the calling code in Solr passes Integer.MAX_VALUE when facet.limit < 0, and update getFacetEntries to not have int overflow if Integer.MAX_VALUE < offset+limit. (fixing other cases extreme cases beyond just facet.limit < 0) tests now all pass, but i'm running some more iterations in case i've tripped something in other random tests.
        Hide
        Hoss Man added a comment -

        Thanks for tracking down the initial problem Chris!

        commits: r1462227 & r1462255

        Show
        Hoss Man added a comment - Thanks for tracking down the initial problem Chris! commits: r1462227 & r1462255
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Hoss Man
            Reporter:
            Mike Spencer
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development