You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "fthevenet (via GitHub)" <gi...@apache.org> on 2023/03/23 16:14:46 UTC

[GitHub] [lucene] fthevenet opened a new issue, #12211: Searches made via DrillSideways may miss documents that should match the query.

fthevenet opened a new issue, #12211:
URL: https://github.com/apache/lucene/issues/12211

   ### Description
   
   Hi,
   
   I use `DrillSideways` quite heavily in a project of mine and it recently realized that sometimes some documents that *should* match a query do not, whenever at least one component of the `DrillDownQuery` involved was of type `PhraseQuery`. 
   This behaviour would be reproducible **every** time from a freshly started application, provided that the indexed corpus stays _exactly the same_. To make matter worst, however, the document that wouldn't match in a given corpus, would something match when part of a slightly different corpus (i.e. more or less documents), and also, after the application had been running for a while, documents that would not match previously would suddenly start showing up...
   
   After quite a bit of debugging, I'm happy to report I believe I have a pretty good idea of what the issue is (and how to fix it).
   In a nutshell, the problem arises from what I believe is a bug in `DrillSidewaysQueryScorer::score`method, where in the course of a search the following code is called:
   https://github.com/apache/lucene/blob/0782535017c9e737350e96fb0f53457c7b8ecf03/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java#L132-L136
   
   First of all, the fact that we call `baseIterator.nextDoc()` here appears inefficient in the event that the iterator for the scorer is a "Two phase iterator", i.e. one that allows to iterate on an approximation of the query matches and confirm that an approximated doc is indeed a match in a second phase, as in this case  `baseIterator.nextDoc()` will always call the more costly matcher and never the approximation.
   But the real problem is the fact that in this case this will lead to a first call to `ExactPhraseMatcher::nextMatch` method and then shortly after a second call to the same matcher as part of the more in depth doc collection:
   https://github.com/apache/lucene/blob/0782535017c9e737350e96fb0f53457c7b8ecf03/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java#L148-L157
   
   In either doQueryFirstScoring or doDrillDownAdvanceScoring, a *second* call to `TwoPhaseMatcher::matches` will be made without the iterator has been re-positioned with a call to `TwoPhaseMatcher::nextDoc`, which the documentation explicitly warns against: 
   ``` java
     /**
      * Return whether the current doc ID that {@link #approximation()} is on matches. This method
      * should only be called when the iterator is positioned -- ie. not when {@link
      * DocIdSetIterator#docID()} is {@code -1} or {@link DocIdSetIterator#NO_MORE_DOCS} -- and at most
      * once.
      */
     public abstract boolean matches() throws IOException;
   ```
   And indeed what I observed is that although an attempt is made to reset the matcher itself in between these two calls to `TwoPhaseMatcher::matches`, this does not reset *the state within the posting themselves*, and this causes some undefined behaviours when attempting to match as inner positions overflows.
   
   Based on these findings, I believe the appropriate fix is to replace: 
   https://github.com/apache/lucene/blob/0782535017c9e737350e96fb0f53457c7b8ecf03/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java#L133
   
   with: 
   ``` java
   baseApproximation.nextDoc()
   ```
   I have back-ported that change in a local build of 9.5.0 and confirmed that if fixes my problem; I will therefore submit a pull-request with the hope that it gets reviewed and merged if deemed the correct way to address the issue.
   
   Cheers,
   Frederic
   
   
   
   
   ### Version and environment details
   
   - OS: Fedora 37 / Windows 10
   - Java Runtime: OpenJDK Runtime Environment Temurin-19.0.2+7 (build 19.0.2+7)
   - Lucene version: 9.5.0


-- 
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: issues-unsubscribe@lucene.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] gsmiller commented on issue #12211: Searches made via DrillSideways may miss documents that should match the query.

Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on issue #12211:
URL: https://github.com/apache/lucene/issues/12211#issuecomment-1481642864

   Great find, and great bug report! Thank you so much! Having a look at the PR now, but it seems reasonable to me.


-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] zhaih closed issue #12211: Searches made via DrillSideways may miss documents that should match the query.

Posted by "zhaih (via GitHub)" <gi...@apache.org>.
zhaih closed issue #12211: Searches made via DrillSideways may miss documents that should match the query.
URL: https://github.com/apache/lucene/issues/12211


-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] zhaih commented on issue #12211: Searches made via DrillSideways may miss documents that should match the query.

Posted by "zhaih (via GitHub)" <gi...@apache.org>.
zhaih commented on issue #12211:
URL: https://github.com/apache/lucene/issues/12211#issuecomment-1496440798

   Seems the bug is fixed, closing the issue. Thanks @fthevenet 


-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org