Re: Relations being opened without any lock whatever

On Sun, Sep 30, 2018 at 03:20:44PM -0400, Tom Lane wrote:
> 1. ALTER TABLE ... ALTER COLUMN TYPE, on a column that is part of
> a foreign key constraint, opens the rel that has the other end of
> the constraint before it's acquired a lock on said rel.
> 
> The comment in ATPostAlterTypeCleanup claims this is "safe because the
> parser won't actually look at the catalogs to detect the existing entry",
> but I think that's largely horsepucky.  The parser absolutely does do
> relation_open, and it expects the caller to have gotten a lock sufficient
> to protect that (cf transformAlterTableStmt).
> 
> It's possible that this doesn't have any real effect.  Since we're
> already holding AccessExclusiveLock on our own end of the FK constraint,
> it'd be impossible for another session to drop the FK constraint, or
> by extension the other table.  Still, running a relcache load on a
> table we have no lock on seems pretty unsafe, especially so in branches
> before we used MVCC for catalog reads.  So I'm inclined to apply the
> attached patch all the way back.  (The mentioned comment also needs
> rewritten; this is just the minimal code change to get rid of the test
> failure.)

Okay, that's bad.  Wouldn't it be sufficient to use what the caller
passes out as lockmode instead of enforcing AEL though?

> 2. pageinspect's tuple_data_split_internal supposes that it needs no
> lock on the referenced table.  Perhaps there was an expectation that
> some earlier function would have taken a lock and not released it,
> but this is demonstrably not happening in the module's own regression
> test.  I think we should just take AccessShareLock there and not try
> to be cute.  Again, this seems to be back-patch material.

Yes, that's incorrect.  So +1 on this one.
--
Michael