From 79976874d22dc3c815e8e95d70171e06d727674a Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:43:09 +0100 Subject: [PATCH 1/7] Use the doc-values skip index to skip per-doc value lookups in LongRangeFacetCutter --- lucene/CHANGES.txt | 2 + .../cutters/ranges/LongRangeFacetCutter.java | 138 ++++++++++++++- .../NonOverlappingLongRangeFacetCutter.java | 18 +- .../OverlappingLongRangeFacetCutter.java | 25 ++- .../facet/utils/RangeFacetBuilderFactory.java | 5 +- .../lucene/sandbox/facet/TestRangeFacet.java | 163 ++++++++++++++++++ 6 files changed, 340 insertions(+), 11 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 1d1d5a8154a7..13f126ce00bd 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -606,6 +606,8 @@ Optimizations * GITHUB#16228: Reuse scratch int[] for ordinal translation. (Tim Brooks) +* GITHUB#16268: Use the doc-values skip index to skip per-doc value lookups for dense blocks in LongRangeFacetCutter. (Jakub Slowinski) + Bug Fixes --------------------- * GITHUB#15754: Fix HTMLStripCharFilter to prevent tags from incorrectly consuming subsequent diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index b9518bfca154..14a7942e54db 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -23,6 +23,11 @@ import org.apache.lucene.facet.MultiLongValues; import org.apache.lucene.facet.MultiLongValuesSource; import org.apache.lucene.facet.range.LongRange; +import org.apache.lucene.index.DocValues; +import org.apache.lucene.index.DocValuesSkipper; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.sandbox.facet.cutters.FacetCutter; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -42,6 +47,10 @@ public abstract class LongRangeFacetCutter implements FacetCutter { // TODO: refactor - weird that we have both multi and single here. final LongValuesSource singleValues; + + // Field to read a DocValuesSkipper from on the single-valued path, or null when disabled. + final String skipField; + final LongRangeAndPos[] sortedRanges; final int requestedRangeCount; @@ -62,17 +71,34 @@ static LongRangeFacetCutter createSingleOrMultiValued( MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, LongRange[] longRanges) { + return createSingleOrMultiValued(longValuesSource, singleLongValuesSource, longRanges, null); + } + + /** Same as above, but uses the {@code skipField} skip index on the single-valued path. */ + static LongRangeFacetCutter createSingleOrMultiValued( + MultiLongValuesSource longValuesSource, + LongValuesSource singleLongValuesSource, + LongRange[] longRanges, + String skipField) { if (areOverlappingRanges(longRanges)) { return new OverlappingLongRangeFacetCutter( - longValuesSource, singleLongValuesSource, longRanges); + longValuesSource, singleLongValuesSource, longRanges, skipField); } return new NonOverlappingLongRangeFacetCutter( - longValuesSource, singleLongValuesSource, longRanges); + longValuesSource, singleLongValuesSource, longRanges, skipField); } public static LongRangeFacetCutter create( MultiLongValuesSource longValuesSource, LongRange[] longRanges) { - return createSingleOrMultiValued(longValuesSource, null, longRanges); + return createSingleOrMultiValued(longValuesSource, null, longRanges, null); + } + + /** Create {@link FacetCutter} for a long field by name, using its skip index when present. */ + public static LongRangeFacetCutter create(String field, LongRange[] longRanges) { + // Leave the single-valued source null. The skip path reads the field directly, and a + // multi-valued segment must fall back to the multi-valued leaf cutter. + return createSingleOrMultiValued( + MultiLongValuesSource.fromLongField(field), null, longRanges, field); } // caller handles conversion of Doubles and DoubleRange to Long and LongRange @@ -80,7 +106,8 @@ public static LongRangeFacetCutter create( LongRangeFacetCutter( MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, - LongRange[] longRanges) { + LongRange[] longRanges, + String skipField) { super(); valuesSource = longValuesSource; if (singleLongValuesSource != null) { @@ -88,6 +115,7 @@ public static LongRangeFacetCutter create( } else { singleValues = MultiLongValuesSource.unwrapSingleton(valuesSource); } + this.skipField = skipField; sortedRanges = new LongRangeAndPos[longRanges.length]; requestedRangeCount = longRanges.length; @@ -124,6 +152,39 @@ public static LongRangeFacetCutter create( */ abstract List buildElementaryIntervals(); + /** + * Returns the {@link DocValuesSkipper} for {@link #skipField} in this segment. Null when: no skip + * field is configured, the field has no skip index, or some doc in this segment has more than one + * value. + */ + final DocValuesSkipper maybeSkipper(LeafReaderContext context) throws IOException { + if (skipField == null) { + return null; + } + SortedNumericDocValues sortedNumeric = DocValues.getSortedNumeric(context.reader(), skipField); + if (DocValues.unwrapSingleton(sortedNumeric) == null) { + return null; + } + return context.reader().getDocValuesSkipper(skipField); + } + + /** Single-valued {@link LongValues} for {@link #skipField} in this segment. */ + final LongValues skipFieldValues(LeafReaderContext context) throws IOException { + NumericDocValues values = + DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), skipField)); + return new LongValues() { + @Override + public long longValue() throws IOException { + return values.longValue(); + } + + @Override + public boolean advanceExact(int doc) throws IOException { + return values.advanceExact(doc); + } + }; + } + private static boolean areOverlappingRanges(LongRange[] ranges) { if (ranges.length == 0) { return false; @@ -252,21 +313,52 @@ abstract static class LongRangeSingleValuedLeafFacetCutter implements LeafFacetC IntervalTracker requestedIntervalTracker; + // Skip index for the faceted field, or null when disabled. + private final DocValuesSkipper skipper; + + // Cached decision from advanceSkipper, valid for every doc up to (and including) upToInclusive: + // when upToSameInterval is true, all those docs map to elementary interval upToIntervalOrd. + private int upToInclusive = -1; + private boolean upToSameInterval; + private int upToIntervalOrd; + LongRangeSingleValuedLeafFacetCutter(LongValues longValues, long[] boundaries, int[] pos) { + this(longValues, boundaries, pos, null); + } + + LongRangeSingleValuedLeafFacetCutter( + LongValues longValues, long[] boundaries, int[] pos, DocValuesSkipper skipper) { this.longValues = longValues; this.boundaries = boundaries; this.pos = pos; + this.skipper = skipper; + // The skip path counts a dense block as one value per doc, so it's single-valued only. + assert skipper == null || skipper.maxValueCount() <= 1 + : "skip-index fast path requires a single-valued field, got maxValueCount=" + + skipper.maxValueCount(); } @Override public boolean advanceExact(int doc) throws IOException { - if (longValues.advanceExact(doc) == false) { + if (skipper != null && doc > upToInclusive) { + advanceSkipper(doc); + } + + int intervalOrd; + if (upToSameInterval) { + // We are inside a dense skip block that maps entirely to one elementary interval, so reuse + // the cached ordinal and skip the per-doc value lookup and binary search. + intervalOrd = upToIntervalOrd; + } else if (longValues.advanceExact(doc)) { + intervalOrd = processValue(longValues.longValue()); + } else { return false; } + if (requestedIntervalTracker != null) { requestedIntervalTracker.clear(); } - elementaryIntervalOrd = processValue(longValues.longValue()); + elementaryIntervalOrd = intervalOrd; maybeRollUp(requestedIntervalTracker); if (requestedIntervalTracker != null) { requestedIntervalTracker.freeze(); @@ -275,6 +367,40 @@ public boolean advanceExact(int doc) throws IOException { return true; } + /** Mirrors {@code HistogramCollector#advanceSkipper}. */ + private void advanceSkipper(int doc) throws IOException { + if (doc > skipper.maxDocID(0)) { + skipper.advance(doc); + } + upToSameInterval = false; + + if (skipper.minDocID(0) > doc) { + // Corner case which happens if doc doesn't have a value and is between two intervals of the + // skip index. Fall back to per-doc lookups until the next block. + upToInclusive = skipper.minDocID(0) - 1; + return; + } + + upToInclusive = skipper.maxDocID(0); + // Now find the highest level where all docs have a value and map to the same interval. + for (int level = 0; level < skipper.numLevels(); ++level) { + int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; + if (skipper.docCount(level) != totalDocsAtLevel) { + // Some docs at this level have no value, so we can't resolve the whole block at once. + break; + } + // Long fields store raw values, the skipper's min/max map straight into the boundary space. + int minInterval = processValue(skipper.minValue(level)); + int maxInterval = processValue(skipper.maxValue(level)); + if (minInterval != maxInterval) { + break; + } + upToInclusive = skipper.maxDocID(level); + upToSameInterval = true; + upToIntervalOrd = minInterval; + } + } + // Returns the value of the interval v belongs or lastIntervalSeen // if no processing is done, it returns the lastIntervalSeen private int processValue(long v) { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java index 3d657a96570d..4bb68dea11b1 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java @@ -22,6 +22,7 @@ import org.apache.lucene.facet.MultiLongValues; import org.apache.lucene.facet.MultiLongValuesSource; import org.apache.lucene.facet.range.LongRange; +import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -32,8 +33,9 @@ class NonOverlappingLongRangeFacetCutter extends LongRangeFacetCutter { NonOverlappingLongRangeFacetCutter( MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, - LongRange[] longRanges) { - super(longValuesSource, singleLongValuesSource, longRanges); + LongRange[] longRanges, + String skipField) { + super(longValuesSource, singleLongValuesSource, longRanges, skipField); } /** @@ -68,6 +70,13 @@ List buildElementaryIntervals() { @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { + // Use the skip index when we can, otherwise fall back to the value source. + DocValuesSkipper skipper = maybeSkipper(context); + if (skipper != null) { + LongValues values = skipFieldValues(context); + return new NonOverlappingLongRangeSingleValueLeafFacetCutter( + values, boundaries, pos, skipper); + } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); return new NonOverlappingLongRangeSingleValueLeafFacetCutter(values, boundaries, pos); @@ -112,6 +121,11 @@ static class NonOverlappingLongRangeSingleValueLeafFacetCutter super(longValues, boundaries, pos); } + NonOverlappingLongRangeSingleValueLeafFacetCutter( + LongValues longValues, long[] boundaries, int[] pos, DocValuesSkipper skipper) { + super(longValues, boundaries, pos, skipper); + } + @Override public int nextOrd() throws IOException { if (elementaryIntervalOrd == NO_MORE_ORDS) { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java index 58586db892f7..2eff548329ae 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java @@ -25,6 +25,7 @@ import org.apache.lucene.facet.MultiLongValues; import org.apache.lucene.facet.MultiLongValuesSource; import org.apache.lucene.facet.range.LongRange; +import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.internal.hppc.IntCursor; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; @@ -43,8 +44,9 @@ class OverlappingLongRangeFacetCutter extends LongRangeFacetCutter { OverlappingLongRangeFacetCutter( MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, - LongRange[] longRanges) { - super(longValuesSource, singleLongValuesSource, longRanges); + LongRange[] longRanges, + String skipField) { + super(longValuesSource, singleLongValuesSource, longRanges, skipField); // Build binary tree on top of intervals: root = split(0, elementaryIntervals.size(), elementaryIntervals); @@ -147,6 +149,13 @@ private static LongRangeNode split(int start, int end, List elem @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { + // Use the skip index when we can, otherwise fall back to the value source. + DocValuesSkipper skipper = maybeSkipper(context); + if (skipper != null) { + LongValues values = skipFieldValues(context); + return new OverlappingSingleValuedRangeLeafFacetCutter( + values, boundaries, pos, requestedRangeCount, root, skipper); + } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); return new OverlappingSingleValuedRangeLeafFacetCutter( @@ -233,6 +242,18 @@ static class OverlappingSingleValuedRangeLeafFacetCutter this.elementaryIntervalRoot = elementaryIntervalRoot; } + OverlappingSingleValuedRangeLeafFacetCutter( + LongValues longValues, + long[] boundaries, + int[] pos, + int requestedRangeCount, + LongRangeNode elementaryIntervalRoot, + DocValuesSkipper skipper) { + super(longValues, boundaries, pos, skipper); + requestedIntervalTracker = new IntervalTracker.MultiIntervalTracker(requestedRangeCount); + this.elementaryIntervalRoot = elementaryIntervalRoot; + } + @Override void maybeRollUp(IntervalTracker rollUpInto) { // TODO: for single valued we can rollup after collecting all documents, e.g. in reduce diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java index 8d69acfdc336..4caccc4b07e4 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java @@ -35,7 +35,10 @@ private RangeFacetBuilderFactory() {} /** Request long range facets for numeric field by name. */ public static CommonFacetBuilder forLongRanges(String field, LongRange... ranges) { - return forLongRanges(field, MultiLongValuesSource.fromLongField(field), ranges); + // Pass the field by name so we can use its skip index when present. + return new CommonFacetBuilder( + field, LongRangeFacetCutter.create(field, ranges), new RangeOrdToLabel(ranges)) + .withSortByOrdinal(); } /** diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java index 739f77fe37c5..7deb908cb93d 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java @@ -21,6 +21,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomNumbers; import java.io.IOException; import java.util.List; +import org.apache.lucene.codecs.lucene90.Lucene90DocValuesFormat; import org.apache.lucene.document.Document; import org.apache.lucene.document.DoubleDocValuesField; import org.apache.lucene.document.DoublePoint; @@ -57,6 +58,8 @@ import org.apache.lucene.search.LongValuesSource; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MultiCollectorManager; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.tests.search.DummyTotalHitCountCollector; @@ -893,6 +896,166 @@ public void testRandomLongsSingleValued() throws Exception { IOUtils.close(r, dir); } + public void testSkipIndexEquivalenceLong() throws Exception { + // mode 1 sorts by the field and mode 2 also shrinks the skip interval, so the blocks are dense + // enough for the fast path to fire. + for (int mode = 0; mode < 3; mode++) { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + if (mode >= 1) { + iwc.setIndexSort(new Sort(new SortField("field", SortField.Type.LONG))); + } + if (mode == 2) { + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + } + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + long v = TestUtil.nextLong(random(), -100, 100); + doc.add(NumericDocValuesField.indexedField("field", v)); + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "mode=" + mode); + + w.close(); + IOUtils.close(dir); + } + } + + public void testSkipIndexEquivalenceExtremeValues() throws Exception { + // Index sorted with extreme values mixed in, so some skip blocks carry Long.MIN/MAX_VALUE as + // their min/max bounds and advanceSkipper's processValue is exercised on those bounds. + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setIndexSort(new Sort(new SortField("field", SortField.Type.LONG))); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + long v = + switch (random().nextInt(4)) { + case 0 -> Long.MIN_VALUE; + case 1 -> Long.MAX_VALUE; + default -> TestUtil.nextLong(random(), -100, 100); + }; + doc.add(NumericDocValuesField.indexedField("field", v)); + doc.add(new LongPoint("point", v)); + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "extreme"); + + w.close(); + IOUtils.close(dir); + } + + public void testSkipIndexEquivalenceSparse() throws Exception { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setIndexSort(new Sort(new SortField("field", SortField.Type.LONG, false))); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + // Leave roughly a third of the docs without a value so skip blocks aren't dense. + if (random().nextInt(3) != 0) { + doc.add( + NumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), -100, 100))); + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "sparse"); + + w.close(); + IOUtils.close(dir); + } + + public void testSkipIndexEquivalenceMultiValued() throws Exception { + Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir); + + int numDocs = atLeast(500); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + int numVals = TestUtil.nextInt(random(), 1, 5); + for (int j = 0; j < numVals; j++) { + doc.add(new SortedNumericDocValuesField("field", TestUtil.nextLong(random(), -100, 100))); + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "multi-valued"); + + w.close(); + IOUtils.close(dir); + } + + // Asserts faceting "field" by name (skip index when available) matches faceting via a + // MultiLongValuesSource (no skip index), over random range sets including extreme bounds. + private void assertSkipIndexEquivalence(RandomIndexWriter w, String desc) throws IOException { + IndexReader r = w.getReader(); + try { + IndexSearcher s = newSearcher(r, false); + + int numIters = atLeast(10); + for (int iter = 0; iter < numIters; iter++) { + int numRange = TestUtil.nextInt(random(), 0, 20); + LongRange[] ranges = new LongRange[numRange]; + for (int rangeID = 0; rangeID < numRange; rangeID++) { + long min; + long max; + if (random().nextInt(20) == 0) { + // Occasionally use extreme bounds to exercise the boundary edges of processValue. + min = random().nextBoolean() ? Long.MIN_VALUE : TestUtil.nextLong(random(), -120, 120); + max = random().nextBoolean() ? Long.MAX_VALUE : TestUtil.nextLong(random(), -120, 120); + } else { + min = TestUtil.nextLong(random(), -120, 120); + max = TestUtil.nextLong(random(), -120, 120); + } + if (min > max) { + long x = min; + min = max; + max = x; + } + ranges[rangeID] = new LongRange("r" + rangeID, min, true, max, true); + } + OrdToLabel ordToLabel = new RangeOrdToLabel(ranges); + + // value-source path, no skipper. + CountFacetRecorder baselineRecorder = new CountFacetRecorder(); + s.search( + MatchAllDocsQuery.INSTANCE, + new FacetFieldCollectorManager<>( + LongRangeFacetCutter.create(MultiLongValuesSource.fromLongField("field"), ranges), + baselineRecorder)); + String baseline = + getAllSortByOrd(getRangeOrdinals(ranges), baselineRecorder, "field", ordToLabel) + .toString(); + + // by-field cutter, uses the skip index. + CountFacetRecorder skipRecorder = new CountFacetRecorder(); + s.search( + MatchAllDocsQuery.INSTANCE, + new FacetFieldCollectorManager<>( + LongRangeFacetCutter.create("field", ranges), skipRecorder)); + String withSkip = + getAllSortByOrd(getRangeOrdinals(ranges), skipRecorder, "field", ordToLabel).toString(); + + assertEquals(desc + " iter=" + iter, baseline, withSkip); + } + } finally { + IOUtils.close(r); + } + } + public void testRandomLongsMultiValued() throws Exception { Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir); From 316407925750b90e049767401a2adaf7384d101f Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:03:43 +0100 Subject: [PATCH 2/7] Remove redundant assertion --- .../sandbox/facet/cutters/ranges/LongRangeFacetCutter.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index 14a7942e54db..1720ca502deb 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -332,10 +332,6 @@ abstract static class LongRangeSingleValuedLeafFacetCutter implements LeafFacetC this.boundaries = boundaries; this.pos = pos; this.skipper = skipper; - // The skip path counts a dense block as one value per doc, so it's single-valued only. - assert skipper == null || skipper.maxValueCount() <= 1 - : "skip-index fast path requires a single-valued field, got maxValueCount=" - + skipper.maxValueCount(); } @Override From 5587f585e1657489529e0b538412b3ab86af7251 Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Tue, 23 Jun 2026 08:26:06 +0100 Subject: [PATCH 3/7] Extend the skip-index fast path to non-dense blocks and reuse the interval tracker --- lucene/CHANGES.txt | 4 +- .../facet/cutters/ranges/IntervalTracker.java | 11 ++++ .../cutters/ranges/LongRangeFacetCutter.java | 66 +++++++++---------- .../NonOverlappingLongRangeFacetCutter.java | 9 ++- .../OverlappingLongRangeFacetCutter.java | 9 ++- .../facet/utils/RangeFacetBuilderFactory.java | 1 - .../lucene/sandbox/facet/TestRangeFacet.java | 40 ++++++++++- 7 files changed, 91 insertions(+), 49 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 13f126ce00bd..fe6af506c792 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -323,6 +323,8 @@ Optimizations * GITHUB#16283: Use Panama Vector API to SIMD-evaluate fixed-cardinality sorted numeric range queries in rangeIntoBitSet. (Costin Leau) +* GITHUB#16268: Use the doc-values skip index to skip per-doc value lookups in LongRangeFacetCutter. (Jakub Slowinski) + Bug Fixes --------------------- @@ -606,8 +608,6 @@ Optimizations * GITHUB#16228: Reuse scratch int[] for ordinal translation. (Tim Brooks) -* GITHUB#16268: Use the doc-values skip index to skip per-doc value lookups for dense blocks in LongRangeFacetCutter. (Jakub Slowinski) - Bug Fixes --------------------- * GITHUB#15754: Fix HTMLStripCharFilter to prevent tags from incorrectly consuming subsequent diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/IntervalTracker.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/IntervalTracker.java index f3b11f56296f..f36c18bd54c8 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/IntervalTracker.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/IntervalTracker.java @@ -36,6 +36,11 @@ interface IntervalTracker extends OrdinalIterator { /** clear recorded information on this tracker. * */ void clear(); + /** + * restart reading from the first recorded ordinal, to replay a {@link #freeze() frozen} tracker + */ + void rewind(); + /** check if any data for the interval has been recorded * */ boolean get(int index); @@ -71,6 +76,12 @@ public void clear() { intervalsWithHit = 0; } + @Override + public void rewind() { + bitFrom = 0; + trackerState = 0; + } + @Override public boolean get(int index) { return tracker.get(index); diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index 1720ca502deb..b4618feff5ce 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -27,7 +27,6 @@ import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.sandbox.facet.cutters.FacetCutter; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -48,7 +47,7 @@ public abstract class LongRangeFacetCutter implements FacetCutter { // TODO: refactor - weird that we have both multi and single here. final LongValuesSource singleValues; - // Field to read a DocValuesSkipper from on the single-valued path, or null when disabled. + // Field name whose skip index is used on the single-valued path, or null when faceting a source. final String skipField; final LongRangeAndPos[] sortedRanges; @@ -153,25 +152,18 @@ public static LongRangeFacetCutter create(String field, LongRange[] longRanges) abstract List buildElementaryIntervals(); /** - * Returns the {@link DocValuesSkipper} for {@link #skipField} in this segment. Null when: no skip - * field is configured, the field has no skip index, or some doc in this segment has more than one - * value. + * Single-valued {@link LongValues} read directly from {@link #skipField} so its skip index can be + * used, or null when there is no skip field or the segment is multi-valued. */ - final DocValuesSkipper maybeSkipper(LeafReaderContext context) throws IOException { + final LongValues singleValuedSkipField(LeafReaderContext context) throws IOException { if (skipField == null) { return null; } - SortedNumericDocValues sortedNumeric = DocValues.getSortedNumeric(context.reader(), skipField); - if (DocValues.unwrapSingleton(sortedNumeric) == null) { - return null; - } - return context.reader().getDocValuesSkipper(skipField); - } - - /** Single-valued {@link LongValues} for {@link #skipField} in this segment. */ - final LongValues skipFieldValues(LeafReaderContext context) throws IOException { NumericDocValues values = DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), skipField)); + if (values == null) { + return null; + } return new LongValues() { @Override public long longValue() throws IOException { @@ -313,15 +305,20 @@ abstract static class LongRangeSingleValuedLeafFacetCutter implements LeafFacetC IntervalTracker requestedIntervalTracker; - // Skip index for the faceted field, or null when disabled. private final DocValuesSkipper skipper; - // Cached decision from advanceSkipper, valid for every doc up to (and including) upToInclusive: - // when upToSameInterval is true, all those docs map to elementary interval upToIntervalOrd. + // advanceSkipper's decisions for the current block; the fields below hold while doc <= + // upToInclusive, after which it runs again for the next block. private int upToInclusive = -1; + // Whether every value in the block maps to the single interval upToIntervalOrd. private boolean upToSameInterval; + // Whether every doc in the block has a value. + private boolean upToDense; private int upToIntervalOrd; + // Interval of the previous doc with a value, for replaying the tracker on a repeat. + private int previousIntervalOrd = -1; + LongRangeSingleValuedLeafFacetCutter(LongValues longValues, long[] boundaries, int[] pos) { this(longValues, boundaries, pos, null); } @@ -342,8 +339,11 @@ public boolean advanceExact(int doc) throws IOException { int intervalOrd; if (upToSameInterval) { - // We are inside a dense skip block that maps entirely to one elementary interval, so reuse - // the cached ordinal and skip the per-doc value lookup and binary search. + // Reuse the cached ordinal, skipping the binary search. A dense block also skips the value + // lookup, a sparse one still needs advanceExact to know whether this doc has a value. + if (upToDense == false && longValues.advanceExact(doc) == false) { + return false; + } intervalOrd = upToIntervalOrd; } else if (longValues.advanceExact(doc)) { intervalOrd = processValue(longValues.longValue()); @@ -351,19 +351,22 @@ public boolean advanceExact(int doc) throws IOException { return false; } - if (requestedIntervalTracker != null) { - requestedIntervalTracker.clear(); - } elementaryIntervalOrd = intervalOrd; - maybeRollUp(requestedIntervalTracker); if (requestedIntervalTracker != null) { - requestedIntervalTracker.freeze(); + if (skipper != null && intervalOrd == previousIntervalOrd) { + // Same interval as the previous doc, so replay its frozen rollup instead of rebuilding. + requestedIntervalTracker.rewind(); + } else { + requestedIntervalTracker.clear(); + maybeRollUp(requestedIntervalTracker); + requestedIntervalTracker.freeze(); + previousIntervalOrd = intervalOrd; + } } return true; } - /** Mirrors {@code HistogramCollector#advanceSkipper}. */ private void advanceSkipper(int doc) throws IOException { if (doc > skipper.maxDocID(0)) { skipper.advance(doc); @@ -378,14 +381,9 @@ private void advanceSkipper(int doc) throws IOException { } upToInclusive = skipper.maxDocID(0); - // Now find the highest level where all docs have a value and map to the same interval. + // Climb to the highest level that still maps to a single interval. for (int level = 0; level < skipper.numLevels(); ++level) { - int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; - if (skipper.docCount(level) != totalDocsAtLevel) { - // Some docs at this level have no value, so we can't resolve the whole block at once. - break; - } - // Long fields store raw values, the skipper's min/max map straight into the boundary space. + // Long fields store raw values, skipper's min/max maps straight into the boundary space. int minInterval = processValue(skipper.minValue(level)); int maxInterval = processValue(skipper.maxValue(level)); if (minInterval != maxInterval) { @@ -394,6 +392,8 @@ private void advanceSkipper(int doc) throws IOException { upToInclusive = skipper.maxDocID(level); upToSameInterval = true; upToIntervalOrd = minInterval; + int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; + upToDense = skipper.docCount(level) == totalDocsAtLevel; } } diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java index 4bb68dea11b1..598d083b5c8a 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java @@ -70,12 +70,11 @@ List buildElementaryIntervals() { @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - // Use the skip index when we can, otherwise fall back to the value source. - DocValuesSkipper skipper = maybeSkipper(context); - if (skipper != null) { - LongValues values = skipFieldValues(context); + LongValues skipFieldValues = singleValuedSkipField(context); + if (skipFieldValues != null) { + DocValuesSkipper skipper = context.reader().getDocValuesSkipper(skipField); return new NonOverlappingLongRangeSingleValueLeafFacetCutter( - values, boundaries, pos, skipper); + skipFieldValues, boundaries, pos, skipper); } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java index 2eff548329ae..c25023ae661b 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java @@ -149,12 +149,11 @@ private static LongRangeNode split(int start, int end, List elem @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - // Use the skip index when we can, otherwise fall back to the value source. - DocValuesSkipper skipper = maybeSkipper(context); - if (skipper != null) { - LongValues values = skipFieldValues(context); + LongValues skipFieldValues = singleValuedSkipField(context); + if (skipFieldValues != null) { + DocValuesSkipper skipper = context.reader().getDocValuesSkipper(skipField); return new OverlappingSingleValuedRangeLeafFacetCutter( - values, boundaries, pos, requestedRangeCount, root, skipper); + skipFieldValues, boundaries, pos, requestedRangeCount, root, skipper); } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java index 4caccc4b07e4..05ab7a315c55 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/utils/RangeFacetBuilderFactory.java @@ -35,7 +35,6 @@ private RangeFacetBuilderFactory() {} /** Request long range facets for numeric field by name. */ public static CommonFacetBuilder forLongRanges(String field, LongRange... ranges) { - // Pass the field by name so we can use its skip index when present. return new CommonFacetBuilder( field, LongRangeFacetCutter.create(field, ranges), new RangeOrdToLabel(ranges)) .withSortByOrdinal(); diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java index 7deb908cb93d..5eabe94d03f6 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java @@ -944,7 +944,6 @@ public void testSkipIndexEquivalenceExtremeValues() throws Exception { default -> TestUtil.nextLong(random(), -100, 100); }; doc.add(NumericDocValuesField.indexedField("field", v)); - doc.add(new LongPoint("point", v)); w.addDocument(doc); } @@ -998,8 +997,43 @@ public void testSkipIndexEquivalenceMultiValued() throws Exception { IOUtils.close(dir); } - // Asserts faceting "field" by name (skip index when available) matches faceting via a - // MultiLongValuesSource (no skip index), over random range sets including extreme bounds. + public void testSkipIndexEquivalenceFewValues() throws Exception { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setIndexSort(new Sort(new SortField("field", SortField.Type.LONG, false))); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + doc.add(NumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), 0, 5))); + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "few-values"); + + w.close(); + IOUtils.close(dir); + } + + public void testSingleValuedNoSkipIndex() throws Exception { + Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + doc.add(new NumericDocValuesField("field", TestUtil.nextLong(random(), -100, 100))); + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "single-valued-no-skip"); + + w.close(); + IOUtils.close(dir); + } + private void assertSkipIndexEquivalence(RandomIndexWriter w, String desc) throws IOException { IndexReader r = w.getReader(); try { From d56df023cd946d0c68ad29e90d741b2d3f21c3f6 Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Mon, 29 Jun 2026 11:02:35 +0100 Subject: [PATCH 4/7] Remove single-valued unwrapping routing --- .../cutters/ranges/LongRangeFacetCutter.java | 20 +++++++++++++------ .../NonOverlappingLongRangeFacetCutter.java | 9 +++++---- .../OverlappingLongRangeFacetCutter.java | 9 +++++---- .../lucene/sandbox/facet/TestRangeFacet.java | 17 ---------------- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index b4618feff5ce..e94346c00f0c 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -27,6 +27,7 @@ import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.sandbox.facet.cutters.FacetCutter; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -152,18 +153,25 @@ public static LongRangeFacetCutter create(String field, LongRange[] longRanges) abstract List buildElementaryIntervals(); /** - * Single-valued {@link LongValues} read directly from {@link #skipField} so its skip index can be - * used, or null when there is no skip field or the segment is multi-valued. + * Returns the {@link DocValuesSkipper} for {@link #skipField} in this segment. Null when: no skip + * field is configured, the field has no skip index, or some doc in this segment has more than one + * value. */ - final LongValues singleValuedSkipField(LeafReaderContext context) throws IOException { + final DocValuesSkipper maybeSkipper(LeafReaderContext context) throws IOException { if (skipField == null) { return null; } - NumericDocValues values = - DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), skipField)); - if (values == null) { + SortedNumericDocValues sortedNumeric = DocValues.getSortedNumeric(context.reader(), skipField); + if (DocValues.unwrapSingleton(sortedNumeric) == null) { return null; } + return context.reader().getDocValuesSkipper(skipField); + } + + /** Single-valued {@link LongValues} for {@link #skipField} in this segment. */ + final LongValues skipFieldValues(LeafReaderContext context) throws IOException { + NumericDocValues values = + DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), skipField)); return new LongValues() { @Override public long longValue() throws IOException { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java index 598d083b5c8a..4bb68dea11b1 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java @@ -70,11 +70,12 @@ List buildElementaryIntervals() { @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - LongValues skipFieldValues = singleValuedSkipField(context); - if (skipFieldValues != null) { - DocValuesSkipper skipper = context.reader().getDocValuesSkipper(skipField); + // Use the skip index when we can, otherwise fall back to the value source. + DocValuesSkipper skipper = maybeSkipper(context); + if (skipper != null) { + LongValues values = skipFieldValues(context); return new NonOverlappingLongRangeSingleValueLeafFacetCutter( - skipFieldValues, boundaries, pos, skipper); + values, boundaries, pos, skipper); } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java index c25023ae661b..2eff548329ae 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java @@ -149,11 +149,12 @@ private static LongRangeNode split(int start, int end, List elem @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - LongValues skipFieldValues = singleValuedSkipField(context); - if (skipFieldValues != null) { - DocValuesSkipper skipper = context.reader().getDocValuesSkipper(skipField); + // Use the skip index when we can, otherwise fall back to the value source. + DocValuesSkipper skipper = maybeSkipper(context); + if (skipper != null) { + LongValues values = skipFieldValues(context); return new OverlappingSingleValuedRangeLeafFacetCutter( - skipFieldValues, boundaries, pos, requestedRangeCount, root, skipper); + values, boundaries, pos, requestedRangeCount, root, skipper); } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java index 5eabe94d03f6..173bf5c6280d 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java @@ -1017,23 +1017,6 @@ public void testSkipIndexEquivalenceFewValues() throws Exception { IOUtils.close(dir); } - public void testSingleValuedNoSkipIndex() throws Exception { - Directory dir = newDirectory(); - RandomIndexWriter w = new RandomIndexWriter(random(), dir); - - int numDocs = atLeast(1000); - for (int i = 0; i < numDocs; i++) { - Document doc = new Document(); - doc.add(new NumericDocValuesField("field", TestUtil.nextLong(random(), -100, 100))); - w.addDocument(doc); - } - - assertSkipIndexEquivalence(w, "single-valued-no-skip"); - - w.close(); - IOUtils.close(dir); - } - private void assertSkipIndexEquivalence(RandomIndexWriter w, String desc) throws IOException { IndexReader r = w.getReader(); try { From 72f37bc185e126481907962833af0aefe07991ba Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:22:00 +0100 Subject: [PATCH 5/7] Address review comments --- .../cutters/ranges/LongRangeFacetCutter.java | 38 ++++++++----------- .../NonOverlappingLongRangeFacetCutter.java | 18 +++++---- .../OverlappingLongRangeFacetCutter.java | 18 +++++---- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index e94346c00f0c..2fa6d97f6d1e 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -27,7 +27,6 @@ import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.sandbox.facet.cutters.FacetCutter; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -48,8 +47,8 @@ public abstract class LongRangeFacetCutter implements FacetCutter { // TODO: refactor - weird that we have both multi and single here. final LongValuesSource singleValues; - // Field name whose skip index is used on the single-valued path, or null when faceting a source. - final String skipField; + // Field faceted by name, whose skip index is used when present, or null when faceting a source. + final String fieldName; final LongRangeAndPos[] sortedRanges; @@ -74,18 +73,18 @@ static LongRangeFacetCutter createSingleOrMultiValued( return createSingleOrMultiValued(longValuesSource, singleLongValuesSource, longRanges, null); } - /** Same as above, but uses the {@code skipField} skip index on the single-valued path. */ + /** Same as above, but uses the {@code fieldName} skip index when present. */ static LongRangeFacetCutter createSingleOrMultiValued( MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, LongRange[] longRanges, - String skipField) { + String fieldName) { if (areOverlappingRanges(longRanges)) { return new OverlappingLongRangeFacetCutter( - longValuesSource, singleLongValuesSource, longRanges, skipField); + longValuesSource, singleLongValuesSource, longRanges, fieldName); } return new NonOverlappingLongRangeFacetCutter( - longValuesSource, singleLongValuesSource, longRanges, skipField); + longValuesSource, singleLongValuesSource, longRanges, fieldName); } public static LongRangeFacetCutter create( @@ -107,7 +106,7 @@ public static LongRangeFacetCutter create(String field, LongRange[] longRanges) MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, LongRange[] longRanges, - String skipField) { + String fieldName) { super(); valuesSource = longValuesSource; if (singleLongValuesSource != null) { @@ -115,7 +114,7 @@ public static LongRangeFacetCutter create(String field, LongRange[] longRanges) } else { singleValues = MultiLongValuesSource.unwrapSingleton(valuesSource); } - this.skipField = skipField; + this.fieldName = fieldName; sortedRanges = new LongRangeAndPos[longRanges.length]; requestedRangeCount = longRanges.length; @@ -153,25 +152,18 @@ public static LongRangeFacetCutter create(String field, LongRange[] longRanges) abstract List buildElementaryIntervals(); /** - * Returns the {@link DocValuesSkipper} for {@link #skipField} in this segment. Null when: no skip - * field is configured, the field has no skip index, or some doc in this segment has more than one - * value. + * Single-valued {@link NumericDocValues} for {@link #fieldName} in this segment, or null when no + * field is configured or some doc in this segment has more than one value. */ - final DocValuesSkipper maybeSkipper(LeafReaderContext context) throws IOException { - if (skipField == null) { + final NumericDocValues singletonFieldValues(LeafReaderContext context) throws IOException { + if (fieldName == null) { return null; } - SortedNumericDocValues sortedNumeric = DocValues.getSortedNumeric(context.reader(), skipField); - if (DocValues.unwrapSingleton(sortedNumeric) == null) { - return null; - } - return context.reader().getDocValuesSkipper(skipField); + return DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), fieldName)); } - /** Single-valued {@link LongValues} for {@link #skipField} in this segment. */ - final LongValues skipFieldValues(LeafReaderContext context) throws IOException { - NumericDocValues values = - DocValues.unwrapSingleton(DocValues.getSortedNumeric(context.reader(), skipField)); + /** Wraps {@link NumericDocValues} as {@link LongValues}. */ + static LongValues asLongValues(NumericDocValues values) { return new LongValues() { @Override public long longValue() throws IOException { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java index 4bb68dea11b1..d3c325d82045 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java @@ -24,6 +24,7 @@ import org.apache.lucene.facet.range.LongRange; import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; import org.apache.lucene.search.LongValuesSource; @@ -34,8 +35,8 @@ class NonOverlappingLongRangeFacetCutter extends LongRangeFacetCutter { MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, LongRange[] longRanges, - String skipField) { - super(longValuesSource, singleLongValuesSource, longRanges, skipField); + String fieldName) { + super(longValuesSource, singleLongValuesSource, longRanges, fieldName); } /** @@ -70,12 +71,13 @@ List buildElementaryIntervals() { @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - // Use the skip index when we can, otherwise fall back to the value source. - DocValuesSkipper skipper = maybeSkipper(context); - if (skipper != null) { - LongValues values = skipFieldValues(context); - return new NonOverlappingLongRangeSingleValueLeafFacetCutter( - values, boundaries, pos, skipper); + NumericDocValues singletonValues = singletonFieldValues(context); + if (singletonValues != null) { + DocValuesSkipper skipper = context.reader().getDocValuesSkipper(fieldName); + if (skipper != null) { + return new NonOverlappingLongRangeSingleValueLeafFacetCutter( + asLongValues(singletonValues), boundaries, pos, skipper); + } } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java index 2eff548329ae..a034f2ef7bb2 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java @@ -27,6 +27,7 @@ import org.apache.lucene.facet.range.LongRange; import org.apache.lucene.index.DocValuesSkipper; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.internal.hppc.IntCursor; import org.apache.lucene.sandbox.facet.cutters.LeafFacetCutter; import org.apache.lucene.search.LongValues; @@ -45,8 +46,8 @@ class OverlappingLongRangeFacetCutter extends LongRangeFacetCutter { MultiLongValuesSource longValuesSource, LongValuesSource singleLongValuesSource, LongRange[] longRanges, - String skipField) { - super(longValuesSource, singleLongValuesSource, longRanges, skipField); + String fieldName) { + super(longValuesSource, singleLongValuesSource, longRanges, fieldName); // Build binary tree on top of intervals: root = split(0, elementaryIntervals.size(), elementaryIntervals); @@ -149,12 +150,13 @@ private static LongRangeNode split(int start, int end, List elem @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - // Use the skip index when we can, otherwise fall back to the value source. - DocValuesSkipper skipper = maybeSkipper(context); - if (skipper != null) { - LongValues values = skipFieldValues(context); - return new OverlappingSingleValuedRangeLeafFacetCutter( - values, boundaries, pos, requestedRangeCount, root, skipper); + NumericDocValues singletonValues = singletonFieldValues(context); + if (singletonValues != null) { + DocValuesSkipper skipper = context.reader().getDocValuesSkipper(fieldName); + if (skipper != null) { + return new OverlappingSingleValuedRangeLeafFacetCutter( + asLongValues(singletonValues), boundaries, pos, requestedRangeCount, root, skipper); + } } if (singleValues != null) { LongValues values = singleValues.getValues(context, null); From b89fd6f5a5236ab1886d07ff3b07358c2b036693 Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:38:15 +0100 Subject: [PATCH 6/7] Use the skip index for multi-valued fields --- .../cutters/ranges/LongRangeFacetCutter.java | 85 ++++++++++-- .../NonOverlappingLongRangeFacetCutter.java | 18 ++- .../OverlappingLongRangeFacetCutter.java | 25 +++- .../lucene/sandbox/facet/TestRangeFacet.java | 124 ++++++++++++++++++ 4 files changed, 230 insertions(+), 22 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index 2fa6d97f6d1e..1e1c87173a2e 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -212,36 +212,63 @@ abstract static class LongRangeMultivaluedLeafFacetCutter implements LeafFacetCu // exclusive ranges. IntervalTracker requestedIntervalTracker; + private final DocValuesSkipper skipper; + + // advanceSkipper's decisions for the current block; the fields below hold while doc <= + // upToInclusive, after which it runs again for the next block. + private int upToInclusive = -1; + // Whether every value in the block maps to the single interval upToIntervalOrd. + private boolean upToSameInterval; + // Whether every doc in the block has a value. + private boolean upToDense; + private int upToIntervalOrd; + LongRangeMultivaluedLeafFacetCutter(MultiLongValues longValues, long[] boundaries, int[] pos) { + this(longValues, boundaries, pos, null); + } + + LongRangeMultivaluedLeafFacetCutter( + MultiLongValues longValues, long[] boundaries, int[] pos, DocValuesSkipper skipper) { this.multiLongValues = longValues; this.boundaries = boundaries; this.pos = pos; + this.skipper = skipper; elementaryIntervalTracker = new IntervalTracker.MultiIntervalTracker(boundaries.length); } @Override public boolean advanceExact(int doc) throws IOException { - if (multiLongValues.advanceExact(doc) == false) { - return false; + if (skipper != null && doc > upToInclusive) { + advanceSkipper(doc); } elementaryIntervalTracker.clear(); - if (requestedIntervalTracker != null) { requestedIntervalTracker.clear(); } - long numValues = multiLongValues.getValueCount(); - - int lastIntervalSeen = -1; - - for (int i = 0; i < numValues; i++) { - lastIntervalSeen = processValue(multiLongValues.nextValue(), lastIntervalSeen); - assert lastIntervalSeen >= 0 && lastIntervalSeen < boundaries.length; - elementaryIntervalTracker.set(lastIntervalSeen); - if (lastIntervalSeen == boundaries.length - 1) { - // we've already reached the end of all possible intervals for this doc - break; + if (upToSameInterval) { + // Every value in the block maps to upToIntervalOrd, so this doc maps to exactly that + // interval. A dense block also skips the value lookup, a sparse one still needs advanceExact + // to know whether this doc has a value. + if (upToDense == false && multiLongValues.advanceExact(doc) == false) { + return false; + } + elementaryIntervalTracker.set(upToIntervalOrd); + } else { + if (multiLongValues.advanceExact(doc) == false) { + return false; + } + long numValues = multiLongValues.getValueCount(); + int lastIntervalSeen = -1; + for (int i = 0; i < numValues; i++) { + lastIntervalSeen = processValue(multiLongValues.nextValue(), lastIntervalSeen); + assert lastIntervalSeen >= 0 && lastIntervalSeen < boundaries.length; + elementaryIntervalTracker.set(lastIntervalSeen); + if (lastIntervalSeen == boundaries.length - 1) { + // we've already reached the end of all possible intervals for this doc + break; + } } } maybeRollUp(requestedIntervalTracker); @@ -255,6 +282,36 @@ public boolean advanceExact(int doc) throws IOException { return true; } + private void advanceSkipper(int doc) throws IOException { + if (doc > skipper.maxDocID(0)) { + skipper.advance(doc); + } + upToSameInterval = false; + + if (skipper.minDocID(0) > doc) { + // Corner case which happens if doc doesn't have a value and is between two intervals of the + // skip index. Fall back to per-doc lookups until the next block. + upToInclusive = skipper.minDocID(0) - 1; + return; + } + + upToInclusive = skipper.maxDocID(0); + // Climb to the highest level that still maps to a single interval. + for (int level = 0; level < skipper.numLevels(); ++level) { + // Long fields store raw values, skipper's min/max maps straight into the boundary space. + int minInterval = processValue(skipper.minValue(level), -1); + int maxInterval = processValue(skipper.maxValue(level), -1); + if (minInterval != maxInterval) { + break; + } + upToInclusive = skipper.maxDocID(level); + upToSameInterval = true; + upToIntervalOrd = minInterval; + int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; + upToDense = skipper.docCount(level) == totalDocsAtLevel; + } + } + // Returns the value of the interval v belongs or lastIntervalSeen // if no processing is done, it returns the lastIntervalSeen private int processValue(long v, int lastIntervalSeen) { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java index d3c325d82045..291531014656 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/NonOverlappingLongRangeFacetCutter.java @@ -71,12 +71,17 @@ List buildElementaryIntervals() { @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - NumericDocValues singletonValues = singletonFieldValues(context); - if (singletonValues != null) { + if (fieldName != null) { DocValuesSkipper skipper = context.reader().getDocValuesSkipper(fieldName); if (skipper != null) { - return new NonOverlappingLongRangeSingleValueLeafFacetCutter( - asLongValues(singletonValues), boundaries, pos, skipper); + NumericDocValues singletonValues = singletonFieldValues(context); + if (singletonValues != null) { + return new NonOverlappingLongRangeSingleValueLeafFacetCutter( + asLongValues(singletonValues), boundaries, pos, skipper); + } + MultiLongValues values = valuesSource.getValues(context); + return new NonOverlappingLongRangeMultiValueLeafFacetCutter( + values, boundaries, pos, skipper); } } if (singleValues != null) { @@ -101,6 +106,11 @@ static class NonOverlappingLongRangeMultiValueLeafFacetCutter super(longValues, boundaries, pos); } + NonOverlappingLongRangeMultiValueLeafFacetCutter( + MultiLongValues longValues, long[] boundaries, int[] pos, DocValuesSkipper skipper) { + super(longValues, boundaries, pos, skipper); + } + @Override public int nextOrd() throws IOException { while (true) { diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java index a034f2ef7bb2..d168129cbdcd 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/OverlappingLongRangeFacetCutter.java @@ -150,12 +150,17 @@ private static LongRangeNode split(int start, int end, List elem @Override public LeafFacetCutter createLeafCutter(LeafReaderContext context) throws IOException { - NumericDocValues singletonValues = singletonFieldValues(context); - if (singletonValues != null) { + if (fieldName != null) { DocValuesSkipper skipper = context.reader().getDocValuesSkipper(fieldName); if (skipper != null) { - return new OverlappingSingleValuedRangeLeafFacetCutter( - asLongValues(singletonValues), boundaries, pos, requestedRangeCount, root, skipper); + NumericDocValues singletonValues = singletonFieldValues(context); + if (singletonValues != null) { + return new OverlappingSingleValuedRangeLeafFacetCutter( + asLongValues(singletonValues), boundaries, pos, requestedRangeCount, root, skipper); + } + MultiLongValues values = valuesSource.getValues(context); + return new OverlappingMultivaluedRangeLeafFacetCutter( + values, boundaries, pos, requestedRangeCount, root, skipper); } } if (singleValues != null) { @@ -192,6 +197,18 @@ static class OverlappingMultivaluedRangeLeafFacetCutter this.elementaryIntervalRoot = elementaryIntervalRoot; } + OverlappingMultivaluedRangeLeafFacetCutter( + MultiLongValues longValues, + long[] boundaries, + int[] pos, + int requestedRangeCount, + LongRangeNode elementaryIntervalRoot, + DocValuesSkipper skipper) { + super(longValues, boundaries, pos, skipper); + requestedIntervalTracker = new IntervalTracker.MultiIntervalTracker(requestedRangeCount); + this.elementaryIntervalRoot = elementaryIntervalRoot; + } + @Override void maybeRollUp(IntervalTracker rollUpInto) { elementaryIntervalUpto = 0; diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java index 173bf5c6280d..ce414a86010f 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java @@ -997,6 +997,113 @@ public void testSkipIndexEquivalenceMultiValued() throws Exception { IOUtils.close(dir); } + public void testSkipIndexEquivalenceMultiValuedFewValues() throws Exception { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + int numVals = TestUtil.nextInt(random(), 1, 3); + for (int j = 0; j < numVals; j++) { + doc.add(SortedNumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), 0, 5))); + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "multi-valued-few-values"); + + w.close(); + IOUtils.close(dir); + } + + public void testSkipIndexEquivalenceMultiValuedExtremeValues() throws Exception { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + int numVals = TestUtil.nextInt(random(), 1, 3); + for (int j = 0; j < numVals; j++) { + long v = + switch (random().nextInt(4)) { + case 0 -> Long.MIN_VALUE; + case 1 -> Long.MAX_VALUE; + default -> TestUtil.nextLong(random(), -5, 5); + }; + doc.add(SortedNumericDocValuesField.indexedField("field", v)); + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "multi-valued-extreme"); + + w.close(); + IOUtils.close(dir); + } + + public void testSkipIndexEquivalenceMultiValuedSparse() throws Exception { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + iwc.setCodec(TestUtil.alwaysDocValuesFormat(new Lucene90DocValuesFormat(4))); + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + // Leave roughly a third of the docs without a value so skip blocks aren't dense. + if (random().nextInt(3) != 0) { + int numVals = TestUtil.nextInt(random(), 1, 3); + for (int j = 0; j < numVals; j++) { + doc.add( + SortedNumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), 0, 5))); + } + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "multi-valued-sparse"); + + w.close(); + IOUtils.close(dir); + } + + public void testSkipIndexEquivalenceLongRunsDefaultInterval() throws Exception { + for (boolean multiValued : new boolean[] {false, true}) { + Directory dir = newDirectory(); + IndexWriterConfig iwc = newIndexWriterConfig(); + // A multi-valued SORTED_NUMERIC field can't be index-sorted with a plain LONG SortField. + if (multiValued == false) { + iwc.setIndexSort(new Sort(new SortField("field", SortField.Type.LONG))); + } + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + + int numDocs = atLeast(20000); + // Few distinct values with long runs => wide same-interval, dense skip spans across levels. + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + long v = (i / 5000) % 4; + if (multiValued) { + doc.add(SortedNumericDocValuesField.indexedField("field", v)); + doc.add(SortedNumericDocValuesField.indexedField("field", v)); + } else { + doc.add(NumericDocValuesField.indexedField("field", v)); + } + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "multi-level-dense mv=" + multiValued); + + w.close(); + IOUtils.close(dir); + } + } + public void testSkipIndexEquivalenceFewValues() throws Exception { Directory dir = newDirectory(); IndexWriterConfig iwc = newIndexWriterConfig(); @@ -1017,6 +1124,23 @@ public void testSkipIndexEquivalenceFewValues() throws Exception { IOUtils.close(dir); } + public void testByNameNoSkipIndexEquivalence() throws Exception { + Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir); + + int numDocs = atLeast(1000); + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + doc.add(new NumericDocValuesField("field", TestUtil.nextLong(random(), -100, 100))); + w.addDocument(doc); + } + + assertSkipIndexEquivalence(w, "by-name-no-skip-index"); + + w.close(); + IOUtils.close(dir); + } + private void assertSkipIndexEquivalence(RandomIndexWriter w, String desc) throws IOException { IndexReader r = w.getReader(); try { From 26019dc40506dbdb4b54f22a678779607fa1c135 Mon Sep 17 00:00:00 2001 From: slow-J <32519034+slow-J@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:49:08 +0100 Subject: [PATCH 7/7] Consolidate skipper block handling --- .../cutters/ranges/LongRangeFacetCutter.java | 227 +++++++----------- .../lucene/sandbox/facet/TestRangeFacet.java | 3 +- 2 files changed, 95 insertions(+), 135 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java index 1e1c87173a2e..07e01275c54b 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java @@ -202,6 +202,76 @@ private static boolean areOverlappingRanges(LongRange[] ranges) { return false; } + // Returns the elementary interval that v belongs to, binary-searching boundaries from lo onwards. + static int toElementaryInterval(long[] boundaries, long v, int lo) { + int hi = boundaries.length - 1; + int lowerBound = lo; + + while (true) { + int mid = (lo + hi) >>> 1; + if (v <= boundaries[mid]) { + if (mid == lowerBound) { + return mid; + } else { + hi = mid - 1; + } + } else if (v > boundaries[mid + 1]) { + lo = mid + 1; + } else { + return mid + 1; + } + } + } + + /** Cached skipper decision for a leaf cutter, shared by the single- and multi-valued cutters. */ + static final class SkipBlock { + private final DocValuesSkipper skipper; + private final long[] boundaries; + + // Cached decision from advance, valid for every doc up to (and including) upToInclusive. + int upToInclusive = -1; + // Whether every value in the block maps to the single interval upToIntervalOrd. + boolean upToSameInterval; + // Whether every doc in the block has a value. + boolean upToDense; + int upToIntervalOrd; + + SkipBlock(DocValuesSkipper skipper, long[] boundaries) { + this.skipper = skipper; + this.boundaries = boundaries; + } + + void advance(int doc) throws IOException { + if (doc > skipper.maxDocID(0)) { + skipper.advance(doc); + } + upToSameInterval = false; + + if (skipper.minDocID(0) > doc) { + // Corner case which happens if doc doesn't have a value and is between two intervals of the + // skip index. Fall back to per-doc lookups until the next block. + upToInclusive = skipper.minDocID(0) - 1; + return; + } + + upToInclusive = skipper.maxDocID(0); + // Climb to the highest level that still maps to a single interval. + for (int level = 0; level < skipper.numLevels(); ++level) { + // Long fields store raw values, skipper's min/max maps straight into the boundary space. + int minInterval = toElementaryInterval(boundaries, skipper.minValue(level), 0); + int maxInterval = toElementaryInterval(boundaries, skipper.maxValue(level), 0); + if (minInterval != maxInterval) { + break; + } + upToInclusive = skipper.maxDocID(level); + upToSameInterval = true; + upToIntervalOrd = minInterval; + int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; + upToDense = skipper.docCount(level) == totalDocsAtLevel; + } + } + } + abstract static class LongRangeMultivaluedLeafFacetCutter implements LeafFacetCutter { private final MultiLongValues multiLongValues; private final long[] boundaries; @@ -212,16 +282,8 @@ abstract static class LongRangeMultivaluedLeafFacetCutter implements LeafFacetCu // exclusive ranges. IntervalTracker requestedIntervalTracker; - private final DocValuesSkipper skipper; - - // advanceSkipper's decisions for the current block; the fields below hold while doc <= - // upToInclusive, after which it runs again for the next block. - private int upToInclusive = -1; - // Whether every value in the block maps to the single interval upToIntervalOrd. - private boolean upToSameInterval; - // Whether every doc in the block has a value. - private boolean upToDense; - private int upToIntervalOrd; + // Non-null when the field has a skip index. + private final SkipBlock skipBlock; LongRangeMultivaluedLeafFacetCutter(MultiLongValues longValues, long[] boundaries, int[] pos) { this(longValues, boundaries, pos, null); @@ -232,14 +294,14 @@ abstract static class LongRangeMultivaluedLeafFacetCutter implements LeafFacetCu this.multiLongValues = longValues; this.boundaries = boundaries; this.pos = pos; - this.skipper = skipper; + this.skipBlock = skipper == null ? null : new SkipBlock(skipper, boundaries); elementaryIntervalTracker = new IntervalTracker.MultiIntervalTracker(boundaries.length); } @Override public boolean advanceExact(int doc) throws IOException { - if (skipper != null && doc > upToInclusive) { - advanceSkipper(doc); + if (skipBlock != null && doc > skipBlock.upToInclusive) { + skipBlock.advance(doc); } elementaryIntervalTracker.clear(); @@ -247,14 +309,13 @@ public boolean advanceExact(int doc) throws IOException { requestedIntervalTracker.clear(); } - if (upToSameInterval) { - // Every value in the block maps to upToIntervalOrd, so this doc maps to exactly that - // interval. A dense block also skips the value lookup, a sparse one still needs advanceExact - // to know whether this doc has a value. - if (upToDense == false && multiLongValues.advanceExact(doc) == false) { + if (skipBlock != null && skipBlock.upToSameInterval) { + // Reuse the cached ordinal, skipping the binary search. A dense block also skips the value + // lookup, a sparse one still needs advanceExact to know whether this doc has a value. + if (skipBlock.upToDense == false && multiLongValues.advanceExact(doc) == false) { return false; } - elementaryIntervalTracker.set(upToIntervalOrd); + elementaryIntervalTracker.set(skipBlock.upToIntervalOrd); } else { if (multiLongValues.advanceExact(doc) == false) { return false; @@ -266,7 +327,6 @@ public boolean advanceExact(int doc) throws IOException { assert lastIntervalSeen >= 0 && lastIntervalSeen < boundaries.length; elementaryIntervalTracker.set(lastIntervalSeen); if (lastIntervalSeen == boundaries.length - 1) { - // we've already reached the end of all possible intervals for this doc break; } } @@ -282,40 +342,10 @@ public boolean advanceExact(int doc) throws IOException { return true; } - private void advanceSkipper(int doc) throws IOException { - if (doc > skipper.maxDocID(0)) { - skipper.advance(doc); - } - upToSameInterval = false; - - if (skipper.minDocID(0) > doc) { - // Corner case which happens if doc doesn't have a value and is between two intervals of the - // skip index. Fall back to per-doc lookups until the next block. - upToInclusive = skipper.minDocID(0) - 1; - return; - } - - upToInclusive = skipper.maxDocID(0); - // Climb to the highest level that still maps to a single interval. - for (int level = 0; level < skipper.numLevels(); ++level) { - // Long fields store raw values, skipper's min/max maps straight into the boundary space. - int minInterval = processValue(skipper.minValue(level), -1); - int maxInterval = processValue(skipper.maxValue(level), -1); - if (minInterval != maxInterval) { - break; - } - upToInclusive = skipper.maxDocID(level); - upToSameInterval = true; - upToIntervalOrd = minInterval; - int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; - upToDense = skipper.docCount(level) == totalDocsAtLevel; - } - } - // Returns the value of the interval v belongs or lastIntervalSeen // if no processing is done, it returns the lastIntervalSeen private int processValue(long v, int lastIntervalSeen) { - int lo = 0, hi = boundaries.length - 1; + int lo = 0; if (lastIntervalSeen != -1) { // this is the multivalued doc case, we need to set lo correctly @@ -333,22 +363,8 @@ private int processValue(long v, int lastIntervalSeen) { return lastIntervalSeen; } } - int lowerBound = lo; - - while (true) { - int mid = (lo + hi) >>> 1; - if (v <= boundaries[mid]) { - if (mid == lowerBound) { - return mid; - } else { - hi = mid - 1; - } - } else if (v > boundaries[mid + 1]) { - lo = mid + 1; - } else { - return mid + 1; - } - } + + return toElementaryInterval(boundaries, v, lo); } void maybeRollUp(IntervalTracker rollUpInto) {} @@ -362,16 +378,8 @@ abstract static class LongRangeSingleValuedLeafFacetCutter implements LeafFacetC IntervalTracker requestedIntervalTracker; - private final DocValuesSkipper skipper; - - // advanceSkipper's decisions for the current block; the fields below hold while doc <= - // upToInclusive, after which it runs again for the next block. - private int upToInclusive = -1; - // Whether every value in the block maps to the single interval upToIntervalOrd. - private boolean upToSameInterval; - // Whether every doc in the block has a value. - private boolean upToDense; - private int upToIntervalOrd; + // Non-null when the field has a skip index. + private final SkipBlock skipBlock; // Interval of the previous doc with a value, for replaying the tracker on a repeat. private int previousIntervalOrd = -1; @@ -385,23 +393,23 @@ abstract static class LongRangeSingleValuedLeafFacetCutter implements LeafFacetC this.longValues = longValues; this.boundaries = boundaries; this.pos = pos; - this.skipper = skipper; + this.skipBlock = skipper == null ? null : new SkipBlock(skipper, boundaries); } @Override public boolean advanceExact(int doc) throws IOException { - if (skipper != null && doc > upToInclusive) { - advanceSkipper(doc); + if (skipBlock != null && doc > skipBlock.upToInclusive) { + skipBlock.advance(doc); } int intervalOrd; - if (upToSameInterval) { + if (skipBlock != null && skipBlock.upToSameInterval) { // Reuse the cached ordinal, skipping the binary search. A dense block also skips the value // lookup, a sparse one still needs advanceExact to know whether this doc has a value. - if (upToDense == false && longValues.advanceExact(doc) == false) { + if (skipBlock.upToDense == false && longValues.advanceExact(doc) == false) { return false; } - intervalOrd = upToIntervalOrd; + intervalOrd = skipBlock.upToIntervalOrd; } else if (longValues.advanceExact(doc)) { intervalOrd = processValue(longValues.longValue()); } else { @@ -410,7 +418,7 @@ public boolean advanceExact(int doc) throws IOException { elementaryIntervalOrd = intervalOrd; if (requestedIntervalTracker != null) { - if (skipper != null && intervalOrd == previousIntervalOrd) { + if (skipBlock != null && intervalOrd == previousIntervalOrd) { // Same interval as the previous doc, so replay its frozen rollup instead of rebuilding. requestedIntervalTracker.rewind(); } else { @@ -424,57 +432,8 @@ public boolean advanceExact(int doc) throws IOException { return true; } - private void advanceSkipper(int doc) throws IOException { - if (doc > skipper.maxDocID(0)) { - skipper.advance(doc); - } - upToSameInterval = false; - - if (skipper.minDocID(0) > doc) { - // Corner case which happens if doc doesn't have a value and is between two intervals of the - // skip index. Fall back to per-doc lookups until the next block. - upToInclusive = skipper.minDocID(0) - 1; - return; - } - - upToInclusive = skipper.maxDocID(0); - // Climb to the highest level that still maps to a single interval. - for (int level = 0; level < skipper.numLevels(); ++level) { - // Long fields store raw values, skipper's min/max maps straight into the boundary space. - int minInterval = processValue(skipper.minValue(level)); - int maxInterval = processValue(skipper.maxValue(level)); - if (minInterval != maxInterval) { - break; - } - upToInclusive = skipper.maxDocID(level); - upToSameInterval = true; - upToIntervalOrd = minInterval; - int totalDocsAtLevel = skipper.maxDocID(level) - skipper.minDocID(level) + 1; - upToDense = skipper.docCount(level) == totalDocsAtLevel; - } - } - - // Returns the value of the interval v belongs or lastIntervalSeen - // if no processing is done, it returns the lastIntervalSeen private int processValue(long v) { - int lo = 0, hi = boundaries.length - 1; - - int lowerBound = lo; - - while (true) { - int mid = (lo + hi) >>> 1; - if (v <= boundaries[mid]) { - if (mid == lowerBound) { - return mid; - } else { - hi = mid - 1; - } - } else if (v > boundaries[mid + 1]) { - lo = mid + 1; - } else { - return mid + 1; - } - } + return toElementaryInterval(boundaries, v, 0); } void maybeRollUp(IntervalTracker rollUpInto) {} diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java index ce414a86010f..a9c553861b38 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/facet/TestRangeFacet.java @@ -1008,7 +1008,8 @@ public void testSkipIndexEquivalenceMultiValuedFewValues() throws Exception { Document doc = new Document(); int numVals = TestUtil.nextInt(random(), 1, 3); for (int j = 0; j < numVals; j++) { - doc.add(SortedNumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), 0, 5))); + doc.add( + SortedNumericDocValuesField.indexedField("field", TestUtil.nextLong(random(), 0, 5))); } w.addDocument(doc); }