> On 3 April 2018 at 15:34, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
> On Fri, Mar 30, 2018 at 7:36 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>
>> I am working on commenting portions of the code to make it more clear
>> and readable. Will update the patches with the comments soon, mostly
>> this Monday.
>>
>
> Here's set of patches rebased on the latest head. I have added a lot
> of comments and revised a lot of code to avoid duplication, causing a
> net reduction in the number of lines.
Thank you. Unfortunately, there are already few more conflicts since last time,
could you rebase again?
In the meantime, I'm a bit confused by `merge_null_partitions` and why
partitions with NULL values should be treated separately? It became even more
confusing when I looked at where this functions is used:
/* If merge is unsuccessful, bail out without any further processing. */
if (merged)
merged = merge_null_partitions(outer_bi, outer_pmap, outer_mmap,
inner_bi, inner_pmap, inner_mmap,
jointype, &next_index, &null_index,
&default_index);
Is this commentary correct?
Also, I don't see anywhere in PostgreSQL itself or in this patch a definition
of the term "NULL partition", can you add it, just to make things clear?
Another question, I see this pattern a lot here when there is a code like:
if (!outer_has_something && inner_has_something)
{
// some logic
}
else if (outer_has_something && !inner_has_something)
{
// some logic symmetric to what we have above
}
By symmetric I mean that the code is more or less the same, just "inner"
variables were changed to "outer" (and required types of joins are opposite).
Is it feasible to actually use completely the same logic, and just change
"inner"/"outer" variables instead?