Re: minor fix for acquire_inherited_sample_rows

On 2018/04/27 22:42, Ashutosh Bapat wrote:
> On Thu, Apr 26, 2018 at 7:08 PM, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> On Thu, Apr 26, 2018 at 9:54 PM, Ashutosh Bapat
>> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>>> On Thu, Apr 26, 2018 at 1:08 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>>> +1. I think we're really abusing equalTupleDescs() for purposes for
>>>> which it was not invented. Instead of changing it, let's invent a new
>>>> function that tests for the thing partitioning cares about (same
>>>> ordering of the same columns with the same type information) and call
>>>> it logicallyEqualTupleDescs() or something like that.
>>>
>>> Why don't we just rely on the output of convert_tuples_by_name(),
>>> which it seems is always called right now? What's advantage of adding
>>> another tuple descriptor comparison?
>>
>> The patch I mentioned in my email above does more or less that (what
>> you're saying we should do). In fact it even modifies
>> convert_tuple_by_name and convert_tuple_by_name_map to remove some
>> redundant computation. See that patch here if you're interested:
>>
>> /message-id/825031be-942c-8c24-6163-13c27f217a3d%40lab.ntt.co.jp
>
> I spent some time looking at the patch. The patch clubs all kinds of
> refactoring together, making review a bit difficult. I think, it would
> be better to split the patch into multiple, each addressing one set of
> changes, it might become easier to review.

Thanks Ashutosh for looking at it. I will think of a way to break it up
when I re-propose it for the next cycle.

Regards,
Amit