Re: including backend ID in relpath of temp rels - updated patch

On Wed, Sep 15, 2010 at 12:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> After further reflection, I think that the above reasoning is wrong.
>> GetNewRelFileNode() guarantees that the OID doesn't collide with the
>> OID column of pg_class, not the relfilenode column of pg_class; and,
>> per the comments to that function, it only guarantees this when
>> creating a new relation (to which we're therefore assigning an OID)
>> and not when rewriting an existing relation.  So it provides no
>> protection against the scenario where backend #1 drops a table that
>> lacks physical storage just as backend #2 assigns that OID to some
>> other relation and begins creating files, which backend #1 then blows
>> away.
>
>> However, I think we're safe for a different reason [ omitted ]
>
> The above scenario is only a risk if you suppose that dropping a
> relation that lacks physical storage will nonetheless result in
> attempted unlink()s.  I think that that's probably not the case;

Why?  How would we know that it didn't have physical storage prior to
attempting the unlinks?

> but it seems related to a question that was bothering me recently.
> Namely: why do we assign relfilenode values to relations that have
> no physical storage?  If we did not do so, then relation drop would
> see a zero in relfilenode and would certainly not attempt an incorrect
> unlink().

I agree that setting relfilenode to 0 for relkinds that lack physical
storage is a good idea, but that's obviously only going to handle that
specific case.

> This came to mind after reading a commit from Bruce in which he had to
> hack up pg_upgrade to preserve relfilenode numbers for storage-less
> relations.  Presumably that hack could get undone.

Seems like a good thing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company