Hi Amul, On Fri, Nov 15, 2019 at 2:21 PM amul sul <sula...@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
