From: | Andy Fan <zhihuifan1213(at)163(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Nikita Malakhov <hukutoc(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Michael Zhilin <m(dot)zhilin(at)postgrespro(dot)ru>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: Shared detoast Datum proposal |
Date: | 2024-05-24 02:46:38 |
Message-ID: | 874janudgi.fsf@163.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> 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.
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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2024-05-24 02:55:34 | Re: Pgoutput not capturing the generated columns |
Previous Message | Tatsuo Ishii | 2024-05-24 02:39:19 | Re: Row pattern recognition |