Ashutosh Chauhan updated PIG-926:

    Attachment: mj_phase2_1.patch

Thanks for the review, Pradeep. My response is inline.

Is there a reason MergeJoinIndexer is a subclass of RandomSampleLoader? It does 
not use numSamples or skipInterval and has its own getNext() - is there any 
benefit to inheritance?
>> Ya, its better not to have that dependency. Now, MergeJoinIndexer doesn't 
>> subclass RandomSampleLoader, instead implements LoadFunc itself.

I see that readObject() method has been removed - is this no longer needed?
>> Ya, its not required anymore.

Also for the case where right pipeline returns POStatus.STATUS_ERR, the code 
currently ignores it and continues. ..
>> I was following the usual practice of passing down ERR in the pipeline. But 
>> in this case it seems to throw ExecException right there. Updated the patch 
>> with it.

Updated the patch with these changes.
FindBugs warning is harmless and can be ignored.

> Merge-Join phase 2
> ------------------
>                 Key: PIG-926
>                 URL: https://issues.apache.org/jira/browse/PIG-926
>             Project: Pig
>          Issue Type: Improvement
>          Components: impl
>            Reporter: Ashutosh Chauhan
>            Assignee: Ashutosh Chauhan
>            Priority: Minor
>         Attachments: mj_phase2_1.patch
> This jira is created to keep track of phase-2 work for MergeJoin. Various 
> limitations exist in phase-1 for Merge Join which are listed on: 
> http://wiki.apache.org/pig/PigMergeJoin Those will be addressed here.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to