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

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>, 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-11-05 13:14:50
Message-ID: CAAJ_b94D5KPaOyHUyvoyYenxyZx0prEuq5+bLU98Usi+E2G9Ow@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 1, 2019 at 3:58 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:

> On Thu, Oct 31, 2019 at 6:49 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
> wrote:
> > Attached is an updated version. If no objections, I'll merge this
> > with the main patch [1].
>
> Done. Attached is a new version of the patch.
>
> Other changes: in generate_matching_part_pairs(), I changed variable
> names to match other functions, simplified assertions, and
> adjusted/added comments a bit.
>

Thanks for the update version.

A query and comments for v25:

583 + * The function returns NULL if we can not find the matching pair of
584 + * partitions. This happens if 1. multiple partitions on one side
match with
585 + * one partition on the other side. 2. a given partition on the outer
side
586 + * doesn't have a matching partition on the inner side. We can not
support the
587 + * first case since we don't have a way to represent multiple
partitions as a
588 + * single relation (RelOptInfo) and then perform join using the ganged
589 + * relation. We can not support the second case since the missing
inner
590 + * partition needs to be represented as an empty relation and we
don't have a
591 + * way to introduce empty relation during join planning after
creating paths
592 + * for all the base relations.
593 + */
594 +PartitionBoundInfo
595 +partition_bounds_merge(int partnatts,

I think the second condition mention for partition_bounds_merge() looks
outdated, do you think we should remove that or am I missing something here?
---

1768 +
1769 + /*
1770 + * If this is an outer join, the merged partition would act as
the
1771 + * default partition of the join; record the index in
*default_index
1772 + * if not done yet.
1773 + */
1774 + if (jointype == JOIN_LEFT || jointype == JOIN_ANTI ||
1775 + jointype == JOIN_FULL)
1776 + {

As decided need to replace this list by IS_OUTER_JOIN(jointype).
---

2020 + if (jointype == JOIN_LEFT || jointype == JOIN_FULL ||
2021 + jointype == JOIN_ANTI)
2022 + {

Same as the previous.
---

I tried a coverage testing and tried to adjust & add a few tests to
improved the
code coverage for the v25 patch. Please have a look at the attached 0002 &
also
attach the coverage output with & without this patch, TWIMW.

0001 is the same v25 patch, reattaching to make CFbot happy.

Regards,
Amul

Attachment Content-Type Size
0001-Improve-partition-matching-for-partitionwise-joins-v.patch application/octet-stream 295.1 KB
0002-Few-more-tests-and-adjustments.patch application/octet-stream 17.3 KB
enable-coverage-without-002-patch.out application/octet-stream 4.4 KB
enable-coverage-with-002-patch.out application/octet-stream 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-11-05 13:38:50 Re: progress report for ANALYZE
Previous Message Alvaro Herrera 2019-11-05 13:12:12 Re: v12 and pg_restore -f-