From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Subject: | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |
Date: | 2022-08-04 23:01:06 |
Message-ID: | 3679800.1659654066@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-08-04 23:11:13 | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |
Previous Message | Andres Freund | 2022-08-04 22:51:47 | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |