Re: pgsql: Add assertions that we hold some relevant lock during relation o

On 2018/10/02 1:43, Tom Lane wrote:
> Add assertions that we hold some relevant lock during relation open.
>
> Opening a relation with no lock at all is unsafe; there's no guarantee
> that we'll see a consistent state of the relevant catalog entries.
> While use of MVCC scans to read the catalogs partially addresses that
> complaint, it's still possible to switch to a new catalog snapshot
> partway through loading the relcache entry. Moreover, whether or not
> you trust the reasoning behind sometimes using less than
> AccessExclusiveLock for ALTER TABLE, that reasoning is certainly not
> valid if concurrent users of the table don't hold a lock corresponding
> to the operation they want to perform.
>
> Hence, add some assertion-build-only checks that require any caller
> of relation_open(x, NoLock) to hold at least AccessShareLock. This
> isn't a full solution, since we can't verify that the lock level is
> semantically appropriate for the action --- but it's definitely of
> some use, because it's already caught two bugs.
>
> We can also assert that callers of addRangeTableEntryForRelation()
> hold at least the lock level specified for the new RTE.
>
> Amit Langote and Tom Lane
>
> Discussion: https://postgr.es/m/16565.1538327894@sss.pgh.pa.us

Thanks for committing.

I'm sorry if it wasn't clear, but the lock manager code was added to the
original 0001 patch by David Rowley; see this message where he posted the
updated version of my patch containing the new code:

https://www.postgresql.org/message-id/CAKJS1f83VJjrnN6hQtZ7vJ%2BE4_%2BtH%3DU2VK6hgr74fZN-%3DDq3Ag%40mail.gmail.com

Thanks,
Amit