On Mon, Apr 23, 2018 at 6:45 PM, Amit Langote <amitlangot...@gmail.com> wrote: > 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. :)
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