Attached the program that reproduces the problem. In the program, I didn't use FastVectorHighlighter, instead, I printed out offsets from TermVectorOffsetInfo. You'll see the following results:

=== WhitespaceAnalyzer ===
あい(0,2)
うえお(3,6)
=== CJKAnalyzer ===
あい(0,2)
うえ(4,6)
えお(5,7)
=== BasicNGramAnalyzer ===
あい(0,2)
うえ(3,5)
えお(4,6)

For people who are seeing garbage characters, I want to rephrase using 'Cn' symbols as follows:

=== WhitespaceAnalyzer ===
C1C2(0,2)
C3C4C5(3,6)
=== CJKAnalyzer ===
C1C2(0,2)
C3C4(4,6)
C4C5(5,7)
=== BasicNGramAnalyzer ===
C1C2(0,2)
C3C4(3,5)
C4C5(4,6)

As you can see, the start offset of 'C3' is 3 in WhitespaceAnalyzer and BasicNGramAnalyzer (an analyzer which uses BasicNGramTokenizer. BasicNGramTokenizer is used in FastVectorHighlighter test code. It works as a 2-gram tokenizer for not only CJK but also ASCII), but is 4 in CJKAnalyzer – incorrect!

I'll look into it tomorrow or after, but volunteers are welcome!

Hi Koji, this looks like a bug in CJK offset calculations, probably involving end()

Personally i find the offset logic a little complex. It currently does both additions and subtractions to the offset and I think there is an off-by-one error in this.

I will play around and see if I can simplify this logic, but please don't wait on me, if you have an idea already how to fix it!

ok i found the bug. the problem is incrementToken() unconditionally increments the offset before it starts its main loop:

line 165:

offset++;

so, when incrementToken() has no more text to return and returns false, it needs to subtract from this.

again i think in the future we try to refactor this offset logic to be simpler, but for the short term, this fixes the bug and all tests pass.

Koji, can you review?

i added a testcase for end() to my patch that fails on trunk, but passes with the fix.

hello, this tokenizer has more serious offset/end problems than I originally thought.

attached is my previous patch and testcase but with 3 more testcases, one still fails.

Hi Robert, thank you for looking this so quickly!

ok i found the bug. the problem is incrementToken() unconditionally increments the offset before it starts its main loop:

line 165:

offset++;

Indeed.

In attached patch, I added one more offset-- line and two more testcases. All tests pass and this patch fixes the original problem that was found in Solr with FastVectorHighlighter.

In attached patch, I added one more offset-- line and two more testcases. All tests pass and this patch fixes the original problem that was found in Solr with FastVectorHighlighter.

nice, fix looks good to me.

Koji, I am testing other end() offsets with all other tokenizers, I noticed that CJKTokenizer does not call correctOffset() in end:

final int finalOffset = offset;

I think instead this should be

final int finalOffset = correctOffset(offset);

in case there is a CharFilter present.

I think instead this should be

final int finalOffset = correctOffset(offset);

Agreed, thank you for pointing this!
I think this is ready to commit. Robert, can you do it? And it'd be great if it could go 2.9 branch so that Solr can use the fix.

Koji, sure I can take care of it.

Also i added LUCENE-2219 to find these bugs in other tokenizers.

In the future I also want to explore if we can somehow use a fake CharFilter in BaseTokenStreamTest to also ensure that correctOffset() is called when setting offsets in both incrementToken() and end(), don't yet know how it would work yet.

thanks Koji!