Skip to content

Commit a37fbd2

Browse files
committed
Remove unnecessary aggregator creations
Avoids eager, unnecessary creation of aggregator instances just to determine their output type inside of the HashAggregationOperator constructor.
1 parent 6f313c9 commit a37fbd2

File tree

3 files changed

+8
-5
lines changed

3 files changed

+8
-5
lines changed

core/trino-main/src/main/java/io/trino/operator/HashAggregationOperator.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,6 @@ public OperatorFactory duplicate()
256256
private final FlatHashStrategyCompiler flatHashStrategyCompiler;
257257
private final AggregationMetrics aggregationMetrics = new AggregationMetrics();
258258

259-
private final List<Type> types;
260-
261259
private HashAggregationBuilder aggregationBuilder;
262260
private final LocalMemoryContext memoryContext;
263261
private WorkProcessor<Page> outputPages;
@@ -305,7 +303,6 @@ private HashAggregationOperator(
305303
this.produceDefaultOutput = produceDefaultOutput;
306304
this.expectedGroups = expectedGroups;
307305
this.maxPartialMemory = requireNonNull(maxPartialMemory, "maxPartialMemory is null");
308-
this.types = toTypes(groupByTypes, aggregatorFactories);
309306
this.spillEnabled = spillEnabled;
310307
this.memoryLimitForMerge = requireNonNull(memoryLimitForMerge, "memoryLimitForMerge is null");
311308
this.memoryLimitForMergeWithMemory = requireNonNull(memoryLimitForMergeWithMemory, "memoryLimitForMergeWithMemory is null");
@@ -540,7 +537,7 @@ private Page getGlobalAggregationOutput()
540537
{
541538
// global aggregation output page will only be constructed once,
542539
// so a new PageBuilder is constructed (instead of using PageBuilder.reset)
543-
PageBuilder output = new PageBuilder(globalAggregationGroupIds.size(), types);
540+
PageBuilder output = new PageBuilder(globalAggregationGroupIds.size(), toTypes(groupByTypes, aggregatorFactories));
544541

545542
for (int groupId : globalAggregationGroupIds) {
546543
output.declarePosition();

core/trino-main/src/main/java/io/trino/operator/aggregation/AggregatorFactory.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ public AggregatorFactory(
5858
checkArgument(step.isInputRaw() || inputChannels.size() == 1, "expected 1 input channel for intermediate aggregation");
5959
}
6060

61+
public Type getOutputType()
62+
{
63+
// Note: this must match Aggregator#getType() and GroupedAggregator#getType()
64+
return step.isOutputPartial() ? intermediateType : finalType;
65+
}
66+
6167
public Aggregator createAggregator(AggregationMetrics metrics)
6268
{
6369
Accumulator accumulator;

core/trino-main/src/main/java/io/trino/operator/aggregation/builder/InMemoryHashAggregationBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ public static List<Type> toTypes(List<? extends Type> groupByType, List<Aggregat
349349
ImmutableList.Builder<Type> types = ImmutableList.builderWithExpectedSize(groupByType.size() + factories.size());
350350
types.addAll(groupByType);
351351
for (AggregatorFactory factory : factories) {
352-
types.add(factory.createAggregator(new AggregationMetrics()).getType());
352+
types.add(factory.getOutputType());
353353
}
354354
return types.build();
355355
}

0 commit comments

Comments
 (0)