Re: [HACKERS] advanced partition matching algorithm for partition-wise join

Thanks Dmitry for your review.

On Mon, May 7, 2018 at 1:06 AM, Dmitry Dolgov <9erthalion6@gmail.com> wrote:
>
> Thank you. Unfortunately, there are already few more conflicts since last time,
> could you rebase again?

Done.

>
> 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?

I agree. It's misleading. Removed.

>
> 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?

By NULL partition, I mean a list partition which holds NULL values.
Added that clarification in the prologue of merge_null_partitions().

>
> 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?

I have added handle_missing_partition() for the same purpose. But I
didn't use it in merge_null_partitions(), which I have done in the
attached patches. Now the number of such instances are just at two
places one in merge_default_partitions() and the other in
handle_missing_partition(). But they are different enough not to
extract into a common function.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company