dsmiley commented on a change in pull request #2365:
URL: https://github.com/apache/lucene-solr/pull/2365#discussion_r576548809
##########
File path: lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
##########
@@ -617,8 +618,17 @@ public double doubleValue() throws IOException {
public boolean advanceExact(int doc) throws IOException {
if (disi.docID() < doc) {
disi.advance(doc);
+ tpiMatch = null;
}
- return disi.docID() == doc && (tpi == null || tpi.matches());
+ if (disi.docID() == doc) {
Review comment:
At first glance, this looked simpler and correct, but on further
reflection, I think this contains a bug. Lets say advanceExact doc==1 and we
call disi.advance(1) and it arrives at 2 because the disi doesn't match 1.
We'd then set matched=false and return (fine; it doesn't match doc==1). Then
say the caller calls advanceExact doc==2. We'd quick return "matched" which is
false. But that's a mistake because the disi in fact matches doc==2 (ignoring
tpi; say it's null).
I like the Boolean tpiMatch because it caches the very thing that needs to
be cached, and it holds null to say we don't know or it doesn't matter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
