Horton

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

Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>

View thread
From: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, amul sul <sulamul@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>, Dmitry Dolgov <9erthalion6@gmail.com>, Antonin Houska <ah@cybertec.at>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-04-01T16:51:17Z
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.

Attachments

On Thu, 26 Mar 2020 at 05:47, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:

> Hi,
>
> three more comments after eye-balling the code for a bit longer.
>
> 1) The patch probably needs to tweak config.sgml which says this about
> the enable_partitionwise_join GUC:
>
>    .. Partitionwise join currently applies only when the join conditions
>    include all the partition keys, which must be of the same data type
>    and have exactly matching sets of child partitions. ..
>

Done. Actually this wasn't updated when partition pruning was introduced,
which could cause a partitionwise join to be not used even when those
conditions were met. Similarly when a query involved whole row reference.
It's hard to explain all the conditions under which partitionwise join
technique will be used. But I have tried to keep it easy to understand.


>
> Which is probably incorrect, because the point of this patch is not to
> require exact match of the partitions, right?
>
> 2) Do we really need the 'merged' flag in try_partitionwise_join? Can't
> we simply use the joinrel->merged flag directly? ISTM the we always
> start with joinrel->merged=false, and then only ever set it to true in
> some cases. I've tried doing that, per the attached 0002 patch. The
> regression tests seem to work fine.
>

Thanks. I just added a small prologue to compute_partition_bounds().


>
> I noticed this because I've tried moving part of the function into a
> separate function, and removing the variable makes that simpler.
>
> The patch also does a couple additional minor tweaks.
>
> 3) I think the for nparts comment is somewhat misleading:
>
>    int nparts;  /* number of partitions; 0 = not partitioned;
>                  * -1 = not yet set */
>
> which says that nparts=0 means not partitioned. But then there are
> conditions like this:
>
>      /* Nothing to do, if the join relation is not partitioned. */
>      if (joinrel->part_scheme == NULL || joinrel->nparts == 0)
>          return;
>
> which (ignoring the obsolete comment) seems to say we can have nparts==0
> even for partitioned tables, no?
>

See my previous mail.


>
> Anyway, attached is the original patch (0001) and two patches with
> proposed changes. 0002 removes the "merged" flag as explained in (2),
> 0003 splits the try_partitionwise_join() function into two parts.
>
> I'm saying these changes have to happen and it's a bit crude (and it
> might be a bit of a bikeshedding).
>

I have added 0005 with the changes I described in this as well as the
previous mail. 0004 is just some white space fixes.


>
>
> regards
>
> --
> Tomas Vondra                  http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>


-- 
Best Wishes,
Ashutosh