LUCENE-9088: JapaneseNumberFilter uses inaccurate PartOfSpeechAttribute #1073
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the JapaneseNumberFilter reads past a single or multiple numeric tokens and emits the new composed token with the attributes of the following token. This will often lead to e.g. wrong part-of-speech attributes on the numeric token, which in turn can lead to wrong filtering by subsequent filters. This change keeps track of the state of the last numeric token while iterating over a number group and restores the last seen state before emiting the composed numeric token, so we use the attributes of the last one.
cbuescher
commented
Dec 11, 2019
| @@ -218,6 +228,11 @@ public final boolean incrementToken() throws IOException { | |||
| // capture the state of this token and emit it on our next incrementToken() | |||
| state = captureState(); | |||
| } | |||
| // we restore state to when we read the last numeral token to get its attributes (e.g. part-of-speech) | |||
| if (lastNumeralTokenState != null) { | |||
| restoreState(lastNumeralTokenState); | |||
There was a problem hiding this comment.
Note: simply setting the PartOfSpeechAttribute to "noun-numeric" on the emited token wasn't as straight forward as I expected, since the implementation wraps a whole org.apache.lucene.analysis.ja.Token. This is why I explored tracking and restoring the last "good" tokens state here.
madrob
reviewed
Dec 18, 2019
Comment on lines
219
to
221
| if (isNumeral(term)) { | ||
| // lastNumeralTokenState = captureState(); | ||
| } |
There was a problem hiding this comment.
Sorry, this shouldn't be left out, it's a left over from when I was doing some local debugging.
I'll push an update shortly. The classes javadocs say that the filter "will inherit the values of the last token used to compose the normalized number", and in order to do so, that state is saved here on every new numeral token encountered.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the JapaneseNumberFilter reads past a single or multiple numeric
tokens and emits the new composed token with the attributes of the following
token. This will often lead to e.g. wrong part-of-speech attributes on the
numeric token, which in turn can lead to wrong filtering by subsequent filters.
This change keeps track of the state of the last numeric token while iterating
over a number group and restores the last seen state before emiting the composed
numeric token, so we use the attributes of the last one.