[ 
https://issues.apache.org/jira/browse/PHOENIX-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14998075#comment-14998075
 ] 

Ankit Singhal commented on PHOENIX-2377:
----------------------------------------

Thanks [~samarthjain] for the review.

I have incorporated all the review comments accept 1st one.

(1) In MaterializedComparableResultIterator, I would get rid of the member 
variable firstElement and the method getFirstElement() since you already have 
that information available when the peek() is called for the first time on the 
iterator. I would also change the compareTo method to :
{code}
+    @Override
+    public int compareTo(MaterializedComparableResultIterator o){
+        return comparator.compare(this.peek(), o.peek());
+
+    }
{code}

I can not change it to peek because peek throws checked SQLException which I 
can not throw from compareTo() method and even cannot handle it there . That's 
why I was using firstElement(now changed to current) to avoid that.

Let me know if it is good to go now.

Regards,
Ankit Singhal

> Use a priority queue in MergeSortResultIterator
> -----------------------------------------------
>
>                 Key: PHOENIX-2377
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-2377
>             Project: Phoenix
>          Issue Type: Sub-task
>            Reporter: Ankit Singhal
>            Priority: Minor
>         Attachments: PHOENIX-2377_v1.patch, PHOENIX-2377_v2.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to