lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (JIRA)" <>
Subject [jira] Updated: (LUCENE-1448) add getFinalOffset() to TokenStream
Date Tue, 11 Nov 2008 09:52:44 GMT


Michael McCandless updated LUCENE-1448:

    Attachment: LUCENE-1448.patch

First cut patch.  It's not ready, though.

First: this patch only addresses the final offset, but shouldn't we also address the final
position?  Eg if StopFilter removes the last few tokens, shouldn't we make it possible to
report those skipped positonIncrements?  Note that Analyzer does provide a getPositionIncrementGap(String
fieldName), which could be used as a workaround if we don't do this.  This does seem less
important so I'd be OK with doing nothing about positions for now, but I'd like to hear other's

Second: I can't figure out how to ask StandardTokenizerImpl for the number of chars it pulled
from the Reader.  Can someone (who understands JFlex well) help out here?

Third: I haven't done all Tokenizers; eg, SinkTokenizer will need a new ctor so that it's
told the final offset.  And I'd like to fix all contrib tokenizers too.

Fourth: I'm nervous about an off-by-one error here.  Here's the diff for DocInverterPerField:

@@ -161,7 +161,13 @@
-            fieldState.offset = offsetEnd+1;
+            final int finalOffset = stream.getFinalOffset();
+            if (finalOffset == -1)
+              fieldState.offset = offsetEnd+1;
+            else
+              fieldState.offset += finalOffset;
           } finally {

What makes me nervous is: our current logic is to set the base for future instances of the
same field to the last endOffset, plus 1.  Why do we have that plus 1?  Logically, I think
it should be as if you had simply concatenated the strings together, which would not add an
extra +1?  So, I'm not adding +1 in the getFinalOffset()'s that I've added, but I'm hoping
someone can explain why we are adding +1 currently.

Note that endOffset is normally "exclusive" while startOffset is "inclusive".  Ie if a String
starts with "abcd " then WhitespaceTokenizer would report a startOffset of 0 and endOffset
of 4 for that first token.

> add getFinalOffset() to TokenStream
> -----------------------------------
>                 Key: LUCENE-1448
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Analysis
>            Reporter: Michael McCandless
>            Assignee: Michael McCandless
>            Priority: Minor
>             Fix For: 2.9
>         Attachments: LUCENE-1448.patch
> If you add multiple Fieldable instances for the same field name to a document, and you
then index those fields with TermVectors storing offsets, it's very likely the offsets for
all but the first field instance will be wrong.
> This is because IndexWriter under the hood adds a cumulative base to the offsets of each
field instance, where that base is 1 + the endOffset of the last token it saw when analyzing
that field.
> But this logic is overly simplistic.  For example, if the WhitespaceAnalyzer is being
used, and the text being analyzed ended in 3 whitespace characters, then that information
is lost and then next field's offsets are then all 3 too small.  Similarly, if a StopFilter
appears in the chain, and the last N tokens were stop words, then the base will be 1 + the
endOffset of the last non-stopword token.
> To fix this, I'd like to add a new getFinalOffset() to TokenStream.  I'm thinking by
default it returns -1, which means "I don't know so you figure it out", meaning we fallback
to the faulty logic we have today.
> This has come up several times on the user's list.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message