Jackie-Jiang commented on code in PR #10058:
URL: https://github.com/apache/pinot/pull/10058#discussion_r1061936474
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java:
##########
@@ -190,6 +194,13 @@ public List<Operator> callJob() {
return new SelectionOnlyCombineOperator(operators, _queryContext,
_executorService);
} else {
// Selection order-by
+ if (QueryContextUtils.isMinMaxBasedSelectionOrderBy(_queryContext)) {
Review Comment:
IMO no need to put it into `QueryContextUtils`. This logic should be within
the combine plan
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/SelectionOrderByCombineOperator.java:
##########
@@ -66,25 +64,12 @@ public String toExplainString() {
* {@inheritDoc}
*
* <p> Execute query on one or more segments in a single thread, and store
multiple intermediate result blocks
- * into BlockingQueue. Try to use
- * {@link
org.apache.pinot.core.operator.combine.MinMaxValueBasedSelectionOrderByCombineOperator}
first, which
- * will skip processing some segments based on the column min/max value.
Otherwise fall back to the default combine
- * (process all segments).
+ * into BlockingQueue.
*/
@Override
protected BaseResultsBlock getNextBlock() {
Review Comment:
Let's remove this override completely
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java:
##########
@@ -190,6 +194,13 @@ public List<Operator> callJob() {
return new SelectionOnlyCombineOperator(operators, _queryContext,
_executorService);
} else {
// Selection order-by
+ if (QueryContextUtils.isMinMaxBasedSelectionOrderBy(_queryContext)) {
+ try {
Review Comment:
We don't need this try-catch as the constructor won't throw exception
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
