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
