xiangfu0 commented on code in PR #20108:
URL: https://github.com/apache/pinot/pull/20108#discussion_r4430179316
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/array/SumArrayLongAggregationFunction.java:
##########
@@ -106,64 +100,69 @@ public void aggregateGroupByMV(int length, int[][]
groupKeysArray, GroupByResult
}
private void setGroupByResult(GroupByResultHolder groupByResultHolder,
long[] values, int groupKey) {
- LongArrayList sumList = groupByResultHolder.getResult(groupKey);
- if (sumList == null) {
- sumList = new LongArrayList();
- groupByResultHolder.setValueForKey(groupKey, sumList);
+ long[] sums = groupByResultHolder.getResult(groupKey);
+ long[] newSums = add(sums, values);
+ if (newSums != sums) {
+ groupByResultHolder.setValueForKey(groupKey, newSums);
}
- aggregateMerge(values, sumList);
}
- private void aggregateMerge(long[] values, LongArrayList sumList) {
- for (int j = sumList.size(); j < values.length; j++) {
- sumList.add(0L);
+ /// Adds the values to the sums and returns the sums, which is a new array
when there are none yet or the values are
+ /// longer.
+ private static long[] add(@Nullable long[] sums, long[] values) {
+ if (sums == null) {
+ return values.clone();
+ }
+ if (sums.length < values.length) {
Review Comment:
MINOR: The regrow path (a longer array arriving after a shorter one, in
`aggregate` and group-by) is new logic, but every SUM_ARRAY test uses arrays of
one length. I checked it locally with rows `{1}, {10,20,30}, {100,200}` over
two blocks (agg plus group-by SV/MV), and it's correct for both the long and
double variants. A small test like that in `ValueAggregationNullHandlingTest`
would keep it covered.
--
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]
