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
