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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
