siknezevic commented on a change in pull request #27246:
URL: https://github.com/apache/spark/pull/27246#discussion_r436226152
##########
File path:
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
##########
@@ -703,6 +702,7 @@ public boolean hasNext() {
@Override
public void loadNext() throws IOException {
Review comment:
Thank you for the suggestion. I will start my feedback with my
understanding of your suggestion. Your idea is to reuse the code. You are
proposing that content of loadNext() method be replaced by invocation of method
hasNext().
I would kindly suggest if you could look loadNext() method again. Perhaps
you missed the fact that last call inside of loadNext() is
“current.loadNext()”. The last call of hasNext() method is “current.hasNext()”.
If we follow your recommendation, then we would change behavior of loadNext()
method. I think that loadNext() moves to the next element, and hasNext() check
if next element exists but it does not move to the next element. I believe that
we cannot reuse the code here.
Could you please let me know if my reasoning make sense. Thank you for your
time.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
