serhiy-bzhezytskyy commented on issue #14399: URL: https://github.com/apache/lucene/issues/14399#issuecomment-5005381173
I looked into this and was able to turn it into a concrete failing test: a `MultiReader` over two indexes sorted in opposite directions (`ndv asc` and `ndv desc`), searched with `ndv asc`. The competitive document lives at the end of the second segment's docid order, so the cached "search sort is a prefix of the index sort" decision from the first segment wrongly early-terminates the second segment and drops it — the search returns a non-competitive value instead of the true top hit. So it's a wrong-results bug, not just a theoretical one. I have a fix along the lines of what @jpountz suggested — decide per segment instead of caching across leaves. In `TopFieldLeafCollector` the prefix check is now computed for each segment, and when it's true the collector calls `disableSkipping()` on that segment's leaf comparators. On the method-move point: rather than moving `FieldComparator#disableSkipping` to `LeafFieldComparator`, I added `disableSkipping()` as a **default (no-op) method on `LeafFieldComparator`** and implemented it on `NumericComparator`/`TermOrdValComparator`'s leaf comparators (they drop their competitive iterator for that segment). That keeps it a backward-compatible API addition rather than a move/removal. `FieldComparator#disableSkipping` stays as-is since it's still used elsewhere. Does that direction seem acceptable, or would you prefer the full move you mentioned? Happy to open a PR with the fix + test if the approach looks right. -- 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]
