[GitHub] [lucene] epotyom commented on a diff in pull request #12555: Fix: Lucene90DocValuesProducer.TermsDict.seekCeil doesn't always position bytes correctly (#12167)

["epotyom (via GitHub)" <gi...@apache.org>]

epotyom commented on code in PR #12555:
URL: https://github.com/apache/lucene/pull/12555#discussion_r1327195378


##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##########
@@ -1205,7 +1205,15 @@ public SeekStatus seekCeil(BytesRef text) throws IOException {
           ord = 0;
           return SeekStatus.END;
         } else {
-          seekExact(0L);
+          // seekBlock doesn't update ord and it repositions bytes when calls getFirstTermFromBlock

Review Comment:
   > in case we're changing the seek behavior in the future.
   
   This is the reason why I think we should not rely on seekExact. For example, if we do:
   
   ```
   ord = 1;
   seekExact(0L);
   ```
   
   and in the future we optimize `seekExact` to not reset `bytes`, this can break again, because seekExact relies on TermDict to be in consistent state. Just to give an example of potential optimization: both ords `0` and `1` are in the same block.  So, when we seek from `1` to `0` we can try using data from the current block without re-reading data from `bytes`?  Even if this particular idea doesn't work, I think that that public methods of this class rely on data to be in a consistent state, so we should probably not rely on them to fix the state?



-- 
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