[GitHub] [spark] siknezevic commented on a change in pull request #27246: [SPARK-30536][CORE][SQL] Sort-merge join operator spilling performance improvements

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org