Solr
  1. Solr
  2. SOLR-6686

facet.threads can return wrong results when using facet.prefix multiple times on same field

    Details

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

      Description

      When using facet.threads, SimpleFacets can return the wrong results when using facet.prefix multiple times on the same field.

      The problem is that SimpleFacets essentially stores the prefix value in a global variable, rather than passing the current prefix value into the Callable. So, the prefix value that is used when getting the term counts is whichever one was the last one parsed.

      STEPS TO REPRODUCE:

      1. Create a document with a string field named "myFieldName" and value "foo"
      2. Create another document with a string field named "myFieldName" and value "bar"
      3. Run this query:
        q=*:*&rows=0&facet=true&facet.field={!key=key1 facet.prefix=foo}myFieldName&facet.field={!key=key2 facet.prefix=bar}myFieldName&facet.threads=1

      EXPECTED:

      <lst name="facet_fields">
        <lst name="key1">
          <int name="foo">1</int>
        </lst>
        <lst name="key2">
          <int name="bar">1</int>
        </lst>
      </lst>

      ACTUAL:

      <lst name="facet_fields">
        <lst name="key1">
          <int name="bar">1</int>
        </lst>
        <lst name="key2">
          <int name="bar">1</int>
        </lst>
      </lst>

      I'm using 4.9, but I think this affects all versions.

      A user previously reported this here:
      http://mail-archives.apache.org/mod_mbox/lucene-solr-user/201405.mbox/%3CBAY169-W52CEF09187A88286DE5417D5270@phx.gbl%3E

      I think this affects parameters other than facet.prefix, but I have not tried that yet.

      1. SOLR-6686.patch
        73 kB
        Shalin Shekhar Mangar
      2. SOLR-6686.patch
        36 kB
        Shalin Shekhar Mangar
      3. SOLR-6686.patch
        33 kB
        Tim Underwood

        Issue Links

          Activity

          Hide
          Tim Underwood added a comment -

          I just ran into this issue on 5.0.0.

          When using facet.threads=-1 I was seeing inconsistent counts or random 400 errors saying "facet.prefix is not supported on numeric types" when refreshing the same query.

          Show
          Tim Underwood added a comment - I just ran into this issue on 5.0.0. When using facet.threads=-1 I was seeing inconsistent counts or random 400 errors saying "facet.prefix is not supported on numeric types" when refreshing the same query.
          Hide
          Tim Underwood added a comment -

          Initial attempt at fixing this. I've removed the mutable state from SimpleFacets and changed the parsedParams() method to return an instance of a new static inner class called ParsedParams. This ParsedParams instance is then passed around the the various methods.

          It's not the cleanest fix but it is working so far for me. A better fix probably involves a larger refactoring of SimpleFacets.

          Show
          Tim Underwood added a comment - Initial attempt at fixing this. I've removed the mutable state from SimpleFacets and changed the parsedParams() method to return an instance of a new static inner class called ParsedParams. This ParsedParams instance is then passed around the the various methods. It's not the cleanest fix but it is working so far for me. A better fix probably involves a larger refactoring of SimpleFacets.
          Hide
          Tim Underwood added a comment -

          Any feedback on the patch?

          Any chance of this making it into 5.2?

          Show
          Tim Underwood added a comment - Any feedback on the patch? Any chance of this making it into 5.2?
          Hide
          Shalin Shekhar Mangar added a comment -

          Hi Tim, this will go into 5.2. I was about to do something similar for SOLR-6353/SOLR-4212 so I incorporated your changes to my patch on SOLR-4212. It's almost ready and should be committed next week.

          Show
          Shalin Shekhar Mangar added a comment - Hi Tim, this will go into 5.2. I was about to do something similar for SOLR-6353 / SOLR-4212 so I incorporated your changes to my patch on SOLR-4212 . It's almost ready and should be committed next week.
          Hide
          Tim Underwood added a comment -

          Does it make sense fix this issue (which is a bug) separately from SOLR-6353 & SOLR-4212 (which as far as I can tell are new features)?

          Show
          Tim Underwood added a comment - Does it make sense fix this issue (which is a bug) separately from SOLR-6353 & SOLR-4212 (which as far as I can tell are new features)?
          Hide
          Shalin Shekhar Mangar added a comment -

          You're right Tim. I'll commit this separately. TBH, I didn't think SOLR-4212 would take so long.

          Show
          Shalin Shekhar Mangar added a comment - You're right Tim. I'll commit this separately. TBH, I didn't think SOLR-4212 would take so long.
          Hide
          Shalin Shekhar Mangar added a comment -

          Here's a patch which applies on trunk.

          Show
          Shalin Shekhar Mangar added a comment - Here's a patch which applies on trunk.
          Hide
          Shalin Shekhar Mangar added a comment -

          Added a test that fails without the fix. I'll commit shortly.

          Show
          Shalin Shekhar Mangar added a comment - Added a test that fails without the fix. I'll commit shortly.
          Hide
          Tim Underwood added a comment -

          Cool. Thank you!

          Show
          Tim Underwood added a comment - Cool. Thank you!
          Hide
          ASF subversion and git services added a comment -

          Commit 1687791 from shalin@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1687791 ]

          SOLR-6686: facet.threads can return wrong results when using facet.prefix multiple times on same field

          Show
          ASF subversion and git services added a comment - Commit 1687791 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1687791 ] SOLR-6686 : facet.threads can return wrong results when using facet.prefix multiple times on same field
          Hide
          ASF subversion and git services added a comment -

          Commit 1687811 from shalin@apache.org in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1687811 ]

          SOLR-6686: facet.threads can return wrong results when using facet.prefix multiple times on same field

          Show
          ASF subversion and git services added a comment - Commit 1687811 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1687811 ] SOLR-6686 : facet.threads can return wrong results when using facet.prefix multiple times on same field
          Hide
          Shalin Shekhar Mangar added a comment -

          Thanks Michael and Tim!

          Show
          Shalin Shekhar Mangar added a comment - Thanks Michael and Tim!
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Shalin Shekhar Mangar
              Reporter:
              Michael Ryan
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development