dweiss commented on issue #15974:
URL: https://github.com/apache/lucene/issues/15974#issuecomment-4294139668
I asked claude and it came up with a very plausible cause of this failure -
```
The bug is an off-by-one error in SimpleTextDocValuesReader.getSkipper()
exposed by the new SkipBlockRangeIterator.
The SimpleText skipper bug
In SimpleTextDocValuesReader.java:919, maxDocID(int level) returned maxDoc
(the exclusive upper bound) instead of maxDoc - 1 (the last valid doc ID,
inclusive):
@Override
public int maxDocID(int level) {
...
return maxDoc; // BUG: valid doc IDs are 0..maxDoc-1, so max is maxDoc-1
}
Per the DocValuesSkipper contract: "Return the maximum doc ID of the
interval on the given level, inclusive." The maximum valid doc ID in any
segment is maxDoc - 1.
Why the old code survived
The old code in DocValuesRangeIterator used maxDocID(0) as a tracked upTo
boundary with a strict > comparison:
this.upTo = skipper.maxDocID(0); // = maxDoc (wrong, but harmless)
...
if (target > upTo) { // target > maxDoc — never triggered for
valid doc IDs!
Because target > maxDoc is never true for valid doc IDs (0 to maxDoc-1),
the skipper was simply never re-advanced for SimpleText. It also used
SortedNumericDocValues
as its approximation, which itself never returns maxDoc as a doc ID.
How the PR triggers the bug
SkipBlockRangeIterator (introduced in e56d5fa) correctly interprets
maxDocID(0) as inclusive, using:
if (target <= skipper.maxDocID(0)) { // target <= maxDoc — TRUE for
target=maxDoc!
if (doc > -1) {
return doc = target; // returns maxDoc as a doc ID!
}
}
When iterating docs in the last block of a SimpleText segment, nextDoc()
on the last valid doc (maxDoc - 1) calls advance(maxDoc). The condition maxDoc
<= maxDoc is
true, so the iterator returns doc = maxDoc. Then matches() calls
values.advanceExact(maxDoc), which seeks past the end of the data file in
SimpleTextDocValuesReader,
and the assertion StringHelper.startsWith(scratch.get(), LENGTH) at line
468 fails.
The fix
SimpleTextDocValuesReader.java:919 — return maxDoc - 1 instead of maxDoc:
return maxDoc - 1; // was: return maxDoc
The SkipBlockRangeIterator logic itself is correct per the contract.
SimpleTextDocValuesReader.getSkipper() had a latent off-by-one that the new
code tripped over.
```
I'll wait for somebody more knowledgeable with those iterators to confirm
but it does look like #15954 uncovered an existing bug in the codec.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
