Message-ID: <17124cd9-6515-2076-6eac-40d44388f833@gmail.com>
Date: Fri, 12 Jan 2024 20:00:01 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW
 UPDATE trigger
Content-Language: en-US
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org,
 Richard Guo <guofenglinux@gmail.com>, Andres Freund <andres@anarazel.de>,
 Peter Geoghegan <pg@bowt.ie>
References: <17798-0907404928dcf0dd@postgresql.org>
 <87fe6c28-80b2-f709-a8db-d105e5240e39@gmail.com>
 <CAMbWs4_pg7ab=OJF2CGLzs=noWEjujerxfo92sm3RL9ce=ogsQ@mail.gmail.com>
 <CAMbWs48FHhuuuWbxygO4Wey7ahdaSwRjzJODKXx7=GFsge_aLg@mail.gmail.com>
 <f5ff446f-72e2-44f7-80c6-4256cf7caeb3@gmail.com>
 <a4a8cf37-e9d5-3c58-fc2e-d217cf66f315@gmail.com>
 <1849610.1682807087@sss.pgh.pa.us>
 <e989408c-1838-61cd-c968-1fcf47c6fbba@gmail.com>
 <CA+TgmoZz1fGaTnOSf1yQhwYovEEkJZ9iioQXnLgaJz+1fBip=g@mail.gmail.com>
 <494cd7c4-c109-6886-c0e3-138a95e7e38b@gmail.com>
 <CA+TgmoYAsQWMqMUNNdyp8V0KVxMoz=0y+3XNCf3ST-o1yyz5Jg@mail.gmail.com>
 <74335d44-819d-60e0-859a-efe74d9c84c2@gmail.com>
 <CA+TgmoYkOp=jDg15ugJDwxqoA6drAPBZmOJkhZJ11HZWhEYx6w@mail.gmail.com>
 <b64c183f-4ae1-0b16-afcf-11101ebc4213@gmail.com>
 <CA+TgmobRr7iWYNNj+-djD6AkoS5trpL8JznXbfO74+dtmCsOsg@mail.gmail.com>
 <325298.1705009396@sss.pgh.pa.us>
 <94ca869c-472b-f1cf-3d7c-aa8530f5475d@gmail.com>
 <CA+TgmoYcYwBxV7J_Us=zz7Wchbg1+M6jPdbogZFYZD1WmZB9ZQ@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: 
 <CA+TgmoYcYwBxV7J_Us=zz7Wchbg1+M6jPdbogZFYZD1WmZB9ZQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/17124cd9-6515-2076-6eac-40d44388f833%40gmail.com>
Precedence: bulk

12.01.2024 17:56, Robert Haas wrote:
> On Fri, Jan 12, 2024 at 8:00 AM Alexander Lakhin <exclusion@gmail.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