Horton

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

Dmitry Dolgov <9erthalion6@gmail.com>

View thread
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Antonin Houska <ah@cybertec.at>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-03-21T23:02:02Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Suppress unused-variable warning.

  2. Allow partitionwise joins in more cases.

  3. Avoid crash in partitionwise join planning under GEQO.

  4. Disable support for partitionwise joins in problematic cases.

  5. Add plan_cache_mode setting

  6. Add test for partitionwise join involving default partition.

  7. Revise API for partition_rbound_cmp/partition_rbound_datum_cmp.

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