You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/06/13 18:17:49 UTC

[GitHub] [lucene] shahrs87 commented on a diff in pull request #907: LUCENE-10357 Ghost fields and postings/points

shahrs87 commented on code in PR #907:
URL: https://github.com/apache/lucene/pull/907#discussion_r896006974


##########
lucene/codecs/src/java/org/apache/lucene/codecs/bloom/BloomFilteringPostingsFormat.java:
##########
@@ -200,8 +200,8 @@ public Terms terms(String field) throws IOException {
         return delegateFieldsProducer.terms(field);
       } else {
         Terms result = delegateFieldsProducer.terms(field);
-        if (result == null) {
-          return null;
+        if (result == null || result == Terms.EMPTY) {

Review Comment:
   Yes, this test case is failing even with this patch: 
   `TestMemoryIndexAgainstDirectory#testRandomQueries` 
   Reproducible by: `gradlew :lucene:memory:test --tests "org.apache.lucene.index.memory.TestMemoryIndexAgainstDirectory.testRandomQueries" -Ptests.jvms=8 -Ptests.jvmargs=-XX:TieredStopAtLevel=1 -Ptests.seed=B19145C39C34BD03 -Ptests.gui=false -Ptests.file.encoding=UTF-8`
   
   The underlying reader it is using is MemoryIndex#MemoryIndexReader [here](https://github.com/apache/lucene/blob/main/lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java#L1405)
   This is the relevant snippet.
   ```
           if (info == null || info.numTokens <= 0) {
             return null;
           }
   ```
   
   Below is the text I copied from LUCENE-10357 description.
   
   > I fear that this could be a source of bugs, as a caller could be tempted to assume that he would get non-null terms on a FieldInfo that has IndexOptions that are not NONE. Should we introduce a contract that FieldsProducer (resp. PointsReader) must return a non-null instance when postings (resp. points) are indexed?
   
   I don't know which all places I need to do null check ? From the above description, looks like only in FieldsProducer related classes. From my limited understanding, this doesn't look like FiledsProducer. @jpountz  please advise.
   



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