On 2018/10/01 2:18, Tom Lane wrote:
> I wrote:
>> 1. You set up transformRuleStmt to insert AccessExclusiveLock into
>> the "OLD" and "NEW" RTEs for a view. This is surely wrong; we do
>> not want to take exclusive lock on a view just to run a query using
>> the view. It should (usually, anyway) just be AccessShareLock.
>> However, because addRangeTableEntryForRelation insists that you
>> hold the requested lock type *now*, just changing the parameter
>> to AccessShareLock doesn't work.
>> I hacked around this for the moment by passing NoLock to
>> addRangeTableEntryForRelation and then changing rte->lockmode
>> after it returns, but man that's ugly. It makes me wonder whether
>> addRangeTableEntryForRelation should be checking the lockmode at all.
>
> It occurred to me that it'd be reasonable to insist that the caller
> holds a lock *at least as strong* as the one being recorded in the RTE,
> and that there's also been discussions about verifying that some lock
> is held when something like heap_open(foo, NoLock) is attempted.
> So I dusted off the part of 0001 that did that, producing the
> attached delta patch.
>
> Unfortunately, I can't commit this, because it exposes at least two
> pre-existing bugs :-(. So we'll need to fix those first, which seems
> like it should be a separate thread. I'm just parking this here for
> the moment.
>
> I think that the call sites should ultimately look like
>
> Assert(CheckRelationLockedByMe(...));
>
> but for hunting down the places where the assertion currently fails,
> it's more convenient if it's just an elog(WARNING).
Should this check that we're not in a parallel worker process?
Thanks,
Amit
