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

Etsuro Fujita <etsuro.fujita@gmail.com>

From: Etsuro Fujita <etsuro.fujita@gmail.com>
To: amul sul <sulamul@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, 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: 2019-11-15T09:10:25Z
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.

Hi Amul,

On Fri, Nov 15, 2019 at 2:21 PM amul sul <sulamul@gmail.com> wrote:
> Thank you Fujita san for the patch & the enhancements.  I am fine with your
> delta patch.

OK, I'll merge the delta patch with the main one in the next version,
if no objections from others.

> I would like to share some thought for other code:
>
> File: partbounds.c:
> 3298 static void
> 3299 get_merged_range_bounds(int partnatts, FmgrInfo *partsupfuncs,
> 3300                         Oid *partcollations, JoinType jointype,
> 3301                         PartitionRangeBound *left_lb,
> 3302                         PartitionRangeBound *left_ub,
> 3303                         PartitionRangeBound *right_lb,
> 3304                         PartitionRangeBound *right_ub,
> 3305                         PartitionRangeBound *merged_lb,
> 3306                         PartitionRangeBound *merged_ub)
>
> Can we rename these argument as inner_*  & outer_*  like we having for the
> functions, like 0003 patch?

+1  (Actually, I too was thinking about that.)

> File: partbounds.c:
> 3322
> 3323         case JOIN_INNER:
> 3324         case JOIN_SEMI:
> 3325             if (compare_range_bounds(partnatts, partsupfuncs, partcollations,
> 3326                                      left_ub, right_ub) < 0)
> 3327                 *merged_ub = *left_ub;
> 3328             else
> 3329                 *merged_ub = *right_ub;
> 3330
> 3331             if (compare_range_bounds(partnatts, partsupfuncs, partcollations,
> 3332                                      left_lb, right_lb) > 0)
> 3333                 *merged_lb = *left_lb;
> 3334             else
> 3335                 *merged_lb = *right_lb;
> 3336             break;
> 3337
>
> How about reusing ub_cmpval & lb_cmpval  instead of calling
> compare_range_bounds() inside get_merged_range_bounds(), like 0004 patch?

Good idea!  So, +1

> Apart from this, I would like to propose 0005 cleanup patch where I have
> rearranged function arguments & code to make sure the arguments & the code
> related to lower bound should come first and then the upper bound.

+1

I'll merge these changes as well into the main patch, if no objections
of others.

Thanks for the review and patches!

Best regards,
Etsuro Fujita