|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Andres Freund <andres(at)anarazel(dot)de>|
|Subject:||Re: Tuple conversion naming|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
>> 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
>> 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
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.
|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.|