Skip to content

Commit

Permalink
Fielddata: Fix iterator over global ordinals.
Browse files Browse the repository at this point in the history
Our iterator over global ordinals is currently incorrect since it does NOT
return -1 (NO_MORE_ORDS) when all ordinals have been consumed. This bug does
not strike immediately with elasticsearch since we always consume ordinals in
a random-access fashion. However it strikes when consuming ordinals through
Lucene helpers such as DocValues#docsWithField.

Close #8580
  • Loading branch information
jpountz committed Nov 24, 2014
1 parent 63db0b9 commit a33f453
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 76 deletions.
Expand Up @@ -38,7 +38,11 @@ public final void setDocument(int docID) {

@Override
public long nextOrd() {
return ordAt(i++);
if (i < cardinality()) {
return ordAt(i++);
} else {
return NO_MORE_ORDS;
}
}

}
Expand Up @@ -28,7 +28,8 @@
import org.elasticsearch.search.MultiValueMode;
import org.junit.Test;

import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;

public abstract class AbstractFieldDataImplTests extends AbstractFieldDataTests {

Expand Down Expand Up @@ -59,6 +60,11 @@ protected String toString(Object value) {

protected abstract void add2SingleValuedDocumentsAndDeleteOneOfThem() throws Exception;

protected long minRamBytesUsed() {
// minimum number of bytes that this fielddata instance is expected to require
return 1;
}

@Test
public void testDeletedDocs() throws Exception {
add2SingleValuedDocumentsAndDeleteOneOfThem();
Expand All @@ -78,7 +84,7 @@ public void testSingleValueAllSet() throws Exception {
IndexFieldData indexFieldData = getForField("value");
AtomicReaderContext readerContext = refreshReader();
AtomicFieldData fieldData = indexFieldData.load(readerContext);
assertThat(fieldData.ramBytesUsed(), greaterThan(0l));
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(minRamBytesUsed()));

SortedBinaryDocValues bytesValues = fieldData.getBytesValues();

Expand Down Expand Up @@ -141,7 +147,7 @@ public void testSingleValueWithMissing() throws Exception {
fillSingleValueWithMissing();
IndexFieldData indexFieldData = getForField("value");
AtomicFieldData fieldData = indexFieldData.load(refreshReader());
assertThat(fieldData.ramBytesUsed(), greaterThan(0l));
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(minRamBytesUsed()));

SortedBinaryDocValues bytesValues = fieldData
.getBytesValues();
Expand All @@ -158,7 +164,7 @@ public void testMultiValueAllSet() throws Exception {
fillMultiValueAllSet();
IndexFieldData indexFieldData = getForField("value");
AtomicFieldData fieldData = indexFieldData.load(refreshReader());
assertThat(fieldData.ramBytesUsed(), greaterThan(0l));
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(minRamBytesUsed()));

SortedBinaryDocValues bytesValues = fieldData.getBytesValues();

Expand Down Expand Up @@ -189,7 +195,7 @@ public void testMultiValueWithMissing() throws Exception {
fillMultiValueWithMissing();
IndexFieldData indexFieldData = getForField("value");
AtomicFieldData fieldData = indexFieldData.load(refreshReader());
assertThat(fieldData.ramBytesUsed(), greaterThan(0l));
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(minRamBytesUsed()));

SortedBinaryDocValues bytesValues = fieldData.getBytesValues();

Expand All @@ -203,7 +209,7 @@ public void testMissingValueForAll() throws Exception {
IndexFieldData indexFieldData = getForField("value");
AtomicFieldData fieldData = indexFieldData.load(refreshReader());
// Some impls (FST) return size 0 and some (PagedBytes) do take size in the case no actual data is loaded
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(0l));
assertThat(fieldData.ramBytesUsed(), greaterThanOrEqualTo(minRamBytesUsed()));

SortedBinaryDocValues bytesValues = fieldData.getBytesValues();

Expand Down

0 comments on commit a33f453

Please sign in to comment.