Lucene - Core
  1. Lucene - Core
  2. LUCENE-2035

TokenSources.getTokenStream() does not assign positionIncrement

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4, 2.4.1, 2.9
    • Fix Version/s: 3.1, 4.0-ALPHA
    • Component/s: modules/highlighter
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      TokenSources.StoredTokenStream does not assign positionIncrement information. This means that all tokens in the stream are considered adjacent. This has implications for the phrase highlighting in QueryScorer when using non-contiguous tokens.

      For example:
      Consider a token stream that creates tokens for both the stemmed and unstemmed version of each word - the fox (jump|jumped)
      When retrieved from the index using TokenSources.getTokenStream(tpv,false), the token stream will be - the fox jump jumped

      Now try a search and highlight for the phrase query "fox jumped". The search will correctly find the document; the highlighter will fail to highlight the phrase because it thinks that there is an additional word between "fox" and "jumped". If we use the original (from the analyzer) token stream then the highlighter works.

      Also, consider the converse - the fox did not jump
      "not" is a stop word and there is an option to increment the position to account for stop words - (the,0) (fox,1) (did,2) (jump,4)
      When retrieved from the index using TokenSources.getTokenStream(tpv,false), the token stream will be - (the,0) (fox,1) (did,2) (jump,3).

      So the phrase query "did jump" will cause the "did" and "jump" terms in the text "did not jump" to be highlighted. If we use the original (from the analyzer) token stream then the highlighter works correctly.

      1. LUCENE-2305.patch
        20 kB
        Christopher Morris
      2. LUCENE-2035.patch
        20 kB
        Mark Miller
      3. LUCENE-2035.patch
        40 kB
        Mark Miller

        Activity

        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1
        Hide
        Uwe Schindler added a comment -

        Resolving again as this issue will not be backported to 2.9/3.0 branches.

        Show
        Uwe Schindler added a comment - Resolving again as this issue will not be backported to 2.9/3.0 branches.
        Hide
        Robert Muir added a comment -

        reopening for possible 2.9.4/3.0.3 backport.

        Show
        Robert Muir added a comment - reopening for possible 2.9.4/3.0.3 backport.
        Hide
        Mark Miller added a comment -

        Thanks Christopher!

        Show
        Mark Miller added a comment - Thanks Christopher!
        Hide
        Christopher Morris added a comment -

        Cheers Mark,

        The custom collector was probably because I was learning the new API at the time.

        The only changes I've made since the patch I submitted were to initialise the ArrayList with tpv.getTerms().length because that represents the minimum size that the list will grow to, and to replace the List and Iterator fields with an array (derived from the list) and an integer pointer. Both of which are probably unnecessary.

        The tests could be improved - the first case could be fixed in it's present form by using the Analyzer to generate the phrase query. If the stemmed word was the middle word of the phrase then that fix wouldn't work.

        Show
        Christopher Morris added a comment - Cheers Mark, The custom collector was probably because I was learning the new API at the time. The only changes I've made since the patch I submitted were to initialise the ArrayList with tpv.getTerms().length because that represents the minimum size that the list will grow to, and to replace the List and Iterator fields with an array (derived from the list) and an integer pointer. Both of which are probably unnecessary. The tests could be improved - the first case could be fixed in it's present form by using the Analyzer to generate the phrase query. If the stemmed word was the middle word of the phrase then that fix wouldn't work.
        Hide
        Mark Miller added a comment -

        I'll commit this soon.

        Show
        Mark Miller added a comment - I'll commit this soon.
        Hide
        Mark Miller added a comment -

        I've broken the new tests back out into there own file, change the hit collector code to just search basically, and improved the test coverage of TokenSources a bit.

        Show
        Mark Miller added a comment - I've broken the new tests back out into there own file, change the hit collector code to just search basically, and improved the test coverage of TokenSources a bit.
        Hide
        Mark Miller added a comment -

        Hey Christopher, why are you going through the trouble of the custom collector to check that there are no hits? Why not just do a standard search?

        Show
        Mark Miller added a comment - Hey Christopher, why are you going through the trouble of the custom collector to check that there are no hits? Why not just do a standard search?
        Hide
        Mark Miller added a comment -

        Thanks for the tests and fix Christopher!

        I've got one more patch coming and ill commit in a few days.

        I'm going to break the tests back out in a separate file again (on second thought I think how you had is a good idea) and remove an author tag. Then after one more review I think this good to go in.

        Show
        Mark Miller added a comment - Thanks for the tests and fix Christopher! I've got one more patch coming and ill commit in a few days. I'm going to break the tests back out in a separate file again (on second thought I think how you had is a good idea) and remove an author tag. Then after one more review I think this good to go in.
        Hide
        Christopher Morris added a comment -

        For the highlighter trunk

        Show
        Christopher Morris added a comment - For the highlighter trunk

          People

          • Assignee:
            Mark Miller
            Reporter:
            Christopher Morris
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development