From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: Dilip Kumar <dilipbalaut@gmail.com>, Andres Freund <andres@anarazel.de>,
        Justin Pryzby <pryzby@telsasoft.com>,
        Ashutosh Sharma <ashu.coek88@gmail.com>,
        Maciek Sakrejda <m.sakrejda@gmail.com>,
        Bruce Momjian <bruce@momjian.us>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Andrew Dunstan <andrew@dunslane.net>,
        Heikki Linnakangas <hlinnaka@iki.fi>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Thomas Munro <thomas.munro@gmail.com>
Subject: Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints
In-reply-to: <3663919.1659650570@sss.pgh.pa.us>
References: 
 <CA+TgmoZHDwEjPHO6RceWZZ5L1FeJ4_VEi0t39rnWFBAbnoq53w@mail.gmail.com>
 <CAFiTN-sdxdSYObqO7-LYy=7tAB1zamT3trPq7ukfupKPMnjYdg@mail.gmail.com>
 <CAFiTN-ujhw-ttQ9-1pdcXtSn3mxhKCBQYvQsq=hQHQZwSn4xjQ@mail.gmail.com>
 <CAFiTN-tSUj56cyS6a88sgn3MWJBOA_VjEQfc7fZmWicKwijraA@mail.gmail.com>
 <CA+TgmoYVDLtS0v6XNTk=xBVStjZu_XaS5FTY74neb_h8YOcF0w@mail.gmail.com>
 <CA+TgmobUW9NQK7EJkV_0F87bqqq5BSHKiX2+wfthLcy4qA7YvQ@mail.gmail.com>
 <20220802175043.GA13682@telsasoft.com>
 <CA+TgmobZufK6PcCRpvjP6dJqOBO-woWZF0cqMHWGvzkbgeaBKw@mail.gmail.com>
 <2767668.1659476794@sss.pgh.pa.us> <20220802220416.GC13682@telsasoft.com>
 <20220802222334.umf5wsa2r63koppn@awork3.anarazel.de>
 <CAFiTN-sG9-ocJFN+KBAQ2mTuF-9Ja+i1Qo81yqr0WDOsqqYpLg@mail.gmail.com>
 <CAFiTN-sL8M+Z6nongs=rSqfP26Vr0O_EYLfcwbO-Emz1GYxtxA@mail.gmail.com>
 <CAFiTN-v4u2WJT6y1n4wtM4Z3tffJonT7zuRKegYao9p-Oyb07Q@mail.gmail.com>
 <CAFiTN-thCGvRF0_WftbDc0KFagB3vgHHGrpNdRMJ6vtkObEMoA@mail.gmail.com>
 <CA+Tgmoa9Heqc3mr=viBQ0Y0tqQz5G8oUAc23ZmbNW5__cud28A!
 ! @mail.gmail.com> <3663919.1659650570@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 04 Aug 2022 18:02:50 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3679799.1659654066.1@sss.pgh.pa.us>
Date: Thu, 04 Aug 2022 19:01:06 -0400
Message-ID: <3679800.1659654066@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3679800.1659654066%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> While I'm at it, I would like to strenuously object to the current
> framing of CreateAndCopyRelationData as a general-purpose copying
> mechanism.

And while I'm piling on, how is this bit in RelationCopyStorageUsingBuffer
not completely broken?

        /* Read block from source relation. */
        srcBuf = ReadBufferWithoutRelcache(src->rd_locator, forkNum, blkno,
                                           RBM_NORMAL, bstrategy_src,
                                           permanent);
        srcPage = BufferGetPage(srcBuf);
        if (PageIsNew(srcPage) || PageIsEmpty(srcPage))
        {
            ReleaseBuffer(srcBuf);
            continue;
        }

        /* Use P_NEW to extend the destination relation. */
        dstBuf = ReadBufferWithoutRelcache(dst->rd_locator, forkNum, P_NEW,
                                           RBM_NORMAL, bstrategy_dst,
                                           permanent);

You can't skip pages just because they are empty.  Well, maybe you could
if you were doing something to ensure that you zero-fill the corresponding
blocks on the destination side.  But this isn't doing that.  It's using
P_NEW for dstBuf, which will have the effect of silently collapsing out
such pages.  Maybe in isolation a heap could withstand that, but its
indexes won't be happy (and I guess t_ctid chain links won't either).

I think you should just lose the if() stanza.  There's no optimization to
be had here that's worth any extra complication.

(This seems worth fixing before beta3, as it looks like a rather
nasty data corruption hazard.)

			regards, tom lane