Re: minor fix for acquire_inherited_sample_rows

On Mon, Apr 23, 2018 at 6:45 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Thanks for the review.
>
> On Mon, Apr 23, 2018 at 8:25 PM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> On Mon, Apr 23, 2018 at 3:44 PM, Amit Langote
>> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> Hi.
>>>
>>> acquire_inherited_sample_rows() currently uses equalTupleDescs() being
>>> false as the condition for going to tupconv.c to determine whether tuple
>>> conversion is needed. But equalTupleDescs() will always return false if
>>> it's passed TupleDesc's of two different tables, which is the most common
>>> case here. So I first thought we should just unconditionally go to
>>> tupconv.c, but there is still one case where we don't need to, which is
>>> the case where the child table is same as the parent table. However, it
>>> would be much cheaper to just check if the relation OIDs are different
>>> instead of calling equalTupleDescs, which the attached patch teaches it to do.
>>
>> We want to check whether tuple conversion is needed. Theoretically
>> (not necessarily practically), one could have tuple descs of two
>> different tables stamped with the same tdtypeid. From that POV,
>> equalTupleDescs seems to be a stronger check than what you have in the
>> patch.
>
> If I'm reading right, that sounds like a +1 to the patch. :)

Not really! To make things clear, I am not in favour of this patch.

>
>> BTW the patch you have posted also has a fix you proposed in some
>> other thread. Probably you want to get rid of it.
>
> Oops, fixed that in the attached.

Thanks.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company