This is an automated email from the ASF dual-hosted git repository.
xiangfu0 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 474efb1c1bf Return ImmutableRoaringBitmap from
JsonIndexReader#getMatchingDocIds to avoid copying the JSON_MATCH posting
(#18694)
474efb1c1bf is described below
commit 474efb1c1bf881b6cdbc27787e31a4942737ae85
Author: Xiang Fu <[email protected]>
AuthorDate: Sun Jun 7 13:07:40 2026 -0700
Return ImmutableRoaringBitmap from JsonIndexReader#getMatchingDocIds to
avoid copying the JSON_MATCH posting (#18694)
JsonMatchFilterOperator only iterates the matching-docId bitmap
(BitmapDocIdSet /
BitmapCollection / getCardinality) within the segment's acquired lifetime
and never
mutates it, yet JsonIndexReader#getMatchingDocIds was contractually
required to return a
caller-owned MutableRoaringBitmap. A reader whose posting list is backed by
read-only /
memory-mapped storage therefore had to copy the posting on every JSON_MATCH
call.
Narrow the JsonIndexReader#getMatchingDocIds overloads to return
ImmutableRoaringBitmap and
document the read-only / borrowed-bitmap contract: the returned bitmap may
be a read-only
view backed by the index's underlying (possibly memory-mapped) storage, is
valid only while
the segment/index is held open, and must not be mutated. Callers that need
to mutate must
copy via toMutableRoaringBitmap().
This brings JSON_MATCH in line with the inverted index
(InvertedIndexReader#getDocIds already
returns ImmutableRoaringBitmap). The stock readers
(ImmutableJsonIndexReader,
MutableJsonIndexImpl) keep returning a freshly-allocated
MutableRoaringBitmap via covariant
override, so behavior is unchanged; an mmap-backed reader may override to
return the posting
list directly and avoid the per-call copy.
Backward-incompatible SPI change: the getMatchingDocIds return type changes
from
MutableRoaringBitmap to ImmutableRoaringBitmap. Downstream implementers are
unaffected
(covariant return); downstream callers that assigned the result to
MutableRoaringBitmap or
mutated it must adjust (copy via toMutableRoaringBitmap()).
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
.../operator/filter/JsonMatchFilterOperator.java | 3 +++
.../segment/local/segment/index/JsonIndexTest.java | 10 ++++----
.../segment/spi/index/reader/JsonIndexReader.java | 28 +++++++++++++---------
3 files changed, 25 insertions(+), 16 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java
index ea6986556ec..01ff5435273 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java
@@ -128,6 +128,9 @@ public class JsonMatchFilterOperator extends
BaseFilterOperator {
}
private ImmutableRoaringBitmap getMatchingDocIdBitmap() {
+ // getMatchingDocIds may return a read-only bitmap backed by the index's
(possibly memory-mapped) storage. This
+ // operator only iterates it (BitmapDocIdSet / BitmapCollection /
getCardinality) within the segment's acquired
+ // lifetime and never mutates it, so a borrowed posting list can be
consumed directly without a copy.
if (_predicate != null) {
return _jsonIndex.getMatchingDocIds(_predicate.getValue(),
_predicate.getCountPredicate());
} else {
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java
index b576a443942..ec2f0307de7 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/JsonIndexTest.java
@@ -45,7 +45,7 @@ import org.apache.pinot.spi.config.table.FieldConfig;
import org.apache.pinot.spi.config.table.JsonIndexConfig;
import org.apache.pinot.spi.utils.JsonUtils;
import org.roaringbitmap.RoaringBitmap;
-import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -269,7 +269,7 @@ public class JsonIndexTest implements
PinotBuffersAfterMethodCheckRule {
}
private void assertDocIds(JsonIndexReader indexReader, String filter, int[]
expected) {
- MutableRoaringBitmap matchingDocIds = getMatchingDocIds(indexReader,
filter);
+ ImmutableRoaringBitmap matchingDocIds = getMatchingDocIds(indexReader,
filter);
try {
assertEquals(matchingDocIds.toArray(), expected);
} catch (AssertionError ae) {
@@ -321,7 +321,7 @@ public class JsonIndexTest implements
PinotBuffersAfterMethodCheckRule {
assertDocIds(reader, "name = 'adam-100000' AND \"addresses[*].street\"
= 'us-100001'", empty());
- MutableRoaringBitmap matchingDocIds = getMatchingDocIds(reader, "name
!= 'adam-100000'");
+ ImmutableRoaringBitmap matchingDocIds = getMatchingDocIds(reader,
"name != 'adam-100000'");
try {
assertEquals(matchingDocIds.getCardinality(), 123_455);
} catch (AssertionError ae) {
@@ -360,7 +360,7 @@ public class JsonIndexTest implements
PinotBuffersAfterMethodCheckRule {
JsonIndexReader[] indexReaders = new JsonIndexReader[]{onHeapReader,
offHeapReader, mutableIndex};
for (JsonIndexReader reader : indexReaders) {
- MutableRoaringBitmap docIds = getMatchingDocIds(reader,
"key1='value1'");
+ ImmutableRoaringBitmap docIds = getMatchingDocIds(reader,
"key1='value1'");
assertEquals(docIds.toArray(), ids(0));
docIds = getMatchingDocIds(reader, "key2='longValue2'");
@@ -422,7 +422,7 @@ public class JsonIndexTest implements
PinotBuffersAfterMethodCheckRule {
}
}
- private MutableRoaringBitmap getMatchingDocIds(JsonIndexReader indexReader,
String filter) {
+ private ImmutableRoaringBitmap getMatchingDocIds(JsonIndexReader
indexReader, String filter) {
return indexReader.getMatchingDocIds(filter);
}
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/JsonIndexReader.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/JsonIndexReader.java
index 06a095b4e20..355c2e2e07e 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/JsonIndexReader.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/JsonIndexReader.java
@@ -23,7 +23,7 @@ import java.util.Set;
import javax.annotation.Nullable;
import org.apache.pinot.segment.spi.index.IndexReader;
import org.roaringbitmap.RoaringBitmap;
-import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
/**
@@ -31,18 +31,24 @@ import org.roaringbitmap.buffer.MutableRoaringBitmap;
*/
public interface JsonIndexReader extends IndexReader {
- MutableRoaringBitmap getMatchingDocIds(String filterString);
+ /// Returns the matching document ids for the given filter string.
+ ///
+ /// The returned bitmap MUST be treated as read-only. It may be a borrowed
view backed by the index's underlying
+ /// (possibly memory-mapped) storage, so it is valid only while the
segment/index is held open and must not be
+ /// mutated. Callers that need to mutate the result should first copy it via
+ /// [ImmutableRoaringBitmap#toMutableRoaringBitmap()]. Stock readers still
return a freshly-allocated bitmap; an
+ /// mmap-backed reader may return the posting list directly to avoid a
per-call copy.
+ ImmutableRoaringBitmap getMatchingDocIds(String filterString);
- /**
- * Returns the matching document ids for the given filter Context.
- */
- MutableRoaringBitmap getMatchingDocIds(Object filterCtx);
+ /// Returns the matching document ids for the given filter context. See
[#getMatchingDocIds(String)] for the
+ /// read-only borrowed-bitmap contract.
+ ImmutableRoaringBitmap getMatchingDocIds(Object filterCtx);
- /**
- * Returns the matching document ids for the given filter.
- * @param flatDocCountFilter predicate used to filter results based on
number of matched flattened document
- */
- default MutableRoaringBitmap getMatchingDocIds(String documentFilter, String
flatDocCountFilter) {
+ /// Returns the matching document ids for the given filter. See
[#getMatchingDocIds(String)] for the read-only
+ /// borrowed-bitmap contract.
+ ///
+ /// @param flatDocCountFilter predicate used to filter results based on
number of matched flattened document
+ default ImmutableRoaringBitmap getMatchingDocIds(String documentFilter,
String flatDocCountFilter) {
return getMatchingDocIds(documentFilter);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
