wirybeaver commented on code in PR #19469:
URL: https://github.com/apache/pinot/pull/19469#discussion_r4102820183


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AggregateOperator.java:
##########
@@ -268,12 +367,87 @@ private MseBlock.Eos consumeGroupBy() {
     MseBlock block = _input.nextBlock();
     while (block.isData()) {
       _groupByExecutor.processBlock((MseBlock.Data) block);
+      if (_spillThreshold > 0 && _groupByExecutor.getNumGroups() >= 
_spillThreshold) {
+        spillCurrentGroups();
+      }
       checkTerminationAndSampleUsage();
       block = _input.nextBlock();
     }
     return (MseBlock.Eos) block;
   }
 
+  private void spillCurrentGroups() {
+    assert _groupByExecutor != null;
+    if (_groupByExecutor.getNumGroups() == 0) {
+      return;
+    }
+    if (_spillManager == null) {
+      _spillManager =
+          new AggregationSpillManager(_spillPartitions, _groupKeyIds.length, 
getSpillSchema(), _aggFunctions);
+      _spillDirectory = _spillManager.getSpillDirectory();
+    }
+    AggregationSpillManager.SpillResult spillResult =
+        _spillManager.spill(_groupByExecutor.getIntermediateResultIterator());
+    _statMap.merge(StatKey.SPILL_COUNT, 1L);
+    _statMap.merge(StatKey.SPILLED_ROWS, spillResult.getRows());
+    _statMap.merge(StatKey.SPILLED_BYTES, spillResult.getBytes());
+    _groupByExecutor = newInputGroupByExecutor();
+  }
+
+  private DataSchema getSpillSchema() {
+    String[] columnNames = _resultSchema.getColumnNames().clone();
+    ColumnDataType[] columnDataTypes = 
_resultSchema.getColumnDataTypes().clone();
+    int numKeys = _groupKeyIds.length;
+    for (int i = 0; i < _aggFunctions.length; i++) {
+      columnDataTypes[numKeys + i] = _aggFunctions[i].getType() == 
AggregationFunctionType.ANYVALUE
+          ? ColumnDataType.OBJECT : 
_aggFunctions[i].getIntermediateResultColumnType();
+    }
+    return new DataSchema(columnNames, columnDataTypes);
+  }
+
+  @Nullable
+  private MseBlock.Data restoreSpillPartition(int partitionId) {
+    assert _spillManager != null;
+    if (!_spillManager.hasPartition(partitionId)) {
+      return null;
+    }
+    MultistageGroupByExecutor executor = newSpillMergeGroupByExecutor();
+    _spillManager.consumePartition(partitionId, block -> {
+      executor.processSpillBlock(block);
+      if (executor.getNumGroups() > _spillThreshold) {
+        throw QueryErrorCode.SERVER_RESOURCE_LIMIT_EXCEEDED.asException(

Review Comment:
   Done in 2e40e9402d3069aa86e22cf7e6858b904795b57f. Restore now admits groups 
up to the existing numGroupsLimit rather than throwing when a hash partition 
exceeds the lower spill trigger. The established 
truncate/NUM_GROUPS_LIMIT_REACHED behavior remains the default; 
errorOnNumGroupsLimit still raises an error. The PR description now documents 
this behavior.



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

Reply via email to