This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new f5d9be62448 Build bitmap and sorted doc id sets only through their
factories (#19746)
f5d9be62448 is described below
commit f5d9be624486484303bd582c888f401fa9172e02
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Sun Oct 4 21:24:16 2026 -0700
Build bitmap and sorted doc id sets only through their factories (#19746)
---
.../java/org/apache/pinot/core/common/BlockDocIdSet.java | 3 ++-
.../pinot/core/operator/docidsets/BitmapDocIdSet.java | 15 ++++++---------
.../apache/pinot/core/operator/docidsets/NotDocIdSet.java | 2 +-
.../core/operator/docidsets/RangelessBitmapDocIdSet.java | 6 +++---
.../pinot/core/operator/docidsets/SortedDocIdSet.java | 2 +-
.../core/operator/filter/BaseColumnFilterOperator.java | 15 +++++++++------
.../core/operator/filter/BitmapBasedFilterOperator.java | 2 +-
.../operator/dociditerators/SortedDocIdIteratorTest.java | 10 ++++------
.../pinot/core/operator/docidsets/AndDocIdSetTest.java | 4 ++--
9 files changed, 29 insertions(+), 30 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/common/BlockDocIdSet.java
b/pinot-core/src/main/java/org/apache/pinot/core/common/BlockDocIdSet.java
index 9fdebcb87df..c960727dd46 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/common/BlockDocIdSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/common/BlockDocIdSet.java
@@ -36,7 +36,8 @@ import org.roaringbitmap.buffer.MutableRoaringBitmap;
/// [org.apache.pinot.core.operator.blocks.FilterBlock].
///
/// A result known to be empty is represented as an [EmptyDocIdSet], so that a
parent can recognize it by type and
-/// short-circuit. The index-based implementations offer a `create` factory
that returns one for an empty input.
+/// short-circuit. The index-based implementations are built only through a
`create` factory, which returns one for
+/// an empty input.
public interface BlockDocIdSet {
/// Returns an iterator of the matching document ids. The document ids
returned from the iterator should be in
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
index b70e756f8b0..b71e2d996e6 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
@@ -23,19 +23,20 @@ import
org.apache.pinot.core.operator.dociditerators.BitmapDocIdIterator;
import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
-public class BitmapDocIdSet implements BlockDocIdSet {
+public final class BitmapDocIdSet implements BlockDocIdSet {
private final BitmapDocIdIterator _iterator;
private final long _numEntriesScannedInFilter;
/// Returns a doc id set over the given documents, or [EmptyDocIdSet] when
there is none.
public static BlockDocIdSet create(ImmutableRoaringBitmap docIds, int
numDocs) {
- return docIds.isEmpty() ? EmptyDocIdSet.unscanned() : new
BitmapDocIdSet(docIds, numDocs);
+ return docIds.isEmpty() ? EmptyDocIdSet.unscanned() : new
BitmapDocIdSet(docIds, numDocs, 0L);
}
/// Returns a doc id set over the given documents found by scanning
`numEntriesScannedInFilter` entries, or
/// [EmptyDocIdSet] when there is none.
public static BlockDocIdSet create(ImmutableRoaringBitmap docIds, int
numDocs, long numEntriesScannedInFilter) {
- return docIds.isEmpty() ? new EmptyDocIdSet(numEntriesScannedInFilter)
+ return docIds.isEmpty()
+ ? new EmptyDocIdSet(numEntriesScannedInFilter)
: new BitmapDocIdSet(docIds, numDocs, numEntriesScannedInFilter);
}
@@ -44,16 +45,12 @@ public class BitmapDocIdSet implements BlockDocIdSet {
return iterator.getDocIds().isEmpty() ? EmptyDocIdSet.unscanned() : new
BitmapDocIdSet(iterator);
}
- public BitmapDocIdSet(ImmutableRoaringBitmap docIds, int numDocs) {
- this(docIds, numDocs, 0L);
- }
-
- public BitmapDocIdSet(ImmutableRoaringBitmap docIds, int numDocs, long
numEntriesScannedInFilter) {
+ private BitmapDocIdSet(ImmutableRoaringBitmap docIds, int numDocs, long
numEntriesScannedInFilter) {
_iterator = new BitmapDocIdIterator(docIds, numDocs);
_numEntriesScannedInFilter = numEntriesScannedInFilter;
}
- public BitmapDocIdSet(BitmapDocIdIterator iterator) {
+ private BitmapDocIdSet(BitmapDocIdIterator iterator) {
_iterator = iterator;
_numEntriesScannedInFilter = 0L;
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/NotDocIdSet.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/NotDocIdSet.java
index c874722e43c..d44b263c74a 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/NotDocIdSet.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/NotDocIdSet.java
@@ -23,7 +23,7 @@ import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.operator.dociditerators.NotDocIdIterator;
-public class NotDocIdSet implements BlockDocIdSet {
+public final class NotDocIdSet implements BlockDocIdSet {
private final BlockDocIdSet _childDocIdSet;
private final int _numDocs;
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/RangelessBitmapDocIdSet.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/RangelessBitmapDocIdSet.java
index 3ff786f5e9d..18d626f49ea 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/RangelessBitmapDocIdSet.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/RangelessBitmapDocIdSet.java
@@ -23,7 +23,7 @@ import
org.apache.pinot.core.operator.dociditerators.RangelessBitmapDocIdIterato
import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
-public class RangelessBitmapDocIdSet implements BlockDocIdSet {
+public final class RangelessBitmapDocIdSet implements BlockDocIdSet {
private final RangelessBitmapDocIdIterator _iterator;
/// Returns a doc id set over the given documents, or [EmptyDocIdSet] when
there is none.
@@ -36,11 +36,11 @@ public class RangelessBitmapDocIdSet implements
BlockDocIdSet {
return iterator.getDocIds().isEmpty() ? EmptyDocIdSet.unscanned() : new
RangelessBitmapDocIdSet(iterator);
}
- public RangelessBitmapDocIdSet(ImmutableRoaringBitmap docIds) {
+ private RangelessBitmapDocIdSet(ImmutableRoaringBitmap docIds) {
this(new RangelessBitmapDocIdIterator(docIds));
}
- public RangelessBitmapDocIdSet(RangelessBitmapDocIdIterator iterator) {
+ private RangelessBitmapDocIdSet(RangelessBitmapDocIdIterator iterator) {
_iterator = iterator;
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SortedDocIdSet.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SortedDocIdSet.java
index 23c1cbc6dfd..fc1537065bf 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SortedDocIdSet.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SortedDocIdSet.java
@@ -34,7 +34,7 @@ public final class SortedDocIdSet implements BlockDocIdSet {
// NOTE: No need to track numDocs because sorted index can only apply to
ImmutableSegment, so the document ids are
// always smaller than numDocs.
- public SortedDocIdSet(List<IntPair> docIdRanges) {
+ private SortedDocIdSet(List<IntPair> docIdRanges) {
_docIdRanges = docIdRanges;
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseColumnFilterOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseColumnFilterOperator.java
index d20f087bd43..16d30ed512a 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseColumnFilterOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseColumnFilterOperator.java
@@ -55,7 +55,7 @@ public abstract class BaseColumnFilterOperator extends
BaseFilterOperator {
@Override
protected BlockDocIdSet getNulls() {
- return _nullBitmap != null ? new BitmapDocIdSet(_nullBitmap, _numDocs) :
EmptyDocIdSet.unscanned();
+ return _nullBitmap != null ? BitmapDocIdSet.create(_nullBitmap, _numDocs)
: EmptyDocIdSet.unscanned();
}
/// The not-false documents are the ones matching the predicate over the
stored values together with the null ones.
@@ -67,9 +67,9 @@ public abstract class BaseColumnFilterOperator extends
BaseFilterOperator {
return matches;
}
if (matches instanceof EmptyDocIdSet) {
- return new BitmapDocIdSet(_nullBitmap, _numDocs,
matches.getNumEntriesScannedInFilter());
+ return BitmapDocIdSet.create(_nullBitmap, _numDocs,
matches.getNumEntriesScannedInFilter());
}
- return new OrDocIdSet(List.of(matches, new BitmapDocIdSet(_nullBitmap,
_numDocs)), _numDocs);
+ return new OrDocIdSet(List.of(matches, BitmapDocIdSet.create(_nullBitmap,
_numDocs)), _numDocs);
}
@Override
@@ -102,8 +102,11 @@ public abstract class BaseColumnFilterOperator extends
BaseFilterOperator {
if (blockDocIdSet instanceof EmptyDocIdSet) {
return blockDocIdSet;
}
- return new AndDocIdSet(List.of(blockDocIdSet,
- new BitmapDocIdSet(ImmutableRoaringBitmap.flip(nullBitmap, 0, (long)
_numDocs), _numDocs)),
- _queryContext.getQueryOptions());
+ BlockDocIdSet nonNulls =
BitmapDocIdSet.create(ImmutableRoaringBitmap.flip(nullBitmap, 0, (long)
_numDocs),
+ _numDocs);
+ if (nonNulls instanceof EmptyDocIdSet) {
+ return new EmptyDocIdSet(blockDocIdSet.getNumEntriesScannedInFilter());
+ }
+ return new AndDocIdSet(List.of(blockDocIdSet, nonNulls),
_queryContext.getQueryOptions());
}
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
index 5fadd719cef..dbc1422369f 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
@@ -80,7 +80,7 @@ public class BitmapBasedFilterOperator extends
BaseFilterOperator {
@Override
protected BlockDocIdSet getNulls() {
- return _nullBitmap != null ? new BitmapDocIdSet(_nullBitmap, _numDocs) :
EmptyDocIdSet.unscanned();
+ return _nullBitmap != null ? BitmapDocIdSet.create(_nullBitmap, _numDocs)
: EmptyDocIdSet.unscanned();
}
@Override
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/SortedDocIdIteratorTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/SortedDocIdIteratorTest.java
index bbeee49443e..5f07173209d 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/SortedDocIdIteratorTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/SortedDocIdIteratorTest.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.pinot.core.common.BlockDocIdIterator;
-import org.apache.pinot.core.operator.docidsets.SortedDocIdSet;
import org.apache.pinot.segment.spi.Constants;
import org.apache.pinot.spi.utils.Pairs;
import org.testng.annotations.Test;
@@ -34,8 +33,7 @@ public class SortedDocIdIteratorTest {
@Test
public void testPairWithSameStartAndEnd() {
- SortedDocIdSet sortedDocIdSet = new SortedDocIdSet(List.of(new
Pairs.IntPair(1, 1)));
- BlockDocIdIterator iterator = sortedDocIdSet.iterator();
+ BlockDocIdIterator iterator = new SortedDocIdIterator(List.of(new
Pairs.IntPair(1, 1)));
List<Integer> result = new ArrayList<>();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
@@ -47,7 +45,7 @@ public class SortedDocIdIteratorTest {
@Test
public void testOneDocIdRange() {
List<Pairs.IntPair> docIdRanges = List.of(new Pairs.IntPair(5, 15));
- SortedDocIdIterator docIdIterator = new
SortedDocIdSet(docIdRanges).iterator();
+ SortedDocIdIterator docIdIterator = new SortedDocIdIterator(docIdRanges);
assertEquals(docIdIterator.next(), 5);
assertEquals(docIdIterator.next(), 6);
assertEquals(docIdIterator.advance(8), 8);
@@ -60,7 +58,7 @@ public class SortedDocIdIteratorTest {
@Test
public void testTwoDocIdRanges() {
List<Pairs.IntPair> docIdRanges = Arrays.asList(new Pairs.IntPair(20, 25),
new Pairs.IntPair(30, 35));
- SortedDocIdIterator docIdIterator = new
SortedDocIdSet(docIdRanges).iterator();
+ SortedDocIdIterator docIdIterator = new SortedDocIdIterator(docIdRanges);
assertEquals(docIdIterator.advance(15), 20);
assertEquals(docIdIterator.next(), 21);
assertEquals(docIdIterator.next(), 22);
@@ -74,7 +72,7 @@ public class SortedDocIdIteratorTest {
public void testDocIdRangesWithSingleDocument() {
List<Pairs.IntPair> docIdRanges = Arrays
.asList(new Pairs.IntPair(3, 3), new Pairs.IntPair(8, 8), new
Pairs.IntPair(15, 15), new Pairs.IntPair(20, 20));
- SortedDocIdIterator docIdIterator = new
SortedDocIdSet(docIdRanges).iterator();
+ SortedDocIdIterator docIdIterator = new SortedDocIdIterator(docIdRanges);
assertEquals(docIdIterator.next(), 3);
assertEquals(docIdIterator.advance(5), 8);
assertEquals(docIdIterator.next(), 15);
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/docidsets/AndDocIdSetTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/docidsets/AndDocIdSetTest.java
index 660596f9fd7..592df5d55f8 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/docidsets/AndDocIdSetTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/docidsets/AndDocIdSetTest.java
@@ -72,7 +72,7 @@ public class AndDocIdSetTest {
List<BlockDocIdSet> docIdSets = new ArrayList<>(numBitmaps);
for (int i = 0; i < numBitmaps; i++) {
copies[i] = bitmaps[i].toMutableRoaringBitmap();
- docIdSets.add(new BitmapDocIdSet(bitmaps[i], NUM_DOCS));
+ docIdSets.add(BitmapDocIdSet.create(bitmaps[i], NUM_DOCS));
}
int[] actualDocIds = collectDocIds(new AndDocIdSet(docIdSets, null));
@@ -90,7 +90,7 @@ public class AndDocIdSetTest {
MutableRoaringBitmap first = MutableRoaringBitmap.bitmapOf(1, 3, 5);
MutableRoaringBitmap second = MutableRoaringBitmap.bitmapOf(0, 2, 4);
List<BlockDocIdSet> docIdSets =
- List.of(new BitmapDocIdSet(first, NUM_DOCS), new
BitmapDocIdSet(second, NUM_DOCS));
+ List.of(BitmapDocIdSet.create(first, NUM_DOCS),
BitmapDocIdSet.create(second, NUM_DOCS));
int[] actualDocIds = collectDocIds(new AndDocIdSet(docIdSets, null));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
