Re: Tuple conversion naming

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, amitdkhan(dot)pg(at)gmail(dot)com
Subject: Re: Tuple conversion naming
Date: 2018-10-02 08:28:26
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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:
>> 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.


In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-10-02 08:55:56 Re: transction_timestamp() inside of procedures
Previous Message Etsuro Fujita 2018-10-02 08:20:13 Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.