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

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: amul sul <sulamul(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-15 09:10:25
Message-ID: CAPmGK14cwchOiVsHPnpz1ujUm3BEzDE1nJyWi3OJDEzyiFFfrg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amul,

On Fri, Nov 15, 2019 at 2:21 PM amul sul <sulamul(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-11-15 09:17:12 Re: SQL/JSON: JSON_TABLE
Previous Message Pantelis Theodosiou 2019-11-15 09:06:10 Re: [PATCH] Implement INSERT SET syntax