Lucene - Core
  1. Lucene - Core
  2. LUCENE-1871

Highlighter wraps caching token filters that are not CachingTokenFilter in CachingTokenFilter

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 2.9
    • Component/s: modules/highlighter
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      I figured this was fine and a rare case that you would have another caching tokenstream to feed the highlighter with - but I guess if its happening to you, especially depending on what you are doing - its not an ideal situation.

        Activity

        Hide
        Mark Miller added a comment -

        Allows you to take ownership of providing an efficiently resettable TokenStream for highlighting

        Show
        Mark Miller added a comment - Allows you to take ownership of providing an efficiently resettable TokenStream for highlighting
        Hide
        Uwe Schindler added a comment -

        Are we also in feature freeze for contrib? As this solution does not affect anything in Token, only the highlighter, we might commit this?

        Show
        Uwe Schindler added a comment - Are we also in feature freeze for contrib? As this solution does not affect anything in Token, only the highlighter, we might commit this?
        Hide
        Mark Miller added a comment -

        With consensus on my side, I have no problem putting this in.

        Show
        Mark Miller added a comment - With consensus on my side, I have no problem putting this in.
        Hide
        Uwe Schindler added a comment -

        I would like to have this in, because there are other use-cases of this. Maybe one TokenStream chain is not caching on the top filter, but maybe directly after the tokenizer and before the lowercase filter (or something like that). In this case you would also add an additional caching to the top of the stream without really needing it.
        Because of this, the marker-interface method would not do it, too, only something like isRewindable() or isCachingTokens would do it (if Filters pass it up to their input). So as far as one filter would cache tokens, you could detect it even on the top stream.
        This is the same like the well-known InputStream usage:

        if (!stream.markSupported) stream=new BufferedInputStream(stream);

        . markSupported of Filter streams also pass to their input.

        I have other places, where it would be good to know if one stream supports reset() for rewinding (or rewind(), which I would prefer). We had this discussion on java-dev some time ago, about decoupling rewinding from resetting a stream.

        Show
        Uwe Schindler added a comment - I would like to have this in, because there are other use-cases of this. Maybe one TokenStream chain is not caching on the top filter, but maybe directly after the tokenizer and before the lowercase filter (or something like that). In this case you would also add an additional caching to the top of the stream without really needing it. Because of this, the marker-interface method would not do it, too, only something like isRewindable() or isCachingTokens would do it (if Filters pass it up to their input). So as far as one filter would cache tokens, you could detect it even on the top stream. This is the same like the well-known InputStream usage: if (!stream.markSupported) stream= new BufferedInputStream(stream); . markSupported of Filter streams also pass to their input. I have other places, where it would be good to know if one stream supports reset() for rewinding (or rewind(), which I would prefer). We had this discussion on java-dev some time ago, about decoupling rewinding from resetting a stream.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development