Re: [HACKERS] advanced partition matching algorithm forpartition-wise join - Mailing list pgsql-hackers

> On 12 March 2018 at 06:00, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
> Thanks for the note. Here are rebased patches.

Since I started to look at this patch, I can share few random notes (although
it's not a complete review, I'm in the progress now), maybe it can be helpful.

In `partition_range_bounds_merge`

    + if (!merged)
    +     break;

is a bit redundant I think, because every time `merged` set to false it
followed by break.

At the end same function

    + if (merged)
    +     merged = merge_default_partitions(outer_bi, outer_pmap, outer_mmap,
    +         inner_bi, inner_pmap, inner_mmap,
    +         jointype, &next_index,
    +         &default_index);

Looks like I misunderstand something in the algorithm, but aren't default
partitions were already processed before e.g. here:

    + /*
    +  * Default partition on the outer side forms the default
    +  * partition of the join result.
    +  */

Also in here

    + /* Free any memory we used in this function. */
    + list_free(merged_datums);
    + list_free(merged_indexes);
    + pfree(outer_pmap);
    + pfree(inner_pmap);
    + pfree(outer_mmap);
    + pfree(inner_mmap);

I think `merged_kinds` is missing.

I've noticed that in some places `IS_PARTITIONED_REL` was replaced

    - if (!IS_PARTITIONED_REL(joinrel))
    + if (joinrel->part_scheme == NULL)

but I'm not quite follow why? Is it because `boundinfo` is not available
anymore at this point? If so, maybe it makes sense to update the commentary for
this macro and mention to not use for joinrel.

Also, as for me it would be helpful to have some commentary about this new
`partsupfunc`, what is exactly the purpose of it (but it's maybe just me
missing some obvious things from partitioning context)

    + FmgrInfo   *partsupfunc;