Lucene - Core
  1. Lucene - Core
  2. LUCENE-6050

Add possibility to specify SHOUD or MUST for each context for AnalyzingInfixSuggester.loockup()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.10.2
    • Fix Version/s: 5.0, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Currently as shown at
      https://github.com/apache/lucene-solr/blob/lucene_solr_4_9_0/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java#L362 , we have:

      lookup(CharSequence key, Set<BytesRef> contexts, int num, boolean allTermsRequired, boolean doHighlight)
      

      and SHOULD is being applied to all contexts.

      We need the ability to specify whether it's a SHOULD or a MUST on each individual context.

      Thanks.

      1. LUCENE-6050.patch
        15 kB
        jane chang
      2. LUCENE-6050.patch
        12 kB
        jane chang
      3. LUCENE-6050.patch
        12 kB
        jane chang
      4. LUCENE-6050.patch
        6 kB
        jane chang

        Issue Links

          Activity

          Hide
          jane chang added a comment -

          here's a patch that adds the allContextsRequired boolean and two tests

          Show
          jane chang added a comment - here's a patch that adds the allContextsRequired boolean and two tests
          Hide
          Arcadius Ahouansou added a comment -

          Hello jane chang .
          Thank you very much for the patch,
          The initial description of this issue was rather confusing, so I have updated it.

          I have checked the patch and your implementation is

          lookup(CharSequence key, Set<BytesRef> contexts, int num, boolean allTermsRequired, boolean doHighlight, boolean allContextsRequired)

          which was accidentally mentioned in the initial issue description.

          In term of flexibility, there are many ways to solve this issue and we could have something like:

          lookup(CharSequence key, Set<BytesRef> contexts, Set<BytesRef> requiredContexts, int num, boolean allTermsRequired, boolean doHighlight)

          or

          lookup(CharSequence key, Map<BytesRef, BooleanClause.Occur> contextInfo, int num, boolean allTermsRequired, boolean doHighlight)

          or

          lookup(CharSequence key, Map<BooleanClause.Occur, Set<BytesRef>> contextInfo, int num, boolean allTermsRequired, boolean doHighlight)

          or ...?
          What do you think jane chang and Michael McCandless would be the cleanest and most flexible way to do this?

          Thanks.

          Show
          Arcadius Ahouansou added a comment - Hello jane chang . Thank you very much for the patch, The initial description of this issue was rather confusing, so I have updated it. I have checked the patch and your implementation is lookup(CharSequence key, Set<BytesRef> contexts, int num, boolean allTermsRequired, boolean doHighlight, boolean allContextsRequired) which was accidentally mentioned in the initial issue description. In term of flexibility, there are many ways to solve this issue and we could have something like: lookup(CharSequence key, Set<BytesRef> contexts, Set<BytesRef> requiredContexts, int num, boolean allTermsRequired, boolean doHighlight) or lookup(CharSequence key, Map<BytesRef, BooleanClause.Occur> contextInfo, int num, boolean allTermsRequired, boolean doHighlight) or lookup(CharSequence key, Map<BooleanClause.Occur, Set<BytesRef>> contextInfo, int num, boolean allTermsRequired, boolean doHighlight) or ...? What do you think jane chang and Michael McCandless would be the cleanest and most flexible way to do this? Thanks.
          Hide
          jane chang added a comment -

          patch for

          lookup(CharSequence key, Map<Set<BytesRef, BooleanClause.Occur>> contextInfo, int num, boolean allTermsRequired, boolean doHighlight)

          and a bunch of tests

          Show
          jane chang added a comment - patch for lookup(CharSequence key, Map<Set<BytesRef, BooleanClause.Occur>> contextInfo, int num, boolean allTermsRequired, boolean doHighlight) and a bunch of tests
          Hide
          jane chang added a comment -

          minor cleanups

          Show
          jane chang added a comment - minor cleanups
          Hide
          Michael McCandless added a comment -

          Thanks jane, patch looks great.

          But I see compilation errors: probably you need to run "ant clean" and then "ant test"?

          Also, could you add the corresponding methods to BlendedInfixSuggester? They can just forward to super and multiply the topN like the current methods do ...

          Show
          Michael McCandless added a comment - Thanks jane, patch looks great. But I see compilation errors: probably you need to run "ant clean" and then "ant test"? Also, could you add the corresponding methods to BlendedInfixSuggester? They can just forward to super and multiply the topN like the current methods do ...
          Hide
          jane chang added a comment -

          fixed compilation errors

          Show
          jane chang added a comment - fixed compilation errors
          Hide
          ASF subversion and git services added a comment -

          Commit 1638441 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1638441 ]

          LUCENE-6050: allow for MUST and MUST_NOT (in addition to SHOULD) context tags in Analyzing/BlendedInfixSuggester

          Show
          ASF subversion and git services added a comment - Commit 1638441 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1638441 ] LUCENE-6050 : allow for MUST and MUST_NOT (in addition to SHOULD) context tags in Analyzing/BlendedInfixSuggester
          Hide
          ASF subversion and git services added a comment -

          Commit 1638442 from Michael McCandless in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1638442 ]

          LUCENE-6050: allow for MUST and MUST_NOT (in addition to SHOULD) context tags in Analyzing/BlendedInfixSuggester

          Show
          ASF subversion and git services added a comment - Commit 1638442 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1638442 ] LUCENE-6050 : allow for MUST and MUST_NOT (in addition to SHOULD) context tags in Analyzing/BlendedInfixSuggester
          Hide
          Michael McCandless added a comment -

          Thanks Arcadius and jane!

          Show
          Michael McCandless added a comment - Thanks Arcadius and jane!
          Hide
          Arcadius Ahouansou added a comment -

          Thank you jane chang for the quick patch.
          Thank you Michael McCandless for reviewing and committing this.

          Show
          Arcadius Ahouansou added a comment - Thank you jane chang for the quick patch. Thank you Michael McCandless for reviewing and committing this.
          Hide
          Michael McCandless added a comment -

          Thank you Arcadius Ahouansou for opening the issue in the first place: this is the hardest part

          Show
          Michael McCandless added a comment - Thank you Arcadius Ahouansou for opening the issue in the first place: this is the hardest part
          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:
              Unassigned
              Reporter:
              Arcadius Ahouansou
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development