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

12.01.2024 17:56, Robert Haas wrote:
> On Fri, Jan 12, 2024 at 8:00 AM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
>> I had decided to move that step out of "if (epqslot_candidate != NULL)"
>> in v3 to play safe when removing ExecMaterializeSlot(newslot) brought by
>> 75e03eabe (more details upthread [1]). I thought that if that commit stated
>> that the slot needs materialization (it was time before 86dc90056, which
>> raised the current issue), then the materialization must be preserved.
>> (The same commit can be found in zheap repository [2], but I found no other
>> explanation of it's purpose...)
>> Though looking at the current core code, I'm inclined to perform
>> ExecMaterializeSlot() only when we have epqslot_candidate, if we can afford
>> not to bother about that zheap-specific or similar scenario...
> I don't understand why it's ever safe to skip ExecMaterializeSlot
> here. IIUC, if we have no EPQ slot, newslot is still not materialized
> and thus dependent on oldslot ... and we're about to
> ExecFetchSlotHeapTuple on oldslot.

To my understanding, when we have no epqslot_candidate, it means that
newslot came from ExecBRUpdateTriggers's callers and it can't be dependent
on an internal oldslot's buffer.
For example, ExecModifyTable() does the following:
            slot = ExecGetUpdateNewTuple(resultRelInfo, context.planSlot,
                                            oldSlot);
            context.relaction = NULL;

            /* Now apply the update. */
            slot = ExecUpdate(&context, resultRelInfo, tupleid, oldtuple,
                                slot, node->canSetTag);

Thus, newslot (aka slot here) depends on oldSlot inside ExecModifyTable(),
but not on oldslot inside ExecBRUpdateTriggers(), so
ExecFetchSlotHeapTuple(oldslot,...) can't affect it. Though then we still
have the question regarding zheap open -- can some external code introduce
new implicit dependencies?

Best regards,
Alexander