Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/938#discussion_r137939427
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
---
@@ -1178,20 +1273,38 @@ private void checkGroupAndAggrValues(int
incomingRowIdx) {
hashCode >>>= bitsInMask;
HashTable.PutStatus putStatus = null;
long allocatedBeforeHTput = allocator.getAllocatedMemory();
-
// ==========================================
// Insert the key columns into the hash table
// ==========================================
+ boolean noReserveMem = reserveValueBatchMemory == 0;
try {
+ if ( noReserveMem && canSpill ) { throw new
RetryAfterSpillException();} // proactive spill, skip put()
+
putStatus = htables[currentPartition].put(incomingRowIdx,
htIdxHolder, hashCode);
+
+ } catch (RetryAfterSpillException re) {
--- End diff --
See above. Should be a checked exception declared by `put()`.
Also, why do we need to throw an exception before calling `put` only to
catch it a couple of lines later?
If the spill code was in a method, rather than just inline, seems we could
do:
```
if ( noReserveMem && canSpill ) { doSpill(); }
try {
...put(...)
} catch (...) {
doSpill();
}
```
---
