Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW UPDATE trigger

On Mon, Jan 8, 2024 at 3:00 AM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
> > But that makes me wonder ... how exactly do oldslot
> > and newslot end up sharing the same buffer without holding two pins on
> > it? tts_heap_copyslot() looks like it basically forces the tuple to be
> > materialized first and then copies that, so you'd end up with, I
> > guess, no buffer pins at all, rather than 1 or 2.
> >
> > I'm obviously missing something important here...
>
> As my analysis [2] showed, epqslot_clean is always equal to newslot, so
> ExecCopySlot() isn't called at all.
>
> [1] https://www.postgresql.org/message-id/17798-0907404928dcf0dd%40postgresql.org
> [2] https://www.postgresql.org/message-id/e989408c-1838-61cd-c968-1fcf47c6fbba%40gmail.com

Sorry, I still don't get it. I can't really follow the analysis in
[2]. Your analysis there relies on analyzing how
ExecBRUpdateTriggers() gets called, but it seems to me that what
matters is what happens inside that function, specifically what
GetTupleForTrigger does. And I think that function is going to either
produce an EPQ tuple or not depending on whether table_tuple_lock
returns TM_Ok and whether it sets tmfd.traversed, independently of how
ExecBRUpdateTriggers() is called.

Also, even if you're right that epqslot_clean always ends up equal to
newslot, my question was about how oldslot and newslot end up sharing
the same buffer without holding two pins on it, and I don't quite see
what epqslot_clean has to do with that. Apologies if I'm being dense
here; this code seems dramatically under-commented to me. But FWICT
the design here is that a slot only holds a pin until somebody copies
it or materializes it. So I don't understand what conceptually could
happen that would end up with two slots holding the same pin, unless
it was just that you had two variables holding the same pointer value.
But if that's what is happening, then materializing one slot would
also materialize the other.

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