Skip to content

Commit

Permalink
Prevent PointValues from returning null for ghost fields
Browse files Browse the repository at this point in the history
getPointValues may currently return null for unknown fields or fields that don't index points.
It can happen that a field no longer has points for any document in a segment after delete+merge, which
causes field info to think that the field is there and has points, yet when calling getPointValues null
is returned.

With this change, we prevent getPointValues from returning null for ghost fields, it will instead return an
empty instance of PointValues.

Relates to apache#11393
  • Loading branch information
javanna committed Sep 20, 2022
1 parent 9acc653 commit 799d30e
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ public PointValues getValues(String fieldName) {
throw new IllegalArgumentException("field=\"" + fieldName + "\" did not index point values");
}

return readers.get(fieldInfo.number);
PointValues pointValues = readers.get(fieldInfo.number);
return pointValues == null ? PointValues.EMPTY : pointValues;
}

@Override
Expand Down
87 changes: 87 additions & 0 deletions lucene/core/src/java/org/apache/lucene/index/PointValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -467,4 +467,91 @@ public final long estimateDocCount(IntersectVisitor visitor) {

/** Returns the total number of documents that have indexed at least one point. */
public abstract int getDocCount();

private static final PointTree EMPTY_POINT_TREE =
new PointTree() {
@Override
public PointTree clone() {
throw new UnsupportedOperationException();
}

@Override
public boolean moveToChild() {
return false;
}

@Override
public boolean moveToSibling() {
return false;
}

@Override
public boolean moveToParent() {
return false;
}

@Override
public byte[] getMinPackedValue() {
return new byte[0];
}

@Override
public byte[] getMaxPackedValue() {
return new byte[0];
}

@Override
public long size() {
return 0;
}

@Override
public void visitDocIDs(IntersectVisitor visitor) {}

@Override
public void visitDocValues(IntersectVisitor visitor) {}
};

public static final PointValues EMPTY =
new PointValues() {
@Override
public PointTree getPointTree() {
return EMPTY_POINT_TREE;
}

@Override
public byte[] getMinPackedValue() {
return null;
}

@Override
public byte[] getMaxPackedValue() {
return null;
}

@Override
public int getNumDimensions() {
return 0;
}

@Override
public int getNumIndexDimensions() {
return 0;
}

@Override
public int getBytesPerDimension() {
return 0;
}

@Override
public long size() {
return 0;
}

@Override
public int getDocCount() {
return 0;
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,16 @@ public void testDeleteAllPointDocs() throws Exception {

w.forceMerge(1);
DirectoryReader r = DirectoryReader.open(w);
assertNull(r.leaves().get(0).reader().getPointValues("int"));
PointValues emptyValues = r.leaves().get(0).reader().getPointValues("int");
assertEquals(0, emptyValues.getDocCount());
assertEquals(0, emptyValues.size());
assertEquals(0, emptyValues.getNumDimensions());
assertEquals(0, emptyValues.getNumIndexDimensions());
assertEquals(0, emptyValues.getBytesPerDimension());
assertEquals(0, emptyValues.getPointTree().size());
assertFalse(emptyValues.getPointTree().moveToChild());
assertFalse(emptyValues.getPointTree().moveToParent());
assertFalse(emptyValues.getPointTree().moveToSibling());
w.close();
r.close();
dir.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,28 @@ private float[] randomVector(int dim) {
return v;
}

public void testDeleteAllPointDocs() throws Exception {
try (Directory dir = newDirectory();
RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {

Document doc = new Document();
doc.add(new StringField("id", "0", Field.Store.NO));
doc.add(new LongPoint("long", 17));
doc.add(new NumericDocValuesField("long", 17));
iw.addDocument(doc);
iw.addDocument(new Document());
iw.commit();

iw.deleteDocuments(new Term("id", "0"));
iw.forceMerge(1);

try (IndexReader reader = iw.getReader()) {
IndexSearcher searcher = newSearcher(reader);
assertEquals(0, searcher.count(new FieldExistsQuery("long")));
}
}
}

private void assertSameMatches(IndexSearcher searcher, Query q1, Query q2, boolean scores)
throws IOException {
final int maxDoc = searcher.getIndexReader().maxDoc();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.index.DocValues;
import org.apache.lucene.index.DocValuesType;
Expand Down Expand Up @@ -1112,8 +1111,8 @@ public PointValues getWrapped() {
}

private void assertStats(int maxDoc) {
assert in.size() > 0;
assert in.getDocCount() > 0;
assert in.size() >= 0;
assert in.getDocCount() >= 0;
assert in.getDocCount() <= in.size();
assert in.getDocCount() <= maxDoc;
}
Expand All @@ -1127,13 +1126,13 @@ public PointTree getPointTree() throws IOException {
@Override
public byte[] getMinPackedValue() throws IOException {
assertThread("Points", creationThread);
return Objects.requireNonNull(in.getMinPackedValue());
return in.getMinPackedValue();
}

@Override
public byte[] getMaxPackedValue() throws IOException {
assertThread("Points", creationThread);
return Objects.requireNonNull(in.getMaxPackedValue());
return in.getMaxPackedValue();
}

@Override
Expand Down Expand Up @@ -1210,7 +1209,7 @@ public byte[] getMaxPackedValue() {
@Override
public long size() {
final long size = in.size();
assert size > 0;
assert size >= 0;
return size;
}

Expand Down

0 comments on commit 799d30e

Please sign in to comment.