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

[GitBox <gi...@apache.org>]

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


##########
lucene/core/src/java/org/apache/lucene/index/MappedMultiFields.java:
##########
@@ -43,8 +43,8 @@ public MappedMultiFields(MergeState mergeState, MultiFields multiFields) {
   @Override
   public Terms terms(String field) throws IOException {
     MultiTerms terms = (MultiTerms) in.terms(field);
-    if (terms == null) {
-      return null;
+    if (terms == null || terms == Terms.EMPTY) {

Review Comment:
   can we leave the `if` statement as is?



##########
lucene/core/src/java/org/apache/lucene/index/CheckIndex.java:
##########
@@ -1378,7 +1378,7 @@ private static Status.TermIndexStatus checkFields(
       computedFieldCount++;
 
       final Terms terms = fields.terms(field);
-      if (terms == null) {
+      if (terms == Terms.EMPTY) {

Review Comment:
   We should remove this `if` statement. There is a check a few lines above that indexing is enabled on the field, so terms must not be null.



##########
lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java:
##########
@@ -595,7 +595,7 @@ private void setField(String field) throws IOException {
 
     DocIdSetIterator nextTerm(String field, BytesRef term) throws IOException {
       setField(field);
-      if (termsEnum != null) {
+      if (termsEnum != null && termsEnum != TermsEnum.EMPTY) {

Review Comment:
   would it work to leave the `if` statement as is?



##########
lucene/test-framework/src/java/org/apache/lucene/tests/codecs/asserting/AssertingPostingsFormat.java:
##########
@@ -79,7 +79,10 @@ public Iterator<String> iterator() {
     @Override
     public Terms terms(String field) throws IOException {
       Terms terms = in.terms(field);
-      return terms == null ? null : new AssertingLeafReader.AssertingTerms(terms);
+      if (terms == Terms.EMPTY) {

Review Comment:
   let's remove this check, and assert that `terms` in not null (`terms(String field)` map only get called on codec APIs if the field is indexed



##########
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:
   since there are no ghost fields anymore, we should be able to remove the `if` statement entirely, does it cause test failures?



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