Lucene - Core
  1. Lucene - Core
  2. LUCENE-6149

Infix suggesters' highlighting, allTermsRequired options are hardwired and not configurable for non-contextual lookup

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.9, 4.10.1, 4.10.2, 4.10.3
    • Fix Version/s: 5.0, 6.0
    • Component/s: modules/other
    • Labels:

      Description

      Highlighting and allTermsRequired are hardwired in AnalyzingInfixSuggester for non-contextual lookup (via Lookup) see true, true below:

      AnalyzingInfixSuggester.java (extends Lookup.java)
      public List<LookupResult> lookup(CharSequence key, Set<BytesRef> contexts, boolean onlyMorePopular, int num) throws IOException {
          return lookup(key, contexts, num, true, true);
      }
      
      /** Lookup, without any context. */
      public List<LookupResult> lookup(CharSequence key, int num, boolean allTermsRequired, boolean doHighlight) throws IOException {
          return lookup(key, null, num, allTermsRequired, doHighlight);
        }
      
      Lookup.java
      public List<LookupResult> lookup(CharSequence key, boolean onlyMorePopular, int num) throws IOException {
          return lookup(key, null, onlyMorePopular, num);
      }
      

      The above means the majority of the current infix suggester lookup always return highlighted results with allTermsRequired in effect. There is no way to change this despite the options and improvement of LUCENE-6050, made to incorporate Boolean lookup clauses (MUST/SHOULD). This shortcoming has also been reported in SOLR-6648.

      The suggesters (AnalyzingInfixSuggester, BlendedInfixSuggester) should provide a proper mechanism to set defaults for highlighting and "allTermsRequired", e.g. in constructors (and in Solr factories, thus configurable via solrconfig.xml).

      1. LUCENE-6149.patch
        10 kB
        Tomás Fernández Löbbe
      2. LUCENE-6149.patch
        10 kB
        Tomás Fernández Löbbe
      3. LUCENE-6149.patch
        9 kB
        Boon Low
      4. LUCENE-6149-v4.10.3.patch
        9 kB
        Boon Low

        Issue Links

          Activity

          Hide
          Boon Low added a comment - - edited

          Patch attached + a couple of unit tests for allTermsRequired=false

          Show
          Boon Low added a comment - - edited Patch attached + a couple of unit tests for allTermsRequired=false
          Hide
          Tomás Fernández Löbbe added a comment -

          I didn't see this patch yet, but as I said in SOLR-6648, I think it makes sense to set those defaults in the constructor.

          Show
          Tomás Fernández Löbbe added a comment - I didn't see this patch yet, but as I said in SOLR-6648 , I think it makes sense to set those defaults in the constructor.
          Hide
          Tomás Fernández Löbbe added a comment -

          Boon Low, could you update the patch to a recent version of trunk? Also, could you add javadocs to the newly added constructors?

          Show
          Tomás Fernández Löbbe added a comment - Boon Low , could you update the patch to a recent version of trunk? Also, could you add javadocs to the newly added constructors?
          Hide
          Boon Low added a comment -

          That patch was based upon and tested with the v4.10.3 release on Dec 20. But I can see that have been significant changes to AnalyzingInfixSuggester in the trunk.

          Shall update and test the patch tomorrow.

          Show
          Boon Low added a comment - That patch was based upon and tested with the v4.10.3 release on Dec 20. But I can see that have been significant changes to AnalyzingInfixSuggester in the trunk. Shall update and test the patch tomorrow.
          Hide
          Boon Low added a comment - - edited

          patch updated w.r.t. trunk 05/01/15

          Show
          Boon Low added a comment - - edited patch updated w.r.t. trunk 05/01/15
          Hide
          Tomás Fernández Löbbe added a comment -

          The patch looks good. I added some more tests to the new code and renamed the field highlighting->highlight

          Show
          Tomás Fernández Löbbe added a comment - The patch looks good. I added some more tests to the new code and renamed the field highlighting->highlight
          Hide
          Tomás Fernández Löbbe added a comment -

          Minor changes to the test and made the new fields private. I'll commit this soon

          Show
          Tomás Fernández Löbbe added a comment - Minor changes to the test and made the new fields private. I'll commit this soon
          Hide
          ASF subversion and git services added a comment -

          Commit 1649893 from Tomás Fernández Löbbe in branch 'dev/trunk'
          [ https://svn.apache.org/r1649893 ]

          LUCENE-6149: Infix suggesters' highlighting and allTermsRequired can be set at the constructor for non-contextual lookup

          Show
          ASF subversion and git services added a comment - Commit 1649893 from Tomás Fernández Löbbe in branch 'dev/trunk' [ https://svn.apache.org/r1649893 ] LUCENE-6149 : Infix suggesters' highlighting and allTermsRequired can be set at the constructor for non-contextual lookup
          Hide
          ASF subversion and git services added a comment -

          Commit 1649955 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1649955 ]

          LUCENE-6149: Infix suggesters' highlighting and allTermsRequired can be set at the constructor for non-contextual lookup

          Show
          ASF subversion and git services added a comment - Commit 1649955 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1649955 ] LUCENE-6149 : Infix suggesters' highlighting and allTermsRequired can be set at the constructor for non-contextual lookup
          Hide
          Boon Low added a comment -

          Thanks Tomás, good to see the patch making into the trunk and branch_5x. I shall find some time soon to update and post the v.4.10.3 patch to include your changes.

          Show
          Boon Low added a comment - Thanks Tomás, good to see the patch making into the trunk and branch_5x. I shall find some time soon to update and post the v.4.10.3 patch to include your changes.
          Hide
          Boon Low added a comment -

          There's a typo the test class: testConstructorDefatuls

          Show
          Boon Low added a comment - There's a typo the test class: testConstructorDefatuls
          Hide
          Boon Low added a comment -

          patch for v4.10.3 release

          Show
          Boon Low added a comment - patch for v4.10.3 release
          Hide
          ASF subversion and git services added a comment -

          Commit 1650132 from Tomás Fernández Löbbe in branch 'dev/trunk'
          [ https://svn.apache.org/r1650132 ]

          LUCENE-6149: Fixed typo

          Show
          ASF subversion and git services added a comment - Commit 1650132 from Tomás Fernández Löbbe in branch 'dev/trunk' [ https://svn.apache.org/r1650132 ] LUCENE-6149 : Fixed typo
          Hide
          ASF subversion and git services added a comment -

          Commit 1650134 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1650134 ]

          LUCENE-6149: Fixed typo

          Show
          ASF subversion and git services added a comment - Commit 1650134 from Tomás Fernández Löbbe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1650134 ] LUCENE-6149 : Fixed typo
          Hide
          Tomás Fernández Löbbe added a comment -

          Fixed. Thanks Boon!

          Show
          Tomás Fernández Löbbe added a comment - Fixed. Thanks Boon!
          Hide
          Anshum Gupta added a comment -

          Bulk close after 5.0 release.

          Show
          Anshum Gupta added a comment - Bulk close after 5.0 release.

            People

            • Assignee:
              Tomás Fernández Löbbe
              Reporter:
              Boon Low
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development