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