jimczi commented on PR #16307: URL: https://github.com/apache/lucene/pull/16307#issuecomment-4913040237
> Is this something we should fix? I can see it being a source of bugs in future Well, you called it @romseygeek 😅 — that `docIDRunEnd()` default was exactly the source. Fixed on main (`6f3c1d7ad69`) and branch_10x (`d1e035949fb`). Quick rundown of what went wrong: bumping the `TwoPhaseIterator#docIDRunEnd()` default to `docID()+1` was a mistake. A two-phase iterator's current approximation doc is only a *candidate* until `matches()` confirms it, so the default has to stay conservative and report "no matching run" (`approximation().docID()`). `DenseConjunctionBulkScorer#scoreWindow` looks at `docIDRunEnd()` *before* calling `matches()`, so on a width-1 window at a partition boundary it treated the unconfirmed clause as fully-matching and collected the (out-of-range) boundary doc via `collectRange` — hence the ±1 under intra-segment concurrency. So I reverted the default and added a javadoc note on why it deliberately differs from `DocIdSetIterator`. The ReqExcl optimization is untouched (it only calls `docIDRunEnd()` after `matches()` and already clamps to `upTo+1`). Also added a `DenseConjunctionBulkScorer` test that checks a two-phase clause on the default `docIDRunEnd()` collects the same docs whether scored in one pass or as adjacent width-1 windows. Thanks @dweiss for the seed and the writeup 🙏 -- 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]
