mcvsubbu commented on a change in pull request #5259: Support Aggregation
functions with multiple arguments.
URL: https://github.com/apache/incubator-pinot/pull/5259#discussion_r409722673
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/DefaultAggregationExecutor.java
##########
@@ -76,22 +78,22 @@ public void aggregate(TransformBlock transformBlock) {
AggregationResultHolder resultHolder = _resultHolders[i];
if (function.getType() == AggregationFunctionType.COUNT) {
// handle count(*) function
- function.aggregate(length, resultHolder);
+ function.aggregate(length, resultHolder, Collections.emptyMap());
} else if (function.getType() == AggregationFunctionType.DISTINCT) {
// handle distinct (col1, col2..) function
// unlike other aggregate functions, distinct can work on multiple
columns
// so we get all the projected columns (ProjectionBlockValSet) from
TransformBlock
// for each column and then pass them over to distinct function since
the uniqueness
// will be determined across tuples and not on a per column basis
- BlockValSet[] blockValSets = new BlockValSet[_expressions.length];
+ Map<String, BlockValSet> blockValSetMap = new HashMap<>();
for (int j = 0; j < _expressions.length; j++) {
- blockValSets[j] = transformBlock.getBlockValueSet(_expressions[j]);
+ blockValSetMap.put(_expressions[j].toString(),
transformBlock.getBlockValueSet(_expressions[j]));
}
- DistinctAggregationFunction distinctFunction =
(DistinctAggregationFunction) function;
- distinctFunction.aggregate(length, resultHolder, blockValSets);
+ function.aggregate(length, resultHolder, blockValSetMap);
} else {
// handle rest of the aggregate functions -- sum, min, max etc
- function.aggregate(length, resultHolder,
transformBlock.getBlockValueSet(_expressions[i]));
+ function.aggregate(length, resultHolder,
Review comment:
wouldnt the code from line 88 to 92 hold here as well? Can we move the call
to `function.aggregate()` below, initializing the map differently (or, are we
optimizing the map creation here with `Collections.singletonMap()`)
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
