From dd4e8b82d711b8f665e91f0d74f159ef1e63939f Mon Sep 17 00:00:00 2001 From: Stefan Vodita <41467371+stefanvodita@users.noreply.github.com> Date: Thu, 7 Jul 2022 17:54:41 +0100 Subject: [PATCH] LUCENE-10603: Stop using SortedSetDocValues.NO_MORE_ORDS in tests (#1004) --- lucene/CHANGES.txt | 4 +- .../BaseLucene80DocValuesFormatTestCase.java | 11 ++--- .../TestBackwardsCompatibility.java | 1 + .../lucene90/TestLucene90DocValuesFormat.java | 9 +--- .../index/TestBinaryDocValuesUpdates.java | 6 ++- .../lucene/index/TestMultiDocValues.java | 29 +++--------- .../index/TestNumericDocValuesUpdates.java | 6 ++- .../lucene/search/TestMinShouldMatch2.java | 3 +- .../lucene/search/join/TestJoinUtil.java | 3 +- .../lucene/index/memory/TestMemoryIndex.java | 1 + .../TestMemoryIndexAgainstDirectory.java | 6 +-- .../index/BaseDocValuesFormatTestCase.java | 44 ++++++++++++------- .../lucene/tests/util/LuceneTestCase.java | 7 ++- 13 files changed, 63 insertions(+), 67 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 3c639f84cc9..b86d3bf1c3c 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -104,8 +104,8 @@ Improvements * LUCENE-10585: Facet module code cleanup (copy/paste scrubbing, simplification and some very minor optimization tweaks). (Greg Miller) -* LUCENE-10603: Update SortedSetDocValues iteration within production code to use - SortedSetDocValues#docValueCount(). (Greg Miller) +* LUCENE-10603: Update SortedSetDocValues iteration to use SortedSetDocValues#docValueCount(). + (Greg Miller, Stefan Vodita) * GITHUB#983: AbstractSortedSetDocValueFacetCounts internal code cleanup/refactoring. (Greg Miller) diff --git a/lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene80/BaseLucene80DocValuesFormatTestCase.java b/lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene80/BaseLucene80DocValuesFormatTestCase.java index b5e5410b9c3..231e2daa17c 100644 --- a/lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene80/BaseLucene80DocValuesFormatTestCase.java +++ b/lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene80/BaseLucene80DocValuesFormatTestCase.java @@ -257,16 +257,12 @@ public abstract class BaseLucene80DocValuesFormatTestCase assertTrue(valueSet.contains(sortedNumeric.nextValue())); } assertEquals(i, sortedSet.nextDoc()); - int sortedSetCount = 0; - while (true) { + + assertEquals(valueSet.size(), sortedSet.docValueCount()); + for (int j = 0; j < sortedSet.docValueCount(); ++j) { long ord = sortedSet.nextOrd(); - if (ord == SortedSetDocValues.NO_MORE_ORDS) { - break; - } assertTrue(valueSet.contains(Long.parseLong(sortedSet.lookupOrd(ord).utf8ToString()))); - sortedSetCount++; } - assertEquals(valueSet.size(), sortedSetCount); } } } @@ -480,6 +476,7 @@ public abstract class BaseLucene80DocValuesFormatTestCase for (int i = 0; i < maxDoc; ++i) { assertEquals(i, values.nextDoc()); final int numValues = in.readVInt(); + assertEquals(numValues, values.docValueCount()); for (int j = 0; j < numValues; ++j) { b.setLength(in.readVInt()); diff --git a/lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java b/lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java index 92a473c34fe..ce6188ccdea 100644 --- a/lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java +++ b/lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java @@ -1205,6 +1205,7 @@ public class TestBackwardsCompatibility extends LuceneTestCase { assertEquals(id, dvShort.longValue()); assertEquals(i, dvSortedSet.nextDoc()); + assertEquals(1, dvSortedSet.docValueCount()); long ord = dvSortedSet.nextOrd(); assertEquals(SortedSetDocValues.NO_MORE_ORDS, dvSortedSet.nextOrd()); term = dvSortedSet.lookupOrd(ord); diff --git a/lucene/core/src/test/org/apache/lucene/codecs/lucene90/TestLucene90DocValuesFormat.java b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/TestLucene90DocValuesFormat.java index ca8fbb14e5b..4acad8330d9 100644 --- a/lucene/core/src/test/org/apache/lucene/codecs/lucene90/TestLucene90DocValuesFormat.java +++ b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/TestLucene90DocValuesFormat.java @@ -265,17 +265,12 @@ public class TestLucene90DocValuesFormat extends BaseCompressingDocValuesFormatT assertTrue(valueSet.contains(sortedNumeric.nextValue())); } assertEquals(i, sortedSet.nextDoc()); + assertEquals(valueSet.size(), sortedSet.docValueCount()); - int sortedSetCount = 0; - while (true) { + for (int j = 0; j < sortedSet.docValueCount(); ++j) { long ord = sortedSet.nextOrd(); - if (ord == SortedSetDocValues.NO_MORE_ORDS) { - break; - } assertTrue(valueSet.contains(Long.parseLong(sortedSet.lookupOrd(ord).utf8ToString()))); - sortedSetCount++; } - assertEquals(valueSet.size(), sortedSetCount); } } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestBinaryDocValuesUpdates.java b/lucene/core/src/test/org/apache/lucene/index/TestBinaryDocValuesUpdates.java index d6d7135bd23..fabb27c82be 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestBinaryDocValuesUpdates.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestBinaryDocValuesUpdates.java @@ -369,12 +369,14 @@ public class TestBinaryDocValuesUpdates extends LuceneTestCase { assertEquals(i, Integer.parseInt(term.utf8ToString())); // For the i=0 case, we added the same value twice, which was dedup'd by IndexWriter so it has // only one value: - if (i != 0) { + if (i == 0) { + assertEquals(1, ssdv.docValueCount()); + } else { + assertEquals(2, ssdv.docValueCount()); ord = ssdv.nextOrd(); term = ssdv.lookupOrd(ord); assertEquals(i * 2, Integer.parseInt(term.utf8ToString())); } - assertEquals(SortedSetDocValues.NO_MORE_ORDS, ssdv.nextOrd()); } reader.close(); diff --git a/lucene/core/src/test/org/apache/lucene/index/TestMultiDocValues.java b/lucene/core/src/test/org/apache/lucene/index/TestMultiDocValues.java index 9672fdac034..e4cb61eca0a 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestMultiDocValues.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestMultiDocValues.java @@ -19,7 +19,6 @@ package org.apache.lucene.index; import static org.apache.lucene.search.DocIdSetIterator.NO_MORE_DOCS; import java.io.IOException; -import java.util.ArrayList; import org.apache.lucene.document.BinaryDocValuesField; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; @@ -279,20 +278,11 @@ public class TestMultiDocValues extends LuceneTestCase { if (docID == NO_MORE_DOCS) { break; } + assertEquals(single.docValueCount(), multi.docValueCount()); - - ArrayList expectedList = new ArrayList<>(); - long ord; - while ((ord = single.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - expectedList.add(ord); + for (int i = 0; i < single.docValueCount(); i++) { + assertEquals(single.nextOrd(), multi.nextOrd()); } - - int upto = 0; - while ((ord = multi.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - assertEquals(expectedList.get(upto).longValue(), ord); - upto++; - } - assertEquals(expectedList.size(), upto); } } testRandomAdvance( @@ -354,18 +344,11 @@ public class TestMultiDocValues extends LuceneTestCase { if (docID == NO_MORE_DOCS) { break; } - ArrayList expectedList = new ArrayList<>(); - long ord; - while ((ord = single.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - expectedList.add(ord); - } - int upto = 0; - while ((ord = multi.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - assertEquals(expectedList.get(upto).longValue(), ord); - upto++; + assertEquals(single.docValueCount(), multi.docValueCount()); + for (int i = 0; i < single.docValueCount(); i++) { + assertEquals(single.nextOrd(), multi.nextOrd()); } - assertEquals(expectedList.size(), upto); } } testRandomAdvance( diff --git a/lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java b/lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java index cbd0338c8e1..0c801fa10c3 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java @@ -459,12 +459,14 @@ public class TestNumericDocValuesUpdates extends LuceneTestCase { long ord = ssdv.nextOrd(); term = ssdv.lookupOrd(ord); assertEquals(i, Integer.parseInt(term.utf8ToString())); - if (i != 0) { + if (i == 0) { + assertEquals(1, ssdv.docValueCount()); + } else { + assertEquals(2, ssdv.docValueCount()); ord = ssdv.nextOrd(); term = ssdv.lookupOrd(ord); assertEquals(i * 2, Integer.parseInt(term.utf8ToString())); } - assertEquals(SortedSetDocValues.NO_MORE_ORDS, ssdv.nextOrd()); } reader.close(); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMinShouldMatch2.java b/lucene/core/src/test/org/apache/lucene/search/TestMinShouldMatch2.java index adde74c7ba0..f3c8893f418 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMinShouldMatch2.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMinShouldMatch2.java @@ -409,7 +409,8 @@ public class TestMinShouldMatch2 extends LuceneTestCase { continue; } long ord; - while ((ord = dv.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { + for (int i = 0; i < dv.docValueCount(); i++) { + ord = dv.nextOrd(); if (ords.contains(ord)) { currentMatched++; score += sims[(int) ord].score(currentDoc, 1); diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java b/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java index 8d97a415409..1567d2df398 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java @@ -1760,7 +1760,8 @@ public class TestJoinUtil extends LuceneTestCase { } if (doc == docTermOrds.docID()) { long ord; - while ((ord = docTermOrds.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { + for (int j = 0; j < docTermOrds.docValueCount(); j++) { + ord = docTermOrds.nextOrd(); final BytesRef joinValue = docTermOrds.lookupOrd(ord); JoinScore joinScore = joinValueToJoinScores.get(joinValue); if (joinScore == null) { diff --git a/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndex.java b/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndex.java index 206c66b2516..7d4152cfc07 100644 --- a/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndex.java +++ b/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndex.java @@ -329,6 +329,7 @@ public class TestMemoryIndex extends LuceneTestCase { assertEquals(3, sortedSetDocValues.getValueCount()); for (int times = 0; times < 3; times++) { assertTrue(sortedSetDocValues.advanceExact(0)); + assertEquals(3, sortedSetDocValues.docValueCount()); assertEquals(0L, sortedSetDocValues.nextOrd()); assertEquals(1L, sortedSetDocValues.nextOrd()); assertEquals(2L, sortedSetDocValues.nextOrd()); diff --git a/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndexAgainstDirectory.java b/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndexAgainstDirectory.java index 21635d2f155..5c4b17a1fe7 100644 --- a/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndexAgainstDirectory.java +++ b/lucene/memory/src/test/org/apache/lucene/index/memory/TestMemoryIndexAgainstDirectory.java @@ -533,14 +533,14 @@ public class TestMemoryIndexAgainstDirectory extends BaseTokenStreamTestCase { controlLeafReader.getSortedSetDocValues("sorted_set"); assertEquals(0, controlSortedSetDocValues.nextDoc()); assertEquals(controlSortedSetDocValues.getValueCount(), sortedSetDocValues.getValueCount()); - for (long controlOrd = controlSortedSetDocValues.nextOrd(); - controlOrd != SortedSetDocValues.NO_MORE_ORDS; - controlOrd = controlSortedSetDocValues.nextOrd()) { + for (int i = 0; i < controlSortedSetDocValues.docValueCount(); i++) { + long controlOrd = controlSortedSetDocValues.nextOrd(); assertEquals(controlOrd, sortedSetDocValues.nextOrd()); assertEquals( controlSortedSetDocValues.lookupOrd(controlOrd), sortedSetDocValues.lookupOrd(controlOrd)); } + assertEquals(SortedSetDocValues.NO_MORE_ORDS, controlSortedSetDocValues.nextOrd()); assertEquals(SortedSetDocValues.NO_MORE_ORDS, sortedSetDocValues.nextOrd()); indexReader.close(); diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseDocValuesFormatTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseDocValuesFormatTestCase.java index 563c32326b2..2c2aa59148d 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseDocValuesFormatTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseDocValuesFormatTestCase.java @@ -1809,6 +1809,8 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(0, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1834,6 +1836,7 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(0, dv.nextDoc()); + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1841,8 +1844,9 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes assertEquals(newBytesRef("hello"), bytes); dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field2"); - assertEquals(0, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1877,6 +1881,7 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes assertEquals(2, dv.getValueCount()); assertEquals(0, dv.nextDoc()); + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1884,6 +1889,7 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes assertEquals(newBytesRef("hello"), bytes); assertEquals(1, dv.nextDoc()); + assertEquals(1, dv.docValueCount()); assertEquals(1, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1909,6 +1915,7 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(0, dv.nextDoc()); + assertEquals(2, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(1, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1938,6 +1945,7 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(0, dv.nextDoc()); + assertEquals(2, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(1, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -1978,11 +1986,13 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes assertEquals(3, dv.getValueCount()); assertEquals(0, dv.nextDoc()); + assertEquals(2, dv.docValueCount()); assertEquals(1, dv.nextOrd()); assertEquals(2, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); assertEquals(1, dv.nextDoc()); + assertEquals(2, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(1, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -2019,8 +2029,9 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(1, dv.getValueCount()); - assertEquals(0, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -2052,8 +2063,9 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(1, dv.getValueCount()); - assertEquals(0, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -2084,8 +2096,9 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(1, dv.getValueCount()); - assertEquals(1, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -2117,8 +2130,9 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes SortedSetDocValues dv = getOnlyLeafReader(ireader).getSortedSetDocValues("field"); assertEquals(1, dv.getValueCount()); - assertEquals(1, dv.nextDoc()); + + assertEquals(1, dv.docValueCount()); assertEquals(0, dv.nextOrd()); assertEquals(NO_MORE_ORDS, dv.nextOrd()); @@ -2320,12 +2334,12 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes } if (docValues != null && stringValues.length > 0) { assertEquals(i, docValues.docID()); - for (int j = 0; j < stringValues.length; j++) { + assertEquals(stringValues.length, docValues.docValueCount()); + for (String stringValue : stringValues) { assert docValues != null; long ord = docValues.nextOrd(); - assert ord != NO_MORE_ORDS; BytesRef scratch = docValues.lookupOrd(ord); - assertEquals(stringValues[j], scratch.utf8ToString()); + assertEquals(stringValue, scratch.utf8ToString()); } assertEquals(NO_MORE_ORDS, docValues.nextOrd()); } @@ -2353,12 +2367,12 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes } if (stringValues.length > 0) { assertEquals(i, docValues.docID()); - for (int j = 0; j < stringValues.length; j++) { + assertEquals(stringValues.length, docValues.docValueCount()); + for (String stringValue : stringValues) { assert docValues != null; long ord = docValues.nextOrd(); - assert ord != NO_MORE_ORDS; BytesRef scratch = docValues.lookupOrd(ord); - assertEquals(stringValues[j], scratch.utf8ToString()); + assertEquals(stringValue, scratch.utf8ToString()); } assertEquals(NO_MORE_ORDS, docValues.nextOrd()); } @@ -2891,13 +2905,13 @@ public abstract class BaseDocValuesFormatTestCase extends BaseIndexFileFormatTes if (values.length > 0) { assertNotNull(sortedSet); assertEquals(j, sortedSet.nextDoc()); - for (int k = 0; k < values.length; k++) { + assertEquals(values.length, sortedSet.docValueCount()); + for (String s : values) { long ord = sortedSet.nextOrd(); - assertTrue(ord != SortedSetDocValues.NO_MORE_ORDS); BytesRef value = sortedSet.lookupOrd(ord); - assertEquals(values[k], value.utf8ToString()); + assertEquals(s, value.utf8ToString()); } - assertEquals(SortedSetDocValues.NO_MORE_ORDS, sortedSet.nextOrd()); + assertEquals(NO_MORE_ORDS, sortedSet.nextOrd()); } String[] numValues = r.document(j).getValues("storedSortedNumeric"); diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCase.java index 2c1a2ebc527..9da1775ee8a 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCase.java @@ -2586,11 +2586,10 @@ public abstract class LuceneTestCase extends Assert { if (docID == NO_MORE_DOCS) { break; } - long ord; - while ((ord = leftValues.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - assertEquals(info, ord, rightValues.nextOrd()); + assertEquals(info, leftValues.docValueCount(), rightValues.docValueCount()); + for (int i = 0; i < leftValues.docValueCount(); i++) { + assertEquals(info, leftValues.nextOrd(), rightValues.nextOrd()); } - assertEquals(info, SortedSetDocValues.NO_MORE_ORDS, rightValues.nextOrd()); } } else { assertNull(info, leftValues);