Solr
  1. Solr
  2. SOLR-2959

edismax uf param does not work with magic fields '_query_' and '_val_'

    Details

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

      Description

      The edismax query parser should recognize the magic fields '_query_' and '_val_' as field names that can be allowed/restricted using "uf" just like any other field name.

      1. SOLR-2959.patch
        7 kB
        Hoss Man
      2. SOLR-2959.patch
        3 kB
        Hoss Man
      3. SOLR-2959.patch
        3 kB
        Michael Watts

        Issue Links

          Activity

          Hide
          Michael Watts added a comment -

          second upload of patch, this time 'granted inclusion'

          Show
          Michael Watts added a comment - second upload of patch, this time 'granted inclusion'
          Hide
          Hoss Man added a comment -

          Michael: thanks for your patch.

          the situation has changed a bit since you opend this issue, particularly with regards to SOLR-3026 and the new support for specifying exactly which fields can/can't be used in the query string.

          it appears that at the moment, (trunk) edismax supports val and query just fine – and in fact the bug is sort of the reverse of before: you can't use the uf param to prevent them from working.

          I've expanded on your test changes to demonstrate this in the update attachment. will tweak issue summary shortly

          Show
          Hoss Man added a comment - Michael: thanks for your patch. the situation has changed a bit since you opend this issue, particularly with regards to SOLR-3026 and the new support for specifying exactly which fields can/can't be used in the query string. it appears that at the moment, (trunk) edismax supports val and query just fine – and in fact the bug is sort of the reverse of before: you can't use the uf param to prevent them from working. I've expanded on your test changes to demonstrate this in the update attachment. will tweak issue summary shortly
          Hide
          Hoss Man added a comment -

          whoops ... my mistake, edismax is not working with those magic fields, the test was just mistakenly passing because of SOLR-3261 ... i think the basic approach from Michael's original patch will can be massaged to make it work properly (with uf) and i'll try update it to do that later today/tomorrow (with a working test)

          Show
          Hoss Man added a comment - whoops ... my mistake, edismax is not working with those magic fields, the test was just mistakenly passing because of SOLR-3261 ... i think the basic approach from Michael's original patch will can be massaged to make it work properly (with uf) and i'll try update it to do that later today/tomorrow (with a working test)
          Hide
          Hoss Man added a comment -

          updated patch: fixes tests, and fixes edismax to support the magic fields.

          this patch also refactors the magic field handling in SolrQueryParser a big so the magic fields are defined by an enum to future proof us against inconsistencies if more magic fields are added down the road.

          Show
          Hoss Man added a comment - updated patch: fixes tests, and fixes edismax to support the magic fields. this patch also refactors the magic field handling in SolrQueryParser a big so the magic fields are defined by an enum to future proof us against inconsistencies if more magic fields are added down the road.
          Hide
          Hoss Man added a comment -

          feedback appreciated, but i'll assume lazy concensus and commit tomorrow if i don't get any.

          Show
          Hoss Man added a comment - feedback appreciated, but i'll assume lazy concensus and commit tomorrow if i don't get any.
          Hide
          Yonik Seeley added a comment -

          +1, looks good.

          Show
          Yonik Seeley added a comment - +1, looks good.
          Hide
          Hoss Man added a comment -

          r1303265 - 3x
          r1303256 - trunk

          Thanks for your patch Michael! .. even with all of the other changes to edismax it helped me track down exactly what needed to be fixed.

          Show
          Hoss Man added a comment - r1303265 - 3x r1303256 - trunk Thanks for your patch Michael! .. even with all of the other changes to edismax it helped me track down exactly what needed to be fixed.
          Hide
          Jan Høydahl added a comment -

          +1 Not tested but patch looks good

          Show
          Jan Høydahl added a comment - +1 Not tested but patch looks good

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development