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