Robert Haas <robertmh...@gmail.com> writes:
> On Wed, May 22, 2024 at 9:46 PM Andy Fan <zhihuifan1...@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
