Thanks for the review. On Mon, Apr 23, 2018 at 8:25 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Mon, Apr 23, 2018 at 3:44 PM, Amit Langote > <langote_amit...@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
condition-may-need-tuple-conversion.patch
Description: Binary data
