Solr
  1. Solr
  2. SOLR-2110

Distributed faceting can create invalid refinement requests when "key" is complex

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      The NPE described here: http://search.lucidimagination.com/search/document/a4c217d153635301
      is due to 2 issues. An exception in a refinement request isn't checked for, and a NPE results, masking the original exception.
      The original exception is caused by Solr using local param dereferencing with a param name derived from the "key" for that facet.
      If the key is something like "a/b/c" the request fails.

      1. SOLR-2110.patch
        1 kB
        Yonik Seeley

        Activity

        Hide
        Yonik Seeley added a comment -

        Proposed fixes:

        • check for facet/exception (or the lack of facet/facet_fields)
        • the generated param name for listing the refinement terms should be mangled/escaped to ensure correct syntax
        • OPTIONAL: param dereferencing should be able to support the full range of values (right now it's just an "id")
        Show
        Yonik Seeley added a comment - Proposed fixes: check for facet/exception (or the lack of facet/facet_fields) the generated param name for listing the refinement terms should be mangled/escaped to ensure correct syntax OPTIONAL: param dereferencing should be able to support the full range of values (right now it's just an "id")
        Hide
        Yonik Seeley added a comment -

        part 1 (SOLR-2111) is done and committed.
        I think we should be able to support more than just a normal "id" for param dereferencing (part 3) - the docs never said anything about restrictions.

        Show
        Yonik Seeley added a comment - part 1 ( SOLR-2111 ) is done and committed. I think we should be able to support more than just a normal "id" for param dereferencing (part 3) - the docs never said anything about restrictions.
        Hide
        Yonik Seeley added a comment -

        OK, here's a draft patch that removes any restriction on dereferencing in local params.
        If the first char is a $, then the value will be treated as a parameter name and dereferenced. The value is read like any other value... may be quoted, etc.

        so legal examples are
        foo=$a/b/c
        foo=$'a/b/c'

        Show
        Yonik Seeley added a comment - OK, here's a draft patch that removes any restriction on dereferencing in local params. If the first char is a $, then the value will be treated as a parameter name and dereferenced. The value is read like any other value... may be quoted, etc. so legal examples are foo=$a/b/c foo=$'a/b/c'
        Hide
        Yonik Seeley added a comment -

        I added tests and committed the patch to remove restrictions on what can be dereferenced in local params.

        Show
        Yonik Seeley added a comment - I added tests and committed the patch to remove restrictions on what can be dereferenced in local params.
        Hide
        Yonik Seeley added a comment -

        Committed escaping code for keys we internally generate (or re-generate). That should finish up this issue.

        Show
        Yonik Seeley added a comment - Committed escaping code for keys we internally generate (or re-generate). That should finish up this issue.
        Hide
        Koji Sekiguchi added a comment -

        I need this fix for branch_3x.

        Show
        Koji Sekiguchi added a comment - I need this fix for branch_3x.
        Hide
        Koji Sekiguchi added a comment -

        branch_3x: Committed revision 1003506.

        Show
        Koji Sekiguchi added a comment - branch_3x: Committed revision 1003506.
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1.0 release

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1.0 release

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development