Fix IndexOutOfBoundsException thrown in DefaultPassageFormatter by unordered matches #13315
Conversation
…faultPassageFormatter Unit test was taken from the mailing list.
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution! |
| } catch (IOException e) { | ||
| throw new IllegalArgumentException("Failed to retrieve term offset", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's weird that start/endOffset can throw IOException but start/endPosition do not :) Nothing to fix in this PR, just noting the inconsistency ...
There was a problem hiding this comment.
It's because the work of loading positions is always done in next(), so there's no IO involved when calling startPosition or endPosition. But we only load offsets when specifically asked, so IO may be involved when calling those methods. It does look a bit funky though, I agree!
|
What a fun and tricky corner case -- thank you @scampi for uncovering this, showing the bug with the added unit tests, and the tentative fix. I think it is actually technically allowed to index term vectors with offsets only (no positions) and then hilite from there: Lucene is recording the character offsets of every occurence of each unique term, per document, and that's all that is needed to hilite. So I like your fix -- I'll merge it soon unless anyone objects. Could you maybe add a I think the intent of |
| } catch (IOException e) { | ||
| throw new IllegalArgumentException("Failed to retrieve term offset", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's because the work of loading positions is always done in next(), so there's no IO involved when calling startPosition or endPosition. But we only load offsets when specifically asked, so IO may be involved when calling those methods. It does look a bit funky though, I agree!
Fix #12431
Description
DefaultPassageFormattermay cause anIndexOutOfBoundsExceptionin case matches of a passage are out of order.This happens when term vectors are stored but not the positions.
The tentative solution consists in ordering matches according to offsets in
DisjunctionMatchesIteratorin case positions don't exist.However, I wonder if it is correct to create such an iterator when positions are not available. Matches with no terms are explicitly removed, and the javadoc of
MATCH_WITH_NO_TERMSmentions that it indicates a match with no term positions. Therefore, from that doc it shouldn't be possible to highlight text without positions. Is that javadoc correct ? Should it be updated ?lucene/lucene/core/src/java/org/apache/lucene/search/MatchesUtils.java
Line 67 in bc678ac
lucene/lucene/core/src/java/org/apache/lucene/search/MatchesUtils.java
Lines 40 to 44 in bc678ac