Jackie-Jiang commented on code in PR #19163:
URL: https://github.com/apache/pinot/pull/19163#discussion_r3723570847
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/converter/stats/MutableColumnStatistics.java:
##########
@@ -112,6 +123,12 @@ public boolean isSorted() {
return false;
}
+ // A single distinct value is always sorted — no scan needed. Cardinality
cannot be 0 here because the segment is
+ // non-empty and every document of a dictionary-encoded column has a dict
id.
+ if (getCardinality() == 1) {
+ return true;
+ }
+
Review Comment:
Good catch, applied to both scan loops in `computeSorted()`.
Noting the placement for anyone reading the thread: this comment is anchored
at the `getCardinality() == 1` short-circuit rather than at the comparison it
describes. The two call sites it applies to are the
`_dictionary.compare(prevDictId, dictId)` calls in the `sortedDocIds` and
natural-docId loops, both now guarded by `dictId != prevDictId`.
Worth adding that the payoff is largest on the `sortedDocIds` path —
iterating in value-sorted order makes equal values adjacent, so a cardinality-C
column does C−1 comparisons instead of numDocs−1. The constant-column case is
already handled by the `getCardinality() == 1` short-circuit above, so what
this saves is low-cardinality columns rather than constant ones.
--
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]
