Re: Orphan page in _bt_split

On 25/09/2025 7:35 AM, Michael Paquier wrote:
> On Mon, Sep 22, 2025 at 07:44:18PM +0300, Константин Книжник wrote:
>> Attached please find rebased version of the patch with fixed mistypings.
> I have looked at v3.
>
> -   leftpage = PageGetTempPage(origpage);
> +   leftpage = leftpage_buf.data;
> +   memcpy(leftpage, origpage, BLCKSZ);
>      _bt_pageinit(leftpage, BufferGetPageSize(buf));
>
> What's the point of copying the contents of origpage to leftpage?
> _bt_pageinit() is going to initialize leftpage (plus a few more things
> set like the  page LSN, etc.), so the memcpy should not be necessary,
> no?

Sorry, memcpy is really not needed here.

>
> +   rightpage = BufferGetPage(rbuf);
> +   memcpy(rightpage, rightpage_buf.data, BLCKSZ);
> +   ropaque = BTPageGetOpaque(rightpage);
>
> When we reach this state of the logic, aka at the beginning of the
> critical section, the right and left pages are in a correct state,
> and your code is OK because we copy the contents of the right page
> back into its legitimate place.  What is not OK to me is that the
> copy of the "temporary" right page back to "rbuf" is not explained.  I
> think that this deserves a comment, especially the part about ropaque
> which is set once by this proposal, then manipulated while in the
> critical section.


I added the comment, please check that it is clear and complete.
Updated version of the patch is attached.