On Thu, Feb 8, 2018 at 10:41 AM, Amit Langote
<Langote_Amit_f8@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