ulysses-you commented on code in PR #57742:
URL: https://github.com/apache/spark/pull/57742#discussion_r3735235121
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala:
##########
@@ -626,13 +788,27 @@ case class HashAggregateExec(
| long $beforeAgg = System.nanoTime();
| $doAggFuncName(partitionIndex);
| $aggTime.add((System.nanoTime() - $beforeAgg) / $NANOS_PER_MILLIS);
+ | $adaptiveStopCheck
|}
- |// output the result
- |$outputFromFastHashMap
- |$outputFromRegularHashMap
+ |$adaptiveResumeBuild
+ |$adaptiveFinalOutput
""".stripMargin
}
+ // Blocking operators normally suppress the child's `shouldStop()` check
because they buffer all
+ // output. With adaptive partial aggregation, pass-through rows are appended
to the output buffer
+ // while consuming child input, so the stop check must be re-enabled to keep
the buffer bounded.
+ override def needStopCheck: Boolean = adaptivePartialAggEnabled
Review Comment:
You are right, and "Addressed" was my mistake -- the only change was a
results-unchanged test, which does not cover buffering at all. I have updated
the comment at `:811` in 87937dbd6d0 as you asked: it no longer claims
`needStopCheck` bounds the buffer, and it names the case where it does not,
along with the point you emphasised -- those rows used to land in the spillable
aggregation map and now land in `BufferedRowIterator.currentRows`, which is not
spillable, so the failure mode is heap rather than spill I/O.
On the measurement itself I still cannot reproduce the 998. I instrumented
`currentRows` directly -- rebuild the generated iterator the way
`WholeStageCodegenExec.doExecute` does, run `processNext()` once, read
`currentRows.size()`. On `explode(sequence(1, 1000))` -> partial aggregate with
`minRows=2`:
```
GEN adaptive=false buffered=1 bypassed=0
GEN adaptive=true buffered=1 bypassed=998
```
The probe distinguishes `bypassed=0` from `bypassed=998`, so the bypass is
firing; the buffer still holds one row.
--
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]
