On Thu, Feb 8, 2018 at 10:41 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > On 2018/02/08 11:55, Amit Langote wrote: >> Hi Ashutosh. >> >> On 2018/02/07 13:51, Ashutosh Bapat wrote: >>> Here's a new patchset with following changes >>> >>> 1. Rebased on the latest head taking care of partition bound >>> comparison function changes >> >> I was about to make these changes myself while revising the fast pruning >> patch. Instead, I decided to take a look at your patch and try to use it >> in my tree. > > I also noticed that a later patch adds partsupfunc to PartitionScheme, > which the pruning patch needs too. So, perhaps would be nice to take out > that portion of the patch. That is, the changes to PartitionScheme struct > definition and those to find_partition_scheme().
I am not sure whether a patch with just that change and without any changes to use that member will be acceptable. So leaving this aside. > > Regarding the latter, wouldn't be nice to have a comment before the code > that does the copying about why we don't compare the partsupfunc field to > decide if we have a match or not. I understand it's because the > partsupfunc array contains pointers, not OIDs. But maybe, that's too > obvious to warrant a comment. It's because partsupfuncs should point to the information of the same function when partopfamily matches and partopcintype matches. I would have added an assertion for that with a comment, but with the pointer that would be risky. Or we can just assert that the oids match. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
