On 2018/10/02 16:40, Andres Freund wrote:
>>> For executing them we have:
>>> - do_convert_tuple
>>> - ConvertPartitionTupleSlot
>>>
>>> which is two randomly differing spellings of related functionality,
>>> without the name indicating that they, for reasons, don't both use
>>> TupleConversionMap.
>>
>> Actually, ConvertPartitionTupleSlot performs two tasks: 1. performs the
>> conversion of the tuple in the input slot using do_convert_tuple, and 2.
>> switches the TupleTableSlot to be used from this point on to a
>> partition-specific one.
>
> Yea, I had this mostly written with the view of the code after the
> "slottification of partition tuple covnersion" thread.
Ah, okay. I'm looking at the new patch you posted.
>>> I'm also not particularly happy about
>>> "do_convert_tuple", which is a pretty generic name.
>>>
>>> A lot of this probably made sense at some time, but we got to clean this
>>> up. We'll grow more partitioning related code, and this IMO makes
>>> reading the code harder - and given the change rate, that's something I
>>> frequently have to do.
>>
>> On the "Slotification of partition tuple conversion" thread, I suggested
>> that we try to decouple partitioning-related tuple conversion from
>> tupconvert.c, which may significantly improve things imho. See the last
>> paragraph of my message here:
>>
>> https://www.postgresql.org/message-id/ed66156b-54fe-b5d2-10bf-3c73a8052e7e%40lab.ntt.co.jp
>>
>> Basically, Amit Khandekar proposed that we add another function with the
>> same functionality as do_convert_tuple to tupconvert.c that accepts and
>> returns TupleTableSlot instead of HeapTuple. Per discussion, it turned
>> out that we won't need a full TupleConversionMap for partitioning-related
>> conversions if we start using this new function, so we could land the new
>> conversion function in execPartition.c instead of tupconvert.c. Perhaps,
>> we could just open-code do_convert_tuple()'s functionality in the existing
>> ConvertPartitionTupleSlot().
>>
>> Of course, that is not to say we shouldn't do anything about the possibly
>> unhelpful names of the functions that tupconvert.c exports.
>
> I'm not quite sure I see the point of a full decoupling of
> partitioning-related tuple conversion from tupconvert.c. Yes, the
> routines should be more clearly named, but why should the code be
> separate?
Hmm, okay. After looking at your patch on the other thread, I feel less
strongly about this.
> I'm kinda wondering if we shouldn't have the tuple
> conversion functions just use the slot based functionality in the back,
> and just store those in the TupConversionMap.
Sorry, I didn't understand this.
Thanks,
Amit
