Re: [HACKERS] Proposal : Parallel Merge Join

On Tue, Dec 13, 2016 at 10:04 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> On Tue, Dec 13, 2016 at 6:42 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>> This patch is hard to read because it is reorganizing a bunch of code
>>> as well as adding new functionality.  Perhaps you could separate it
>>> into two patches, one with the refactoring and then the other with the
>>> new functionality.
>>
>> Okay, I can do that.
>
> I have created two patches as per the suggestion.
>
> 1. mergejoin_refactoring_v2.patch --> Move functionality of
> considering various merge join path into new function.
> 2. parallel_mergejoin_v2.patch -> This adds the functionality of
> supporting partial mergejoin paths. This will apply on top of
> mergejoin_refactoring_v2.patch.

Committed the refactoring patch with some mild cosmetic adjustments.

As to the second patch:

+        if (jointype == JOIN_UNIQUE_INNER)
+            jointype = JOIN_INNER;

Isn't this dead code?  save_jointype might that value, but jointype won't.

Apart from that and some cosmetic issues it looks basically OK to me
on a first read-through.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company