Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.6.1, 4.4
    • Fix Version/s: 6.6, 7.0
    • Component/s: highlighter
    • Labels:
      None

      Description

      When a wildcard is present in the hl.fl field, the field is not split up at commas/spaces into components. As a consequence settings like hl.fl=_highlight,_data do not work.

      Splitting the string first and evaluating wildcards on each component afterwards would be more powerful and consistent with the documentation.

      1. highlight-wildcards.patch
        5 kB
        Daniel Debray
      2. SOLR-5127.patch
        9 kB
        Christine Poerschke
      3. SOLR-5127.patch
        7 kB
        Christine Poerschke
      4. SOLR-5127.patch
        6 kB
        Christine Poerschke

        Issue Links

          Activity

          Hide
          derbyd Daniel Debray added a comment -

          I have implemented this issue. Wildcard fields that have been supplied using multiple hl_fl params or one single param (comma or whitespace seperated list on fields) can now be resolved to the requested fields.
          I extended one unit test to cover this case.

          Could someone have a look at this patch and apply it to the 4.8 and/or 5.0 branch?

          Show
          derbyd Daniel Debray added a comment - I have implemented this issue. Wildcard fields that have been supplied using multiple hl_fl params or one single param (comma or whitespace seperated list on fields) can now be resolved to the requested fields. I extended one unit test to cover this case. Could someone have a look at this patch and apply it to the 4.8 and/or 5.0 branch?
          Hide
          simon.endele Simon Endele added a comment -

          I implemented a similar solution, which seems to work for us.

          May be interesting:
          Using the PostingsSolrHighlighter an expression like hl.fl=*_text,myfield even causes the following exception:

          java.lang.IllegalArgumentException: fieldsIn must not be empty
          at org.apache.lucene.search.postingshighlight.PostingsHighlighter.highlightFieldsAsObjects(PostingsHighlighter.java:342)
          at org.apache.lucene.search.postingshighlight.PostingsHighlighter.highlightFields(PostingsHighlighter.java:303)
          at org.apache.solr.highlight.PostingsSolrHighlighter.doHighlighting(PostingsSolrHighlighter.java:140)
          at org.apache.solr.handler.component.HighlightComponent.process(HighlightComponent.java:146)
          at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:218)
          
          Show
          simon.endele Simon Endele added a comment - I implemented a similar solution, which seems to work for us. May be interesting: Using the PostingsSolrHighlighter an expression like hl.fl=*_text,myfield even causes the following exception: java.lang.IllegalArgumentException: fieldsIn must not be empty at org.apache.lucene.search.postingshighlight.PostingsHighlighter.highlightFieldsAsObjects(PostingsHighlighter.java:342) at org.apache.lucene.search.postingshighlight.PostingsHighlighter.highlightFields(PostingsHighlighter.java:303) at org.apache.solr.highlight.PostingsSolrHighlighter.doHighlighting(PostingsSolrHighlighter.java:140) at org.apache.solr.handler.component.HighlightComponent.process(HighlightComponent.java:146) at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:218)
          Hide
          rwhaddad Ramsey Haddad added a comment -

          This problem still exists in the code.
          But, the patch is fairly old and might need a minor tweak?

          Any reason to not have this fix?

          Show
          rwhaddad Ramsey Haddad added a comment - This problem still exists in the code. But, the patch is fairly old and might need a minor tweak? Any reason to not have this fix?
          Hide
          cpoerschke Christine Poerschke added a comment -

          https://cwiki.apache.org/confluence/display/solr/Highlighting#Highlighting-Usage mentions no restrictions on how hl.fl wildcards may be used i.e. this sounds like a bug (will update issue type to match).

          Show
          cpoerschke Christine Poerschke added a comment - https://cwiki.apache.org/confluence/display/solr/Highlighting#Highlighting-Usage mentions no restrictions on how hl.fl wildcards may be used i.e. this sounds like a bug (will update issue type to match).
          Hide
          cpoerschke Christine Poerschke added a comment -

          Daniel Debray's 2014 patch could no longer be automatically applied to the current (master) code base and so instead attached patch is combination of manual patch apply and adapt.

          still to do:

          • run all the tests (precommit and the changed tests already pass)
          • check if Simon Endele's observation w.r.t. PostingsSolrHighlighter still applies (with or without this patch)
          • decide if a slight change to existing behaviour is acceptable - David Smiley any thoughts on this?
            • existing behaviour: *_text expands to (say) (en_text and de_text)
            • changed behaviour: *_text expands to either (en_text and de_text) or (de_text and en_text)
          Show
          cpoerschke Christine Poerschke added a comment - Daniel Debray 's 2014 patch could no longer be automatically applied to the current (master) code base and so instead attached patch is combination of manual patch apply and adapt. still to do: run all the tests (precommit and the changed tests already pass) check if Simon Endele 's observation w.r.t. PostingsSolrHighlighter still applies (with or without this patch) decide if a slight change to existing behaviour is acceptable - David Smiley any thoughts on this? existing behaviour: *_text expands to (say) ( en_text and de_text ) changed behaviour: *_text expands to either ( en_text and de_text ) or ( de_text and en_text )
          Hide
          dsmiley David Smiley added a comment -

          It's unclear what the difference is; I think you simply changed the order? That's insignificant, I think.

          Show
          dsmiley David Smiley added a comment - It's unclear what the difference is; I think you simply changed the order? That's insignificant, I think.
          Hide
          dsmiley David Smiley added a comment -

          You could declare expandedFields to be a LinkedHashSet and the results would be deterministic.

          Show
          dsmiley David Smiley added a comment - You could declare expandedFields to be a LinkedHashSet and the results would be deterministic.
          Hide
          cpoerschke Christine Poerschke added a comment -

          attaching updated patch:

          • done: switched from HashSet to LinkedHashSet as suggested
          • done: in TestPostingsSolrHighlighter added testMultipleFieldsViaWildcard (the new test fails without the fix and passes with the fix)
          • in-progress: running of all solr/core tests
          Show
          cpoerschke Christine Poerschke added a comment - attaching updated patch: done: switched from HashSet to LinkedHashSet as suggested done: in TestPostingsSolrHighlighter added testMultipleFieldsViaWildcard (the new test fails without the fix and passes with the fix) in-progress: running of all solr/core tests
          Hide
          dsmiley David Smiley added a comment -

          I suggest moving the testMultipleFieldsViaWildcard to TestUnifiedSolrHighlighter. PostingsHighlighter is probably only of backwards-compatibility interest. It morphed into the UH.

          Show
          dsmiley David Smiley added a comment - I suggest moving the testMultipleFieldsViaWildcard to TestUnifiedSolrHighlighter. PostingsHighlighter is probably only of backwards-compatibility interest. It morphed into the UH.
          Hide
          cpoerschke Christine Poerschke added a comment -
          • testMultipleFieldsViaWildcard (also) added in TestUnifiedSolrHighlighter
          • ant precommit passes
          • solr/core tests pass
          Show
          cpoerschke Christine Poerschke added a comment - testMultipleFieldsViaWildcard (also) added in TestUnifiedSolrHighlighter ant precommit passes solr/core tests pass
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f9ca49a8d59a89e30ce670e2eedcf6560e7ed91d in lucene-solr's branch refs/heads/master from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f9ca49a ]

          SOLR-5127: Multiple highlight fields and wildcards are now supported e.g. hl.fl=title,text_*
          (Sven-S. Porst, Daniel Debray, Simon Endele, Christine Poerschke)

          Show
          jira-bot ASF subversion and git services added a comment - Commit f9ca49a8d59a89e30ce670e2eedcf6560e7ed91d in lucene-solr's branch refs/heads/master from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f9ca49a ] SOLR-5127 : Multiple highlight fields and wildcards are now supported e.g. hl.fl=title,text_* (Sven-S. Porst, Daniel Debray, Simon Endele, Christine Poerschke)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c3ebfdc4e2dbca62a56175a5ad4e6e1f5dcea2ee in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c3ebfdc ]

          SOLR-5127: Multiple highlight fields and wildcards are now supported e.g. hl.fl=title,text_*
          (Sven-S. Porst, Daniel Debray, Simon Endele, Christine Poerschke)

          Show
          jira-bot ASF subversion and git services added a comment - Commit c3ebfdc4e2dbca62a56175a5ad4e6e1f5dcea2ee in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c3ebfdc ] SOLR-5127 : Multiple highlight fields and wildcards are now supported e.g. hl.fl=title,text_* (Sven-S. Porst, Daniel Debray, Simon Endele, Christine Poerschke)
          Hide
          cpoerschke Christine Poerschke added a comment -

          Thanks everyone!

          Show
          cpoerschke Christine Poerschke added a comment - Thanks everyone!
          Hide
          ssp Sven-S. Porst added a comment -

          Cool, thanks for taking care of this Christine Poerschke!

          Show
          ssp Sven-S. Porst added a comment - Cool, thanks for taking care of this Christine Poerschke !

            People

            • Assignee:
              cpoerschke Christine Poerschke
              Reporter:
              ssp Sven-S. Porst
            • Votes:
              5 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development