xiangfu0 commented on code in PR #18891:
URL: https://github.com/apache/pinot/pull/18891#discussion_r3532886919
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/json/ImmutableJsonIndexReader.java:
##########
@@ -783,34 +797,76 @@ public String[][] getValuesMV(int[] docIds, int length,
Map<String, RoaringBitma
@Override
public String[] getValuesSV(int[] docIds, int length, Map<String,
RoaringBitmap> valueToMatchingDocs,
boolean isFlattenedDocIds) {
- Int2ObjectOpenHashMap<String> docIdToValues = new
Int2ObjectOpenHashMap<>(docIds.length);
- RoaringBitmap docIdMask = RoaringBitmap.bitmapOf(docIds);
-
- for (Map.Entry<String, RoaringBitmap> entry :
valueToMatchingDocs.entrySet()) {
- String value = entry.getKey();
- RoaringBitmap matchingDocIds = entry.getValue();
- if (isFlattenedDocIds) {
- matchingDocIds.forEach((IntConsumer) flattenedDocId -> {
+ String[] values = new String[length];
+ if (length == 0) {
+ return values;
+ }
+
+ if (isFlattenedDocIds) {
+ // Flattened doc ids must be converted to real doc ids before they can
be matched against the block's doc ids,
+ // so we still build a docId -> value lookup keyed by the converted doc
id.
+ Int2ObjectOpenHashMap<String> docIdToValues = new
Int2ObjectOpenHashMap<>(length);
+ RoaringBitmap docIdMask = RoaringBitmap.bitmapOf(docIds);
+ for (Map.Entry<String, RoaringBitmap> entry :
valueToMatchingDocs.entrySet()) {
+ String value = entry.getKey();
+ entry.getValue().forEach((IntConsumer) flattenedDocId -> {
int docId = getDocId(flattenedDocId);
if (docIdMask.contains(docId)) {
docIdToValues.put(docId, value);
}
});
- } else {
- RoaringBitmap intersection = RoaringBitmap.and(matchingDocIds,
docIdMask);
- if (intersection.isEmpty()) {
- continue;
+ }
+ for (int i = 0; i < length; i++) {
+ values[i] = docIdToValues.get(docIds[i]);
+ }
+ return values;
+ }
+
+ // Non-flattened path (the only path used by jsonExtractIndex SV): docIds
are real, ascending-sorted doc ids for
+ // the current block, and for a single-value path each doc belongs to at
most one value's posting list. We can
+ // therefore scatter values directly, iterating each value's posting list
only over the block's [lo, hi] doc id
+ // range with a peekable iterator. This avoids allocating a per-block doc
id mask bitmap, a RoaringBitmap.and per
+ // distinct value, and an intermediate docId -> value hash map.
+ int lo = docIds[0];
Review Comment:
This assumes the block docIds are ascending, but selection queries can run
the TransformOperator over DESC doc-id blocks when ordering by a sorted column
via withOrder(DESC). In that case lo > hi, so the range scan never fills
matching values and jsonExtractIndex can return null/default or throw for rows
that do have the path. Keep the order-insensitive mask path unless ascending is
guaranteed, or detect non-ascending blocks and scatter with the true min/max
range.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
