scottlan0113 commented on code in PR #19438:
URL: https://github.com/apache/pinot/pull/19438#discussion_r3919809691


##########
pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java:
##########
@@ -158,42 +158,84 @@ private BaseCombineOperator getCombineOperator() {
           // LIMIT is at or below the threshold there is no memory to save, so 
the short-circuit wins instead.
           && _queryContext.getLimit() > distinctFlushThreshold
           && !leafReturnsFinalResult) {
-        return new StreamingDistinctCombineOperator(operators, _queryContext, 
_executorService,
-            distinctFlushThreshold);
+        return createStreamingDistinctCombineOperator(operators);
       }
     }
     if (QueryContextUtils.isAggregationQuery(_queryContext)) {
       if (_queryContext.getGroupByExpressions() == null) {
         // Aggregation only
-        return new AggregationCombineOperator(operators, _queryContext, 
_executorService);
+        return createAggregationCombineOperator(operators);
       } else {
-        // Sorted aggregation group-by, when safeTrim and limit is not too 
large
-        if (_queryContext.shouldSortAggregateUnderSafeTrim()) {
-          if (operators.size() < 
_queryContext.getSortAggregateSequentialCombineNumSegmentsThreshold()) {
-            return new SequentialSortedGroupByCombineOperator(operators, 
_queryContext, _executorService);
-          }
-          return new SortedGroupByCombineOperator(operators, _queryContext, 
_executorService);
-        }
-        // Aggregation group-by
-        return new GroupByCombineOperator(operators, _queryContext, 
_executorService);
+        return createGroupByCombineOperator(operators);
       }
     } else if (QueryContextUtils.isSelectionQuery(_queryContext)) {
-      if (_queryContext.getLimit() == 0 || 
_queryContext.getOrderByExpressions() == null) {
-        // Selection only
-        return new SelectionOnlyCombineOperator(operators, _queryContext, 
_executorService);
-      } else {
-        // Selection order-by
-        List<OrderByExpressionContext> orderByExpressions = 
_queryContext.getOrderByExpressions();
-        assert orderByExpressions != null;
-        if (orderByExpressions.get(0).getExpression().getType() == 
ExpressionContext.Type.IDENTIFIER) {
-          return new 
MinMaxValueBasedSelectionOrderByCombineOperator(operators, _queryContext, 
_executorService);
-        } else {
-          return new SelectionOrderByCombineOperator(operators, _queryContext, 
_executorService);
-        }
-      }
+      return createSelectionCombineOperator(operators);
     } else {
       assert QueryContextUtils.isDistinctQuery(_queryContext);
-      return new DistinctCombineOperator(operators, _queryContext, 
_executorService);
+      return createDistinctCombineOperator(operators);
+    }
+  }
+
+  /// Returns the combine operator for an aggregation query with no GROUP BY.
+  protected BaseCombineOperator 
createAggregationCombineOperator(List<Operator> operators) {

Review Comment:
   Reordered to select / aggregate / group-by / distinct. The streaming ones 
follow the same order.



-- 
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]

Reply via email to