KKcorps commented on code in PR #19039:
URL: https://github.com/apache/pinot/pull/19039#discussion_r3635846272
##########
pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPruner.java:
##########
@@ -97,8 +97,12 @@ public List<IndexSegment> prune(List<IndexSegment> segments,
QueryContext query)
return List.of(segments.get(0));
}
- // Skip pruning segments for upsert table because valid doc index is
equivalent to a filter
- if (segments.get(0).getValidDocIds() != null) {
+ // Skip pruning when a post-selection doc mask makes the raw total-doc
count overstate the surviving rows, which
+ // would otherwise under-return under LIMIT. Both signals are set
uniformly across a table's segments, so the
+ // first segment suffices: upsert's valid-doc index, or an
externally-supplied deleted-doc set. Skipping keeps
+ // results correct -- every segment is still scanned and the mask applied
during the scan.
+ IndexSegment firstSegment = segments.get(0);
+ if (firstSegment.getValidDocIds() != null ||
firstSegment.hasDeletedDocIds()) {
Review Comment:
Checking for every segment is going to be too expensive. If we need to go on
that path then we should just decide based on table config if pruning needs to
be skipped or not
Also, this similar issue can happen in upserts as well where only recent
segment's PKs were upserts but old one were not and had few rows.
--
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]
