[
https://issues.apache.org/jira/browse/LUCENE-6014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14177279#comment-14177279
]
Robert Muir commented on LUCENE-6014:
-------------------------------------
Well again my concern is, the optimization is based on a non-obvious dependency
there between two classes (skipper and postingsreader) and afaik, omitting skip
point for first doc and so on was done after the fact, so i have concerns about
cases like advance() on "newborn" docsenum here... is the optimization safe? or
not?
Basically in such cases, a comment and ideally an assert are really needed.
Sometimes things are complicated, but if the code had these comments:
{code}
// NOTE: we don't have to check for buffer exhaustion because its guaranteed
either target is in this buffer, or we hit NO_MORE_DOCS
// this is because we have a skip per-block, and we always skip.
// See tests in XXX (some file that tests corner cases like df=128 and df=256
directly)
assert actuallySkipped || docFreq < blockSize;
{code}
Then I would not have complained. but as-is, who is to say this code is safe?
> Lucene41PostingsReader.advance() doc scanning is buggy
> ------------------------------------------------------
>
> Key: LUCENE-6014
> URL: https://issues.apache.org/jira/browse/LUCENE-6014
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Robert Muir
>
> The "code duplication" here to optimize scanning to target after "possible
> skipping" looks really buggy.
> For example, it never checks to refill buffer in the loop if it gets
> exhausted. But the code gives no indication about why its safe for it to make
> this assumption.
> If nobody understands why this optimization is allowed to make such a
> shortcut, then please add a comment indicating why, with asserts tied to
> exact specific constants in skipwriter (if applicable), otherwise i will
> remove the optimization completely in 72 hours.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
