From: | amul sul <sulamul(at)gmail(dot)com> |
---|---|
To: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
Cc: | 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>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, 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>, Thomas Munro <thomas(dot)munro(at)enterprisedb(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: | 2019-10-25 09:59:03 |
Message-ID: | CAAJ_b97idvMzkBN-er5NNQQ4uw31Tc-3RFaaNKeS0oTKLwS8bQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 16, 2019 at 6:20 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:
> On Wed, Sep 25, 2019 at 12:59 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
> wrote:
> > I will continue to review the rest of the patch.
>
> I've been reviewing the rest of the patch. Here are my review comments:
>
[....]
> So I'd like to propose to introduce separate functions like
> process_outer_partition() and process_inner_partition() in the
> attached, instead of handle_missing_partition(). (I added a fast path
> to these functions that if both outer/inner sides have the default
> partitions, give up on merging partitions. Also, I modified the
> caller sites of proposed functions so that they call these if
> necessary.)
>
Agree -- process_outer_partition() & process_inner_partition() approach
looks
much cleaner than before.
Here are the few comments:
Note that LHS numbers are the line numbers in your previously posted
patch[1].
455 + if (inner_has_default ||
456 + jointype == JOIN_LEFT ||
457 + jointype == JOIN_ANTI ||
458 + jointype == JOIN_FULL)
459 + {
460 + if (!process_outer_partition(&outer_map,
512 + if (outer_has_default || jointype == JOIN_FULL)
513 + {
514 + if (!process_inner_partition(&outer_map,
How about adding these conditions to the else block of
process_outer_partition()
& process_inner_partition() function respectively so that these functions
can be
called unconditionally? Thoughts/Comments?
---
456 + jointype == JOIN_LEFT ||
457 + jointype == JOIN_ANTI ||
458 + jointype == JOIN_FULL)
Also, how about using IS_OUTER_JOIN() instead. But we need an assertion to
restrict JOIN_RIGHT or something.
For the convenience, I did both aforesaid changes in the attached delta
patch that
can be applied atop of your previously posted patch[1]. Kindly have a look
& share
your thoughts, thanks.
--
1273 + * *next_index is incremented when creating a new merged partition
associated
1274 + * with the given outer partition.
1275 + */
Correction: s/outer/inner
---
1338 + * In range partitioning, if the given outer partition is
already
1339 + * merged (eg, because we found an overlapping range earlier),
we know
1340 + * where it fits in the join result; nothing to do in that
case. Else
1341 + * create a new merged partition.
Correction: s/outer/inner.
---
1712 /*
1713 * If the NULL partition was missing from the inner side of
the join,
s/inner side/outer side
--
Regards,
Amul
1]
https://postgr.es/m/CAPmGK145V8DNCNQ2gQBgNE3QqoJGjsmK5WMwaA3FMirNM723KQ%40mail.gmail.com
Attachment | Content-Type | Size |
---|---|---|
delta.patch | application/x-patch | 10.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2019-10-25 11:30:26 | Re: tuplesort test coverage |
Previous Message | Amit Kapila | 2019-10-25 09:58:47 | Re: Ordering of header file inclusion |