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