Skip to content

Commit 1d4f702

Browse files
keljpountz
kel
authored and
jpountz
committed
Calculate and cache result when advanceExact is called (#26920)
Cache final result instead of result of advanceExact. Fix SortedNumericDoubleValues does not test MEDIAN mode Replace deprecated random string generation method
1 parent 19dc629 commit 1d4f702

File tree

2 files changed

+66
-29
lines changed

2 files changed

+66
-29
lines changed

‎core/src/main/java/org/elasticsearch/search/MultiValueMode.java‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -416,11 +416,11 @@ public NumericDocValues select(final SortedNumericDocValues values, final long m
416416
if (singleton != null) {
417417
return new AbstractNumericDocValues() {
418418

419-
private boolean hasValue;
419+
private long value;
420420

421421
@Override
422422
public boolean advanceExact(int target) throws IOException {
423-
hasValue = singleton.advanceExact(target);
423+
this.value = singleton.advanceExact(target) ? singleton.longValue() : missingValue;
424424
return true;
425425
}
426426

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

432432
@Override
433433
public long longValue() throws IOException {
434-
return hasValue ? singleton.longValue() : missingValue;
434+
return this.value;
435435
}
436436
};
437437
} else {
438438
return new AbstractNumericDocValues() {
439439

440-
private boolean hasValue;
440+
private long value;
441441

442442
@Override
443443
public boolean advanceExact(int target) throws IOException {
444-
hasValue = values.advanceExact(target);
444+
this.value = values.advanceExact(target) ? pick(values) : missingValue;
445445
return true;
446446
}
447447

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

453453
@Override
454454
public long longValue() throws IOException {
455-
return hasValue ? pick(values) : missingValue;
455+
return value;
456456
}
457457
};
458458
}
@@ -533,35 +533,33 @@ public NumericDoubleValues select(final SortedNumericDoubleValues values, final
533533
final NumericDoubleValues singleton = FieldData.unwrapSingleton(values);
534534
if (singleton != null) {
535535
return new NumericDoubleValues() {
536-
537-
private boolean hasValue;
536+
private double value;
538537

539538
@Override
540539
public boolean advanceExact(int doc) throws IOException {
541-
hasValue = singleton.advanceExact(doc);
540+
this.value = singleton.advanceExact(doc) ? singleton.doubleValue() : missingValue;
542541
return true;
543542
}
544543

545544
@Override
546545
public double doubleValue() throws IOException {
547-
return hasValue ? singleton.doubleValue() : missingValue;
546+
return this.value;
548547
}
549-
550548
};
551549
} else {
552550
return new NumericDoubleValues() {
553551

554-
private boolean hasValue;
552+
private double value;
555553

556554
@Override
557555
public boolean advanceExact(int target) throws IOException {
558-
hasValue = values.advanceExact(target);
556+
value = values.advanceExact(target) ? pick(values) : missingValue;
559557
return true;
560558
}
561559

562560
@Override
563561
public double doubleValue() throws IOException {
564-
return hasValue ? pick(values) : missingValue;
562+
return this.value;
565563
}
566564
};
567565
}
@@ -638,17 +636,17 @@ public BinaryDocValues select(final SortedBinaryDocValues values, final BytesRef
638636
}
639637
return new AbstractBinaryDocValues() {
640638

641-
private boolean hasValue;
639+
private BytesRef value;
642640

643641
@Override
644642
public boolean advanceExact(int target) throws IOException {
645-
hasValue = singleton.advanceExact(target);
643+
this.value = singleton.advanceExact(target) ? singleton.binaryValue() : missingValue;
646644
return true;
647645
}
648646

649647
@Override
650648
public BytesRef binaryValue() throws IOException {
651-
return hasValue ? singleton.binaryValue() : missingValue;
649+
return this.value;
652650
}
653651
};
654652
} else {

‎core/src/test/java/org/elasticsearch/search/MultiValueModeTests.java‎

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
package org.elasticsearch.search;
2121

22-
import com.carrotsearch.randomizedtesting.generators.RandomStrings;
23-
2422
import org.apache.lucene.index.BinaryDocValues;
2523
import org.apache.lucene.index.DocValues;
2624
import org.apache.lucene.index.NumericDocValues;
@@ -160,6 +158,8 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
160158
for (int i = 0; i < maxDoc; ++i) {
161159
assertTrue(selected.advanceExact(i));
162160
final long actual = selected.longValue();
161+
verifyLongValueCanCalledMoreThanOnce(selected, actual);
162+
163163
long expected = 0;
164164
if (values.advanceExact(i) == false) {
165165
expected = missingValue;
@@ -203,6 +203,12 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
203203
}
204204
}
205205

206+
private void verifyLongValueCanCalledMoreThanOnce(NumericDocValues values, long expected) throws IOException {
207+
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
208+
assertEquals(expected, values.longValue());
209+
}
210+
}
211+
206212
private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
207213
for (long missingValue : new long[] { 0, randomLong() }) {
208214
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX, MultiValueMode.SUM, MultiValueMode.AVG}) {
@@ -212,6 +218,8 @@ private void verifySortedNumeric(Supplier<SortedNumericDocValues> supplier, int
212218
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
213219
assertTrue(selected.advanceExact(root));
214220
final long actual = selected.longValue();
221+
verifyLongValueCanCalledMoreThanOnce(selected, actual);
222+
215223
long expected = 0;
216224
if (mode == MultiValueMode.MAX) {
217225
expected = Long.MIN_VALUE;
@@ -320,14 +328,13 @@ public int docValueCount() {
320328
private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> supplier, int maxDoc) throws IOException {
321329
for (long missingValue : new long[] { 0, randomLong() }) {
322330
for (MultiValueMode mode : MultiValueMode.values()) {
323-
if (MultiValueMode.MEDIAN.equals(mode)) {
324-
continue;
325-
}
326331
SortedNumericDoubleValues values = supplier.get();
327332
final NumericDoubleValues selected = mode.select(values, missingValue);
328333
for (int i = 0; i < maxDoc; ++i) {
329334
assertTrue(selected.advanceExact(i));
330335
final double actual = selected.doubleValue();
336+
verifyDoubleValueCanCalledMoreThanOnce(selected, actual);
337+
331338
double expected = 0.0;
332339
if (values.advanceExact(i) == false) {
333340
expected = missingValue;
@@ -371,6 +378,12 @@ private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> suppl
371378
}
372379
}
373380

381+
private void verifyDoubleValueCanCalledMoreThanOnce(NumericDoubleValues values, double expected) throws IOException {
382+
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
383+
assertTrue(Double.compare(values.doubleValue(), expected) == 0);
384+
}
385+
}
386+
374387
private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
375388
for (long missingValue : new long[] { 0, randomLong() }) {
376389
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX, MultiValueMode.SUM, MultiValueMode.AVG}) {
@@ -379,7 +392,9 @@ private void verifySortedNumericDouble(Supplier<SortedNumericDoubleValues> suppl
379392
int prevRoot = -1;
380393
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
381394
assertTrue(selected.advanceExact(root));
382-
final double actual = selected.doubleValue();;
395+
final double actual = selected.doubleValue();
396+
verifyDoubleValueCanCalledMoreThanOnce(selected, actual);
397+
383398
double expected = 0.0;
384399
if (mode == MultiValueMode.MAX) {
385400
expected = Long.MIN_VALUE;
@@ -421,7 +436,7 @@ public void testSingleValuedStrings() throws Exception {
421436
final FixedBitSet docsWithValue = randomBoolean() ? null : new FixedBitSet(numDocs);
422437
for (int i = 0; i < array.length; ++i) {
423438
if (randomBoolean()) {
424-
array[i] = new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8));
439+
array[i] = new BytesRef(randomAlphaOfLengthBetween(8, 8));
425440
if (docsWithValue != null) {
426441
docsWithValue.set(i);
427442
}
@@ -456,7 +471,7 @@ public void testMultiValuedStrings() throws Exception {
456471
for (int i = 0; i < numDocs; ++i) {
457472
final BytesRef[] values = new BytesRef[randomInt(4)];
458473
for (int j = 0; j < values.length; ++j) {
459-
values[j] = new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8));
474+
values[j] = new BytesRef(randomAlphaOfLengthBetween(8, 8));
460475
}
461476
Arrays.sort(values);
462477
array[i] = values;
@@ -489,13 +504,15 @@ public int docValueCount() {
489504
}
490505

491506
private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int maxDoc) throws IOException {
492-
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8)) }) {
507+
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(randomAlphaOfLengthBetween(8, 8)) }) {
493508
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
494509
SortedBinaryDocValues values = supplier.get();
495510
final BinaryDocValues selected = mode.select(values, missingValue);
496511
for (int i = 0; i < maxDoc; ++i) {
497512
assertTrue(selected.advanceExact(i));
498513
final BytesRef actual = selected.binaryValue();
514+
verifyBinaryValueCanCalledMoreThanOnce(selected, actual);
515+
499516
BytesRef expected = null;
500517
if (values.advanceExact(i) == false) {
501518
expected = missingValue;
@@ -524,15 +541,23 @@ private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int ma
524541
}
525542
}
526543

544+
private void verifyBinaryValueCanCalledMoreThanOnce(BinaryDocValues values, BytesRef expected) throws IOException {
545+
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
546+
assertEquals(values.binaryValue(), expected);
547+
}
548+
}
549+
527550
private void verifySortedBinary(Supplier<SortedBinaryDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
528-
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(RandomStrings.randomAsciiOfLength(random(), 8)) }) {
551+
for (BytesRef missingValue : new BytesRef[] { new BytesRef(), new BytesRef(randomAlphaOfLengthBetween(8, 8)) }) {
529552
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
530553
SortedBinaryDocValues values = supplier.get();
531554
final BinaryDocValues selected = mode.select(values, missingValue, rootDocs, new BitSetIterator(innerDocs, 0L), maxDoc);
532555
int prevRoot = -1;
533556
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
534557
assertTrue(selected.advanceExact(root));
535558
final BytesRef actual = selected.binaryValue();
559+
verifyBinaryValueCanCalledMoreThanOnce(selected, actual);
560+
536561
BytesRef expected = null;
537562
for (int child = innerDocs.nextSetBit(prevRoot + 1); child != -1 && child < root; child = innerDocs.nextSetBit(child + 1)) {
538563
if (values.advanceExact(child)) {
@@ -658,7 +683,11 @@ private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc)
658683
SortedSetDocValues values = supplier.get();
659684
final SortedDocValues selected = mode.select(values);
660685
for (int i = 0; i < maxDoc; ++i) {
661-
final long actual = selected.advanceExact(i) ? selected.ordValue() : -1;
686+
long actual = -1;
687+
if (selected.advanceExact(i)) {
688+
actual = selected.ordValue();
689+
verifyOrdValueCanCalledMoreThanOnce(selected, selected.ordValue());
690+
}
662691
int expected = -1;
663692
if (values.advanceExact(i)) {
664693
for (long ord = values.nextOrd(); ord != SortedSetDocValues.NO_MORE_ORDS; ord = values.nextOrd()) {
@@ -679,13 +708,23 @@ private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc)
679708
}
680709
}
681710

711+
private void verifyOrdValueCanCalledMoreThanOnce(SortedDocValues values, long expected) throws IOException {
712+
for (int j = 0, numCall = randomIntBetween(1, 10); j < numCall; j++) {
713+
assertEquals(values.ordValue(), expected);
714+
}
715+
}
716+
682717
private void verifySortedSet(Supplier<SortedSetDocValues> supplier, int maxDoc, FixedBitSet rootDocs, FixedBitSet innerDocs) throws IOException {
683718
for (MultiValueMode mode : new MultiValueMode[] {MultiValueMode.MIN, MultiValueMode.MAX}) {
684719
SortedSetDocValues values = supplier.get();
685720
final SortedDocValues selected = mode.select(values, rootDocs, new BitSetIterator(innerDocs, 0L));
686721
int prevRoot = -1;
687722
for (int root = rootDocs.nextSetBit(0); root != -1; root = root + 1 < maxDoc ? rootDocs.nextSetBit(root + 1) : -1) {
688-
final int actual = selected.advanceExact(root) ? selected.ordValue() : -1;
723+
int actual = -1;
724+
if (selected.advanceExact(root)) {
725+
actual = selected.ordValue();
726+
verifyOrdValueCanCalledMoreThanOnce(selected, actual);
727+
}
689728
int expected = -1;
690729
for (int child = innerDocs.nextSetBit(prevRoot + 1); child != -1 && child < root; child = innerDocs.nextSetBit(child + 1)) {
691730
if (values.advanceExact(child)) {

0 commit comments

Comments
 (0)