Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, May 22, 2024 at 9:46 PM Andy Fan <zhihuifan1213@163.com> wrote:
>> Please give me one more chance to explain this. What I mean is:
>>
>> Take SELECT f(a) FROM t1 join t2...; for example,
>>
>> When we read the Datum of a Var, we read it from tts->tts_values[*], no
>> matter what kind of TupleTableSlot is. So if we can put the "detoast
>> datum" back to the "original " tts_values, then nothing need to be
>> changed.
>
> Yeah, I don't think you can do that.
>
>> - Saving the "detoast datum" version into tts_values[*] doesn't break
>> the above semantic and the ExprEval engine just get a detoast version
>> so it doesn't need to detoast it again.
>
> I don't think this is true. If it is true, it needs to be extremely
> well-justified. Even if this seems to work in simple cases, I suspect
> there will be cases where it breaks badly, resulting in memory leaks
> or server crashes or some other kind of horrible problem that I can't
> quite imagine. Unfortunately, my knowledge of this code isn't
> fantastic, so I can't say exactly what bad thing will happen, and I
> can't even say with 100% certainty that anything bad will happen, but
> I bet it will. It seems like it goes against everything I understand
> about how TupleTableSlots are supposed to be used. (Andres would be
> the best person to give a definitive answer here.)
>
>> - The keypoint is the memory management and effeiciency. for now I think
>> all the places where "slot->tts_nvalid" is set to 0 means the
>> tts_values[*] is no longer validate, then this is the place we should
>> release the memory for the "detoast datum". All the other places like
>> ExecMaterializeSlot or ExecCopySlot also need to think about the
>> "detoast datum" as well.
>
> This might be a way of addressing some of the issues with this idea,
> but I doubt it will be acceptable. I don't think we want to complicate
> the slot API for this feature. There's too much downside to doing
> that, in terms of performance and understandability.
Thanks for the doubt! Let's see if Andres has time to look at this. I
feel great about the current state.
--
Best Regards
Andy Fan