stoty commented on a change in pull request #1114:
URL: https://github.com/apache/phoenix/pull/1114#discussion_r565101726
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/iterate/RoundRobinResultIterator.java
##########
@@ -114,6 +114,7 @@ public Tuple next() throws SQLException {
index = (index + 1) % size;
}
}
+ close();
Review comment:
> When RRIterator#peek() (next result) is null, we close the iterator.
But if we don't get any next result after looping through all RRIterators, we
should close() here, that's the plan right?
Yes. When all iterators set numScannersCacheExhausted , and we're at the
end, getIterators() will return and empty collection, as close() is skipped.
> Although this looks good, curious if caller can implement the logic of
calling RRResultIterator#close() if RRResultIterator#next() returns null. But
if caller logic turns out to be more complicated, sure we should go this route
I believe.
This is is the pattern everywhere in the code. This is also less
error-prone, and encapsulates the logic within the class, I see no reason to
change this, either.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
