Skip to content

Commit

Permalink
Calculate and cache result when advanceExact is called (#26920)
Browse files Browse the repository at this point in the history 
Cache final result instead of result of advanceExact.
Fix SortedNumericDoubleValues does not test MEDIAN mode
Replace deprecated random string generation method
  • Loading branch information
@jpountz
kel authored and jpountz committed Oct 9, 2017
1 parent edcbe9f commit a6ebf8b
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 29 deletions.
32 changes: 15 additions & 17 deletions core/src/main/java/org/elasticsearch/search/MultiValueMode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,11 @@ public NumericDocValues select(final SortedNumericDocValues values, final long m
if (singleton != null) {
return new AbstractNumericDocValues() {

private boolean hasValue;
private long value;

@Override
public boolean advanceExact(int target) throws IOException {
hasValue = singleton.advanceExact(target);
this.value = singleton.advanceExact(target) ? singleton.longValue() : missingValue;
return true;
}

Expand All @@ -431,17 +431,17 @@ public int docID() {

@Override
public long longValue() throws IOException {
return hasValue ? singleton.longValue() : missingValue;
return this.value;
}
};
} else {
return new AbstractNumericDocValues() {

private boolean hasValue;
private long value;

@Override
public boolean advanceExact(int target) throws IOException {
hasValue = values.advanceExact(target);
this.value = values.advanceExact(target) ? pick(values) : missingValue;
return true;
}

Expand All @@ -452,7 +452,7 @@ public int docID() {

@Override
public long longValue() throws IOException {
return hasValue ? pick(values) : missingValue;
return value;
}
};
}
Expand Down Expand Up @@ -533,35 +533,33 @@ public NumericDoubleValues select(final SortedNumericDoubleValues values, final
final NumericDoubleValues singleton = FieldData.unwrapSingleton(values);
if (singleton != null) {
return new NumericDoubleValues() {

private boolean hasValue;
private double value;

@Override
public boolean advanceExact(int doc) throws IOException {
hasValue = singleton.advanceExact(doc);
this.value = singleton.advanceExact(doc) ? singleton.doubleValue() : missingValue;
return true;
}

@Override
public double doubleValue() throws IOException {
return hasValue ? singleton.doubleValue() : missingValue;
return this.value;
}

};
} else {
return new NumericDoubleValues() {

private boolean hasValue;
private double value;

@Override
public boolean advanceExact(int target) throws IOException {
hasValue = values.advanceExact(target);
value = values.advanceExact(target) ? pick(values) : missingValue;
return true;
}

@Override
public double doubleValue() throws IOException {
return hasValue ? pick(values) : missingValue;
return this.value;
}
};
}
Expand Down Expand Up @@ -638,17 +636,17 @@ public BinaryDocValues select(final SortedBinaryDocValues values, final BytesRef
}
return new AbstractBinaryDocValues() {

private boolean hasValue;
private BytesRef value;

@Override
public boolean advanceExact(int target) throws IOException {
hasValue = singleton.advanceExact(target);
this.value = singleton.advanceExact(target) ? singleton.binaryValue() : missingValue;
return true;
}

@Override
public BytesRef binaryValue() throws IOException {
return hasValue ? singleton.binaryValue() : missingValue;
return this.value;
}
};
} else {
Expand Down
63 changes: 51 additions & 12 deletions core/src/test/java/org/elasticsearch/search/MultiValueModeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.search;

import com.carrotsearch.randomizedtesting.generators.RandomStrings;

import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.index.DocValues;
import org.apache.lucene.index.NumericDocValues;
Expand Down Expand Up @@ -160,6 +158,8 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
for (int i = 0; i < maxDoc; ++i) {
assertTrue(selected.advanceExact(i));
final long actual = selected.longValue();
verifyLongValueCanCalledMoreThanOnce(selected, actual);

long expected = 0;
if (values.advanceExact(i) == false) {
expected = missingValue;
Expand Down Expand Up @@ -203,6 +203,12 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
}
}

private void verifyLongValueCanCalledMoreThanOnce(NumericDocValues values, long expected) throws IOException {
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
assertEquals(expected, values.longValue());
}
}

private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
for (long missingValue : new long[] { 0, randomLong() }) {
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX, MultiValueMode.SUM, MultiValueMode.AVG}) {
Expand All @@ -212,6 +218,8 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
assertTrue(selected.advanceExact(root));
final long actual = selected.longValue();
verifyLongValueCanCalledMoreThanOnce(selected, actual);

long expected = 0;
if (mode == MultiValueMode.MAX) {
expected = Long.MIN_VALUE;
Expand Down Expand Up @@ -320,14 +328,13 @@ public int docValueCount() {
private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> supplier, int maxDoc) throws IOException {
for (long missingValue : new long[] { 0, randomLong() }) {
for (MultiValueMode mode : MultiValueMode.values()) {
if (MultiValueMode.MEDIAN.equals(mode)) {
continue;
}
SortedNumericDoubleValues values = supplier.get();
final NumericDoubleValues selected = mode.select(values, missingValue);
for (int i = 0; i < maxDoc; ++i) {
assertTrue(selected.advanceExact(i));
final double actual = selected.doubleValue();
verifyDoubleValueCanCalledMoreThanOnce(selected, actual);

double expected = 0.0;
if (values.advanceExact(i) == false) {
expected = missingValue;
Expand Down Expand Up @@ -371,6 +378,12 @@ private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> suppl
}
}

private void verifyDoubleValueCanCalledMoreThanOnce(NumericDoubleValues values, double expected) throws IOException {
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
assertTrue(Double.compare(values.doubleValue(), expected) == 0);
}
}

private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
for (long missingValue : new long[] { 0, randomLong() }) {
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX, MultiValueMode.SUM, MultiValueMode.AVG}) {
Expand All @@ -379,7 +392,9 @@ private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> suppl
int prevRoot = -1;
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
assertTrue(selected.advanceExact(root));
final double actual = selected.doubleValue();;
final double actual = selected.doubleValue();
verifyDoubleValueCanCalledMoreThanOnce(selected, actual);

double expected = 0.0;
if (mode == MultiValueMode.MAX) {
expected = Long.MIN_VALUE;
Expand Down Expand Up @@ -421,7 +436,7 @@ public void testSingleValuedStrings() throws Exception {
final FixedBitSet docsWithValue = randomBoolean() ? null : new FixedBitSet(numDocs);
for (int i = 0; i < array.length; ++i) {
if (randomBoolean()) {
array[i] = new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8));
array[i] = new BytesRef(randomAlphaOfLengthBetween(8, 8));
if (docsWithValue != null) {
docsWithValue.set(i);
}
Expand Down Expand Up @@ -456,7 +471,7 @@ public void testMultiValuedStrings() throws Exception {
for (int i = 0; i < numDocs; ++i) {
final BytesRef[] values = new BytesRef[randomInt(4)];
for (int j = 0; j < values.length; ++j) {
values[j] = new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8));
values[j] = new BytesRef(randomAlphaOfLengthBetween(8, 8));
}
Arrays.sort(values);
array[i] = values;
Expand Down Expand Up @@ -489,13 +504,15 @@ public int docValueCount() {
}

private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int maxDoc) throws IOException {
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8)) }) {
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(randomAlphaOfLengthBetween(8, 8)) }) {
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
SortedBinaryDocValues values = supplier.get();
final BinaryDocValues selected = mode.select(values, missingValue);
for (int i = 0; i < maxDoc; ++i) {
assertTrue(selected.advanceExact(i));
final BytesRef actual = selected.binaryValue();
verifyBinaryValueCanCalledMoreThanOnce(selected, actual);

BytesRef expected = null;
if (values.advanceExact(i) == false) {
expected = missingValue;
Expand Down Expand Up @@ -524,15 +541,23 @@ private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int ma
}
}

private void verifyBinaryValueCanCalledMoreThanOnce(BinaryDocValues values, BytesRef expected) throws IOException {
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
assertEquals(values.binaryValue(), expected);
}
}

private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8)) }) {
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(randomAlphaOfLengthBetween(8, 8)) }) {
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
SortedBinaryDocValues values = supplier.get();
final BinaryDocValues selected = mode.select(values, missingValue, rootDocs, new BitSetIterator(innerDocs, 0L), maxDoc);
int prevRoot = -1;
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
assertTrue(selected.advanceExact(root));
final BytesRef actual = selected.binaryValue();
verifyBinaryValueCanCalledMoreThanOnce(selected, actual);

BytesRef expected = null;
for (int child = innerDocs.nextSetBit(prevRoot + 1); child != -1 && child < root; child = innerDocs.nextSetBit(child + 1)) {
if (values.advanceExact(child)) {
Expand Down Expand Up @@ -658,7 +683,11 @@ private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc)
SortedSetDocValues values = supplier.get();
final SortedDocValues selected = mode.select(values);
for (int i = 0; i < maxDoc; ++i) {
final long actual = selected.advanceExact(i) ? selected.ordValue() : -1;
long actual = -1;
if (selected.advanceExact(i)) {
actual = selected.ordValue();
verifyOrdValueCanCalledMoreThanOnce(selected, selected.ordValue());
}
int expected = -1;
if (values.advanceExact(i)) {
for (long ord = values.nextOrd(); ord != SortedSetDocValues.NO_MORE_ORDS; ord = values.nextOrd()) {
Expand All @@ -679,13 +708,23 @@ private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc)
}
}

private void verifyOrdValueCanCalledMoreThanOnce(SortedDocValues values, long expected) throws IOException {
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
assertEquals(values.ordValue(), expected);
}
}

private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
SortedSetDocValues values = supplier.get();
final SortedDocValues selected = mode.select(values, rootDocs, new BitSetIterator(innerDocs, 0L));
int prevRoot = -1;
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
final int actual = selected.advanceExact(root) ? selected.ordValue() : -1;
int actual = -1;
if (selected.advanceExact(root)) {
actual = selected.ordValue();
verifyOrdValueCanCalledMoreThanOnce(selected, actual);
}
int expected = -1;
for (int child = innerDocs.nextSetBit(prevRoot + 1); child != -1 && child < root; child = innerDocs.nextSetBit(child + 1)) {
if (values.advanceExact(child)) {
Expand Down

0 comments on commit a6ebf8b

Please sign in to comment.