Thanks for the review.
On Mon, Apr 23, 2018 at 8:25 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Mon, Apr 23, 2018 at 3:44 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.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. :)
> 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,
Amit