Re: Shared detoast Datum proposal

On Wed, May 22, 2024 at 9:46 PM Andy Fan <zhihuifan1213(at)163(dot)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.

--
Robert Haas
EDB: http://www.enterprisedb.com