From: | Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, amul sul <sulamul(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Antonin Houska <ah(at)cybertec(dot)at>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] advanced partition matching algorithm for partition-wise join |
Date: | 2020-04-01 16:51:17 |
Message-ID: | CAG-ACPWF_PJpN9MjRAahuz=CWrH_Lq63nNOzzmf1X4qroHuiuQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 26 Mar 2020 at 05:47, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)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
Attachment | Content-Type | Size |
---|---|---|
0002-remove-merged-flag.patch | text/x-patch | 3.8 KB |
0003-split-try_partitionwise_join.patch | text/x-patch | 6.5 KB |
0001-v33.patch | text/x-patch | 227.1 KB |
0004-Fix-some-white-space-errors-in-the-previous-commits.patch | text/x-patch | 1.1 KB |
0005-Address-Tomas-s-comments.patch | text/x-patch | 26.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2020-04-01 17:03:37 | Re: snapshot too old issues, first around wraparound and then more. |
Previous Message | Tom Lane | 2020-04-01 16:29:51 | Re: proposal \gcsv |