agavra commented on code in PR #9767:
URL: https://github.com/apache/pinot/pull/9767#discussion_r1017337713


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AggregateOperator.java:
##########
@@ -53,17 +56,33 @@
  * If the input is single value, the output type will be input type. 
Otherwise, the output type will be double.
  */
 public class AggregateOperator extends BaseOperator<TransferableBlock> {
+
+  interface Merger extends BiFunction<Object, Object, Object> {
+  }
+
   private static final String EXPLAIN_NAME = "AGGREGATE_OPERATOR";
+  private static final Map<String, Merger> MERGERS = ImmutableMap

Review Comment:
   this lets us (1) inject mergers for testing and (2) throw in the constructor 
instead of waiting until a block is merged if there's an unknown aggregate 
function.



##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AggregateOperator.java:
##########
@@ -123,11 +138,14 @@ public String toExplainString() {
   @Override
   protected TransferableBlock getNextBlock() {
     try {
-      if (!_readyToConstruct) {
-        consumeInputBlocks();
+      if (!_readyToConstruct && !consumeInputBlocks()) {

Review Comment:
   made a minor change here to continue to producing the block instead of 
returning a no-op block when we read EOS from the input source



##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AggregateOperator.java:
##########
@@ -228,4 +240,38 @@ private static Key extraRowKey(Object[] row, 
List<RexExpression> groupSet) {
     }
     return new Key(keyElements);
   }
+
+  private static class Holder {

Review Comment:
   minor refactor so we don't need to maintain three different arrays 
(including `Map[]`, which is generally considered bad practice)



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

Reply via email to