gianm commented on a change in pull request #10053:
URL: https://github.com/apache/druid/pull/10053#discussion_r442988947
##########
File path:
processing/src/main/java/org/apache/druid/query/topn/TopNQueryEngine.java
##########
@@ -158,6 +156,40 @@ private TopNMapFn getMapFn(
return new TopNMapFn(query, topNAlgorithm);
}
+ /**
+ * {@link PooledTopNAlgorithm} (and {@link
AggregateTopNMetricFirstAlgorithm} which utilizes the pooled
+ * algorithm) are optimized off-heap algorithms for aggregating dictionary
encoded string columns. These algorithms
+ * rely on dictionary ids being unique so to aggregate on the dictionary ids
directly and defer
+ * {@link org.apache.druid.segment.DimensionSelector#lookupName(int)} until
as late as possible in query processing.
+ *
+ * When these conditions are not true, we have an on-heap fall-back
algorithm, the {@link HeapBasedTopNAlgorithm}
+ * (and {@link TimeExtractionTopNAlgorithm} for a specialized form for long
columns) which aggregates on values of
+ * selectors.
+ */
+ private static boolean requiresHeapAlgorithm(
Review comment:
I think this method would be easier to read if it was called
`canUsePooledAlgorithm` and the checks were all flipped. This is because IMO it
makes sense to view the heap algorithm as the base case, and the pooled
algorithm as a special case, meaning the logic should be "can we do the special
case".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
